Title: [164125] trunk/Source/WebCore
- Revision
- 164125
- Author
- benja...@webkit.org
- Date
- 2014-02-14 13:00:19 -0800 (Fri, 14 Feb 2014)
Log Message
Clean up JSDOMStringMap::deleteProperty
https://bugs.webkit.org/show_bug.cgi?id=128801
Reviewed by Geoffrey Garen.
Follow up on my cleaning of JSDOMStringMapCustom, this time for deletion.
Previously, we would query for the name, then call deleteItem() if the name was
found. Instead, everything is moved to deleteItem which then return if the deletion
is successful or not.
By using convertPropertyNameToAttributeName, we can use Element::hasAttribute() directly
to find if the attribute exists or not. If it exists, we remove it.
Theoretically we could have a single pass over the attributes to find->delete but this
code is not hot enough to justify anything fancy at this point.
Finally, we no longer check for isValidPropertyName() on deletion. JSDOMStringMapCustom
was the last client of DatasetDOMStringMap::deleteItem() and it could not call deleteItem()
with invalid name since the name would have failed DatasetDOMStringMap::contains().
The spec does not specify any name checking either for deletion so we are safe just ignoring
invalid input.
* bindings/js/JSDOMStringMapCustom.cpp:
(WebCore::JSDOMStringMap::deleteProperty):
* dom/DatasetDOMStringMap.cpp:
(WebCore::DatasetDOMStringMap::deleteItem):
* dom/DatasetDOMStringMap.h:
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (164124 => 164125)
--- trunk/Source/WebCore/ChangeLog 2014-02-14 20:59:27 UTC (rev 164124)
+++ trunk/Source/WebCore/ChangeLog 2014-02-14 21:00:19 UTC (rev 164125)
@@ -1,3 +1,34 @@
+2014-02-14 Benjamin Poulain <benja...@webkit.org>
+
+ Clean up JSDOMStringMap::deleteProperty
+ https://bugs.webkit.org/show_bug.cgi?id=128801
+
+ Reviewed by Geoffrey Garen.
+
+ Follow up on my cleaning of JSDOMStringMapCustom, this time for deletion.
+
+ Previously, we would query for the name, then call deleteItem() if the name was
+ found. Instead, everything is moved to deleteItem which then return if the deletion
+ is successful or not.
+
+ By using convertPropertyNameToAttributeName, we can use Element::hasAttribute() directly
+ to find if the attribute exists or not. If it exists, we remove it.
+
+ Theoretically we could have a single pass over the attributes to find->delete but this
+ code is not hot enough to justify anything fancy at this point.
+
+ Finally, we no longer check for isValidPropertyName() on deletion. JSDOMStringMapCustom
+ was the last client of DatasetDOMStringMap::deleteItem() and it could not call deleteItem()
+ with invalid name since the name would have failed DatasetDOMStringMap::contains().
+ The spec does not specify any name checking either for deletion so we are safe just ignoring
+ invalid input.
+
+ * bindings/js/JSDOMStringMapCustom.cpp:
+ (WebCore::JSDOMStringMap::deleteProperty):
+ * dom/DatasetDOMStringMap.cpp:
+ (WebCore::DatasetDOMStringMap::deleteItem):
+ * dom/DatasetDOMStringMap.h:
+
2014-02-14 Benjamin Poulain <bpoul...@apple.com>
Improve the performance on mobile of FTPDirectoryDocument
Modified: trunk/Source/WebCore/bindings/js/JSDOMStringMapCustom.cpp (164124 => 164125)
--- trunk/Source/WebCore/bindings/js/JSDOMStringMapCustom.cpp 2014-02-14 20:59:27 UTC (rev 164124)
+++ trunk/Source/WebCore/bindings/js/JSDOMStringMapCustom.cpp 2014-02-14 21:00:19 UTC (rev 164125)
@@ -27,7 +27,6 @@
#include "JSDOMStringMap.h"
#include "DOMStringMap.h"
-#include "Element.h"
#include "JSNode.h"
#include <wtf/text/AtomicString.h>
@@ -58,16 +57,10 @@
Base::getOwnPropertyNames(thisObject, exec, propertyNames, mode);
}
-bool JSDOMStringMap::deleteProperty(JSCell* cell, ExecState* exec, PropertyName propertyName)
+bool JSDOMStringMap::deleteProperty(JSCell* cell, ExecState*, PropertyName propertyName)
{
JSDOMStringMap* thisObject = jsCast<JSDOMStringMap*>(cell);
- AtomicString stringName = propertyNameToAtomicString(propertyName);
- if (!thisObject->m_impl->contains(stringName))
- return false;
- ExceptionCode ec = 0;
- thisObject->m_impl->deleteItem(stringName, ec);
- setDOMException(exec, ec);
- return !ec;
+ return thisObject->m_impl->deleteItem(propertyNameToString(propertyName));
}
bool JSDOMStringMap::deletePropertyByIndex(JSCell* cell, ExecState* exec, unsigned index)
Modified: trunk/Source/WebCore/dom/DatasetDOMStringMap.cpp (164124 => 164125)
--- trunk/Source/WebCore/dom/DatasetDOMStringMap.cpp 2014-02-14 20:59:27 UTC (rev 164124)
+++ trunk/Source/WebCore/dom/DatasetDOMStringMap.cpp 2014-02-14 21:00:19 UTC (rev 164125)
@@ -191,19 +191,6 @@
return nullAtom;
}
-bool DatasetDOMStringMap::contains(const String& name)
-{
- if (!m_element.hasAttributes())
- return false;
-
- for (const Attribute& attribute : m_element.attributesIterator()) {
- if (propertyNameMatchesAttributeName(name, attribute.localName()))
- return true;
- }
-
- return false;
-}
-
void DatasetDOMStringMap::setItem(const String& name, const String& value, ExceptionCode& ec)
{
if (!isValidPropertyName(name)) {
@@ -214,14 +201,14 @@
m_element.setAttribute(convertPropertyNameToAttributeName(name), value, ec);
}
-void DatasetDOMStringMap::deleteItem(const String& name, ExceptionCode& ec)
+bool DatasetDOMStringMap::deleteItem(const String& name)
{
- if (!isValidPropertyName(name)) {
- ec = SYNTAX_ERR;
- return;
- }
+ AtomicString attributeName = convertPropertyNameToAttributeName(name);
+ if (!m_element.hasAttribute(attributeName))
+ return false;
- m_element.removeAttribute(convertPropertyNameToAttributeName(name));
+ m_element.removeAttribute(attributeName);
+ return true;
}
} // namespace WebCore
Modified: trunk/Source/WebCore/dom/DatasetDOMStringMap.h (164124 => 164125)
--- trunk/Source/WebCore/dom/DatasetDOMStringMap.h 2014-02-14 20:59:27 UTC (rev 164124)
+++ trunk/Source/WebCore/dom/DatasetDOMStringMap.h 2014-02-14 21:00:19 UTC (rev 164125)
@@ -49,9 +49,8 @@
void getNames(Vector<String>&);
const AtomicString& item(const String& name, bool& isValid);
- bool contains(const String& name);
void setItem(const String& name, const String& value, ExceptionCode&);
- void deleteItem(const String& name, ExceptionCode&);
+ bool deleteItem(const String& name);
Element* element() { return &m_element; }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
