Diff
Modified: trunk/Source/WebCore/ChangeLog (98571 => 98572)
--- trunk/Source/WebCore/ChangeLog 2011-10-27 14:51:32 UTC (rev 98571)
+++ trunk/Source/WebCore/ChangeLog 2011-10-27 14:51:52 UTC (rev 98572)
@@ -1,3 +1,40 @@
+2011-10-27 Kentaro Hara <[email protected]>
+
+ Refactor OptionsObject.cpp
+ https://bugs.webkit.org/show_bug.cgi?id=70572
+
+ Reviewed by Adam Barth.
+
+ For example, OptionsObject::getKeyBool() is an alias of
+ OptionsObject::getKeyValue(const String& key, bool& value).
+ We should remove OptionsObject::getKeyXXXX() (XXXX is some specific type)
+ and unify them into OptionsObject::get(const String& key, XXXX& value).
+ c.f. Corresponding JSC methods are unified into
+ JSDictionary::convertValue(JSC::ExecState*, JSC::JSValue, XXXX&).
+
+ The result of git diff is weird, but this patch is making just the following changes:
+ - Replaced getKeyBool(), getKeyInt32(), getKeyDouble() and getKeyString() with get().
+ - Renamed getKeyStringWithUndefinedOrNullCheck() to getWithUndefinedOrNullCheck().
+ - Removed getKeyDOMStringList() and getKeyKeyRange(), since these are not used.
+ - Move definitions of get() from .h to .cpp.
+
+ No new tests. No change in behavior.
+
+ * bindings/v8/OptionsObject.cpp:
+ (WebCore::OptionsObject::get): Renamed from getKeyValue().
+ (WebCore::OptionsObject::getKey): No change to this method. git diff seems to misunderstand as if it is modified.
+ (WebCore::OptionsObject::getKeyDOMStringList): Removed, since no one is using it.
+ (WebCore::OptionsObject::getKeyKeyRange): Ditto.
+ (WebCore::OptionsObject::getWithUndefinedOrNullCheck): No change to this method. git diff seems to misunderstand as if it is modified.
+ * bindings/v8/OptionsObject.h: Moved definitions of get() to OptionsObject.cpp
+ * bindings/v8/custom/V8EventConstructors.cpp:
+ * bindings/v8/custom/V8WebKitMutationObserverCustom.cpp:
+ (WebCore::V8WebKitMutationObserver::observeCallback): Replaced getKeyXXXX() to get();
+ * storage/IDBDatabase.cpp:
+ (WebCore::IDBDatabase::createObjectStore): Ditto.
+ * storage/IDBObjectStore.cpp:
+ (WebCore::IDBObjectStore::createIndex): Ditto.
+
2011-10-27 Andreas Kling <[email protected]>
StyleSheet.parentStyleSheet does not work.
Modified: trunk/Source/WebCore/bindings/v8/OptionsObject.cpp (98571 => 98572)
--- trunk/Source/WebCore/bindings/v8/OptionsObject.cpp 2011-10-27 14:51:32 UTC (rev 98571)
+++ trunk/Source/WebCore/bindings/v8/OptionsObject.cpp 2011-10-27 14:51:52 UTC (rev 98572)
@@ -65,8 +65,24 @@
return WebCore::isUndefinedOrNull(m_options);
}
-bool OptionsObject::getKeyBool(const String& key, bool& value) const
+bool OptionsObject::getKey(const String& key, v8::Local<v8::Value>& value) const
{
+ if (isUndefinedOrNull())
+ return false;
+ v8::Local<v8::Object> options = m_options->ToObject();
+ ASSERT(!options.IsEmpty());
+
+ v8::Handle<v8::String> v8Key = v8String(key);
+ if (!options->Has(v8Key))
+ return false;
+ value = options->Get(v8Key);
+ if (value.IsEmpty())
+ return false;
+ return true;
+}
+
+bool OptionsObject::get(const String& key, bool& value) const
+{
v8::Local<v8::Value> v8Value;
if (!getKey(key, v8Value))
return false;
@@ -78,7 +94,7 @@
return true;
}
-bool OptionsObject::getKeyInt32(const String& key, int32_t& value) const
+bool OptionsObject::get(const String& key, int32_t& value) const
{
v8::Local<v8::Value> v8Value;
if (!getKey(key, v8Value))
@@ -91,7 +107,7 @@
return true;
}
-bool OptionsObject::getKeyDouble(const String& key, double& value) const
+bool OptionsObject::get(const String& key, double& value) const
{
v8::Local<v8::Value> v8Value;
if (!getKey(key, v8Value))
@@ -104,7 +120,7 @@
return true;
}
-bool OptionsObject::getKeyString(const String& key, String& value) const
+bool OptionsObject::get(const String& key, String& value) const
{
v8::Local<v8::Value> v8Value;
if (!getKey(key, v8Value))
@@ -118,71 +134,17 @@
return true;
}
-bool OptionsObject::getKeyStringWithUndefinedOrNullCheck(const String& key, String& value) const
+bool OptionsObject::get(const String& key, ScriptValue& value) const
{
v8::Local<v8::Value> v8Value;
- if (!getKey(key, v8Value) || v8Value->IsNull() || v8Value->IsUndefined())
- return false;
-
- // FIXME: It is possible for this to throw in which case we'd be getting back
- // an empty string and returning true when we should be returning false.
- // See fast/dom/Geolocation/script-tests/argument-types.js for a similar
- // example.
- value = WebCore::isUndefinedOrNull(v8Value) ? String() : v8ValueToWebCoreString(v8Value);
- return true;
-}
-
-PassRefPtr<DOMStringList> OptionsObject::getKeyDOMStringList(const String& key) const
-{
- v8::Local<v8::Value> v8Value;
if (!getKey(key, v8Value))
- return 0;
-
- if (!v8Value->IsArray())
- return 0;
-
- RefPtr<DOMStringList> ret = DOMStringList::create();
- v8::Local<v8::Array> v8Array = v8::Local<v8::Array>::Cast(v8Value);
- for (size_t i = 0; i < v8Array->Length(); ++i) {
- v8::Local<v8::Value> indexedValue = v8Array->Get(v8::Integer::New(i));
- ret->append(v8ValueToWebCoreString(indexedValue));
- }
- return ret.release();
-}
-
-#if ENABLE(INDEXED_DATABASE)
-
-PassRefPtr<IDBKeyRange> OptionsObject::getKeyKeyRange(const String& key) const
-{
- v8::Local<v8::Value> v8Value;
- if (!getKey(key, v8Value))
- return 0;
-
- if (!V8IDBKeyRange::HasInstance(v8Value))
- return 0;
-
- return V8IDBKeyRange::toNative(v8::Handle<v8::Object>::Cast(v8Value));
-}
-
-#endif
-
-bool OptionsObject::getKey(const String& key, v8::Local<v8::Value>& value) const
-{
- if (isUndefinedOrNull())
return false;
- v8::Local<v8::Object> options = m_options->ToObject();
- ASSERT(!options.IsEmpty());
- v8::Handle<v8::String> v8Key = v8String(key);
- if (!options->Has(v8Key))
- return false;
- value = options->Get(v8Key);
- if (value.IsEmpty())
- return false;
+ value = ScriptValue(v8Value);
return true;
}
-bool OptionsObject::getKeyValue(const String& key, unsigned short& value) const
+bool OptionsObject::get(const String& key, unsigned short& value) const
{
v8::Local<v8::Value> v8Value;
if (!getKey(key, v8Value))
@@ -195,7 +157,7 @@
return true;
}
-bool OptionsObject::getKeyValue(const String& key, unsigned& value) const
+bool OptionsObject::get(const String& key, unsigned& value) const
{
v8::Local<v8::Value> v8Value;
if (!getKey(key, v8Value))
@@ -208,7 +170,7 @@
return true;
}
-bool OptionsObject::getKeyValue(const String& key, unsigned long long& value) const
+bool OptionsObject::get(const String& key, unsigned long long& value) const
{
v8::Local<v8::Value> v8Value;
if (!getKey(key, v8Value))
@@ -225,7 +187,7 @@
return true;
}
-bool OptionsObject::getKeyValue(const String& key, RefPtr<DOMWindow>& value) const
+bool OptionsObject::get(const String& key, RefPtr<DOMWindow>& value) const
{
v8::Local<v8::Value> v8Value;
if (!getKey(key, v8Value))
@@ -242,7 +204,7 @@
return true;
}
-bool OptionsObject::getKeyValue(const String& key, MessagePortArray& value) const
+bool OptionsObject::get(const String& key, MessagePortArray& value) const
{
v8::Local<v8::Value> v8Value;
if (!getKey(key, v8Value))
@@ -251,4 +213,18 @@
return getMessagePortArray(v8Value, value);
}
+bool OptionsObject::getWithUndefinedOrNullCheck(const String& key, String& value) const
+{
+ v8::Local<v8::Value> v8Value;
+ if (!getKey(key, v8Value) || v8Value->IsNull() || v8Value->IsUndefined())
+ return false;
+
+ // FIXME: It is possible for this to throw in which case we'd be getting back
+ // an empty string and returning true when we should be returning false.
+ // See fast/dom/Geolocation/script-tests/argument-types.js for a similar
+ // example.
+ value = WebCore::isUndefinedOrNull(v8Value) ? String() : v8ValueToWebCoreString(v8Value);
+ return true;
+}
+
} // namespace WebCore
Modified: trunk/Source/WebCore/bindings/v8/OptionsObject.h (98571 => 98572)
--- trunk/Source/WebCore/bindings/v8/OptionsObject.h 2011-10-27 14:51:32 UTC (rev 98571)
+++ trunk/Source/WebCore/bindings/v8/OptionsObject.h 2011-10-27 14:51:52 UTC (rev 98572)
@@ -46,44 +46,19 @@
OptionsObject& operator=(const OptionsObject&);
bool isUndefinedOrNull() const;
- bool getKeyBool(const String& key, bool& value) const;
- bool getKeyInt32(const String& key, int32_t& value) const;
- bool getKeyDouble(const String& key, double& value) const;
- bool getKeyString(const String& key, String& value) const;
- bool getKeyStringWithUndefinedOrNullCheck(const String& key, String& value) const;
- PassRefPtr<DOMStringList> getKeyDOMStringList(const String& key) const;
- PassRefPtr<IDBKeyRange> getKeyKeyRange(const String& key) const;
- bool getKeyValue(const String& key, bool& value) const
- {
- return getKeyBool(key, value);
- }
- bool getKeyValue(const String& key, int32_t& value) const
- {
- return getKeyInt32(key, value);
- }
- bool getKeyValue(const String& key, double& value) const
- {
- return getKeyDouble(key, value);
- }
- bool getKeyValue(const String& key, String& value) const
- {
- return getKeyString(key, value);
- }
- bool getKeyValue(const String& key, ScriptValue& value) const
- {
- v8::Local<v8::Value> v8Value;
- if (!getKey(key, v8Value))
- return false;
+ bool get(const String&, bool&) const;
+ bool get(const String&, int32_t&) const;
+ bool get(const String&, double&) const;
+ bool get(const String&, String&) const;
+ bool get(const String&, ScriptValue&) const;
+ bool get(const String&, unsigned short&) const;
+ bool get(const String&, unsigned&) const;
+ bool get(const String&, unsigned long long&) const;
+ bool get(const String&, RefPtr<DOMWindow>&) const;
+ bool get(const String&, MessagePortArray&) const;
- value = ScriptValue(v8Value);
- return true;
- }
- bool getKeyValue(const String&, unsigned short&) const;
- bool getKeyValue(const String&, unsigned&) const;
- bool getKeyValue(const String&, unsigned long long&) const;
- bool getKeyValue(const String& key, RefPtr<DOMWindow>& value) const;
- bool getKeyValue(const String& key, MessagePortArray& value) const;
+ bool getWithUndefinedOrNullCheck(const String&, String&) const;
private:
bool getKey(const String& key, v8::Local<v8::Value>&) const;
Modified: trunk/Source/WebCore/bindings/v8/custom/V8EventConstructors.cpp (98571 => 98572)
--- trunk/Source/WebCore/bindings/v8/custom/V8EventConstructors.cpp 2011-10-27 14:51:32 UTC (rev 98571)
+++ trunk/Source/WebCore/bindings/v8/custom/V8EventConstructors.cpp 2011-10-27 14:51:52 UTC (rev 98572)
@@ -117,7 +117,7 @@
return false;
#define FILL_PROPERTY(propertyName) \
- options.getKeyValue(#propertyName, eventInit.propertyName); // This can fail but it is OK.
+ options.get(#propertyName, eventInit.propertyName); // This can fail but it is OK.
INSTANTIATE_INITIALIZING_CONSTRUCTOR_FOR_EVENT(DICTIONARY_START, DICTIONARY_END, FILL_PARENT_PROPERTIES, FILL_PROPERTY)
INSTANTIATE_INITIALIZING_CONSTRUCTOR_FOR_CUSTOM_EVENT(DICTIONARY_START, DICTIONARY_END, FILL_PARENT_PROPERTIES, FILL_PROPERTY)
Modified: trunk/Source/WebCore/bindings/v8/custom/V8WebKitMutationObserverCustom.cpp (98571 => 98572)
--- trunk/Source/WebCore/bindings/v8/custom/V8WebKitMutationObserverCustom.cpp 2011-10-27 14:51:32 UTC (rev 98571)
+++ trunk/Source/WebCore/bindings/v8/custom/V8WebKitMutationObserverCustom.cpp 2011-10-27 14:51:52 UTC (rev 98572)
@@ -90,17 +90,17 @@
OptionsObject optionsObject(args[1]);
unsigned options = 0;
bool option;
- if (optionsObject.getKeyValue("childList", option) && option)
+ if (optionsObject.get("childList", option) && option)
options |= WebKitMutationObserver::ChildList;
- if (optionsObject.getKeyValue("attributes", option) && option)
+ if (optionsObject.get("attributes", option) && option)
options |= WebKitMutationObserver::Attributes;
- if (optionsObject.getKeyValue("characterData", option) && option)
+ if (optionsObject.get("characterData", option) && option)
options |= WebKitMutationObserver::CharacterData;
- if (optionsObject.getKeyValue("subtree", option) && option)
+ if (optionsObject.get("subtree", option) && option)
options |= WebKitMutationObserver::Subtree;
- if (optionsObject.getKeyValue("attributeOldValue", option) && option)
+ if (optionsObject.get("attributeOldValue", option) && option)
options |= WebKitMutationObserver::AttributeOldValue;
- if (optionsObject.getKeyValue("characterDataOldValue", option) && option)
+ if (optionsObject.get("characterDataOldValue", option) && option)
options |= WebKitMutationObserver::CharacterDataOldValue;
imp->observe(target, options);
Modified: trunk/Source/WebCore/storage/IDBDatabase.cpp (98571 => 98572)
--- trunk/Source/WebCore/storage/IDBDatabase.cpp 2011-10-27 14:51:32 UTC (rev 98571)
+++ trunk/Source/WebCore/storage/IDBDatabase.cpp 2011-10-27 14:51:52 UTC (rev 98572)
@@ -82,14 +82,14 @@
}
String keyPath;
- bool keyPathExists = options.getKeyStringWithUndefinedOrNullCheck("keyPath", keyPath);
+ bool keyPathExists = options.getWithUndefinedOrNullCheck("keyPath", keyPath);
if (keyPathExists && !IDBIsValidKeyPath(keyPath)) {
ec = IDBDatabaseException::NON_TRANSIENT_ERR;
return 0;
}
bool autoIncrement = false;
- options.getKeyBool("autoIncrement", autoIncrement);
+ options.get("autoIncrement", autoIncrement);
// FIXME: Look up evictable and pass that on as well.
RefPtr<IDBObjectStoreBackendInterface> objectStore = m_backend->createObjectStore(name, keyPath, autoIncrement, m_setVersionTransaction->backend(), ec);
Modified: trunk/Source/WebCore/storage/IDBObjectStore.cpp (98571 => 98572)
--- trunk/Source/WebCore/storage/IDBObjectStore.cpp 2011-10-27 14:51:32 UTC (rev 98571)
+++ trunk/Source/WebCore/storage/IDBObjectStore.cpp 2011-10-27 14:51:52 UTC (rev 98572)
@@ -156,7 +156,7 @@
}
bool unique = false;
- options.getKeyBool("unique", unique);
+ options.get("unique", unique);
RefPtr<IDBIndexBackendInterface> index = m_objectStore->createIndex(name, keyPath, unique, m_transaction->backend(), ec);
ASSERT(!index != !ec); // If we didn't get an index, we should have gotten an exception code. And vice versa.
