Title: [154586] trunk/Source/WebCore
- Revision
- 154586
- Author
- [email protected]
- Date
- 2013-08-25 23:05:05 -0700 (Sun, 25 Aug 2013)
Log Message
JSHTMLFormElement::canGetItemsForName needlessly allocates a Vector
https://bugs.webkit.org/show_bug.cgi?id=120277
Reviewed by Sam Weinig.
Added HTMLFormElement::hasNamedElement and used it in JSHTMLFormElement::canGetItemsForName.
This required fixing a bug in HTMLFormElement::getNamedElements that the first call to getNamedElements
after replacing an element A with another element B of the same name caused it to erroneously append A
to namedItems via the aliases mapping. Because getNamedElements used to be always called in pairs, this
wrong behavior was never visible to the Web. Fixed the bug by not adding the old element to namedItem
when namedItem's size is 1.
Also renamed m_elementAliases to m_pastNamesMap along with related member functions.
No new tests are added since there should be no Web exposed behavioral change.
* bindings/js/JSHTMLFormElementCustom.cpp:
(WebCore::JSHTMLFormElement::canGetItemsForName):
* html/HTMLFormElement.cpp:
(WebCore::HTMLFormElement::elementFromPastNamesMap):
(WebCore::HTMLFormElement::addElementToPastNamesMap):
(WebCore::HTMLFormElement::hasNamedElement):
(WebCore::HTMLFormElement::getNamedElements):
* html/HTMLFormElement.h:
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (154585 => 154586)
--- trunk/Source/WebCore/ChangeLog 2013-08-26 02:21:32 UTC (rev 154585)
+++ trunk/Source/WebCore/ChangeLog 2013-08-26 06:05:05 UTC (rev 154586)
@@ -1,3 +1,31 @@
+2013-08-25 Ryosuke Niwa <[email protected]>
+
+ JSHTMLFormElement::canGetItemsForName needlessly allocates a Vector
+ https://bugs.webkit.org/show_bug.cgi?id=120277
+
+ Reviewed by Sam Weinig.
+
+ Added HTMLFormElement::hasNamedElement and used it in JSHTMLFormElement::canGetItemsForName.
+
+ This required fixing a bug in HTMLFormElement::getNamedElements that the first call to getNamedElements
+ after replacing an element A with another element B of the same name caused it to erroneously append A
+ to namedItems via the aliases mapping. Because getNamedElements used to be always called in pairs, this
+ wrong behavior was never visible to the Web. Fixed the bug by not adding the old element to namedItem
+ when namedItem's size is 1.
+
+ Also renamed m_elementAliases to m_pastNamesMap along with related member functions.
+
+ No new tests are added since there should be no Web exposed behavioral change.
+
+ * bindings/js/JSHTMLFormElementCustom.cpp:
+ (WebCore::JSHTMLFormElement::canGetItemsForName):
+ * html/HTMLFormElement.cpp:
+ (WebCore::HTMLFormElement::elementFromPastNamesMap):
+ (WebCore::HTMLFormElement::addElementToPastNamesMap):
+ (WebCore::HTMLFormElement::hasNamedElement):
+ (WebCore::HTMLFormElement::getNamedElements):
+ * html/HTMLFormElement.h:
+
2013-08-25 Andreas Kling <[email protected]>
RenderLayerBacking::renderer() should return a reference.
Modified: trunk/Source/WebCore/bindings/js/JSHTMLFormElementCustom.cpp (154585 => 154586)
--- trunk/Source/WebCore/bindings/js/JSHTMLFormElementCustom.cpp 2013-08-26 02:21:32 UTC (rev 154585)
+++ trunk/Source/WebCore/bindings/js/JSHTMLFormElementCustom.cpp 2013-08-26 06:05:05 UTC (rev 154586)
@@ -39,9 +39,7 @@
bool JSHTMLFormElement::canGetItemsForName(ExecState*, HTMLFormElement* form, PropertyName propertyName)
{
- Vector<RefPtr<Node> > namedItems;
- form->getNamedElements(propertyNameToAtomicString(propertyName), namedItems);
- return namedItems.size();
+ return form->hasNamedElement(propertyNameToAtomicString(propertyName));
}
JSValue JSHTMLFormElement::nameGetter(ExecState* exec, JSValue slotBase, PropertyName propertyName)
Modified: trunk/Source/WebCore/html/HTMLFormElement.cpp (154585 => 154586)
--- trunk/Source/WebCore/html/HTMLFormElement.cpp 2013-08-26 02:21:32 UTC (rev 154585)
+++ trunk/Source/WebCore/html/HTMLFormElement.cpp 2013-08-26 06:05:05 UTC (rev 154586)
@@ -611,36 +611,38 @@
return hasInvalidControls;
}
-HTMLFormControlElement* HTMLFormElement::elementForAlias(const AtomicString& alias)
+HTMLFormControlElement* HTMLFormElement::elementFromPastNamesMap(const AtomicString& pastName) const
{
- if (alias.isEmpty() || !m_elementAliases)
+ if (pastName.isEmpty() || !m_pastNamesMap)
return 0;
- return m_elementAliases->get(alias.impl());
+ return m_pastNamesMap->get(pastName.impl());
}
-void HTMLFormElement::addElementAlias(HTMLFormControlElement* element, const AtomicString& alias)
+void HTMLFormElement::addElementToPastNamesMap(HTMLFormControlElement* element, const AtomicString& pastName)
{
- if (alias.isEmpty())
+ if (pastName.isEmpty())
return;
- if (!m_elementAliases)
- m_elementAliases = adoptPtr(new AliasMap);
- m_elementAliases->set(alias.impl(), element);
+ if (!m_pastNamesMap)
+ m_pastNamesMap = adoptPtr(new PastNamesMap);
+ m_pastNamesMap->set(pastName.impl(), element);
}
+bool HTMLFormElement::hasNamedElement(const AtomicString& name)
+{
+ return elements()->hasNamedItem(name) || elementFromPastNamesMap(name);
+}
+
void HTMLFormElement::getNamedElements(const AtomicString& name, Vector<RefPtr<Node> >& namedItems)
{
+ // http://www.whatwg.org/specs/web-apps/current-work/multipage/forms.html#dom-form-nameditem
elements()->namedItems(name, namedItems);
- HTMLFormControlElement* aliasElement = elementForAlias(name);
- if (aliasElement) {
- if (namedItems.find(aliasElement) == notFound) {
- // We have seen it before but it is gone now. Still, we need to return it.
- // FIXME: The above comment is not clear enough; it does not say why we need to do this.
- namedItems.append(aliasElement);
- }
- }
- if (namedItems.size() && namedItems.first() != aliasElement)
- addElementAlias(static_cast<HTMLFormControlElement*>(namedItems.first().get()), name);
+ // FIXME: The specification says we should not add the element from the past when names map when namedItems is not empty.
+ HTMLFormControlElement* elementFromPast = elementFromPastNamesMap(name);
+ if (namedItems.size() && namedItems.first() != elementFromPast)
+ addElementToPastNamesMap(static_cast<HTMLFormControlElement*>(namedItems.first().get()), name);
+ else if (elementFromPast && namedItems.find(elementFromPast) == notFound)
+ namedItems.append(elementFromPast);
}
void HTMLFormElement::documentDidResumeFromPageCache()
Modified: trunk/Source/WebCore/html/HTMLFormElement.h (154585 => 154586)
--- trunk/Source/WebCore/html/HTMLFormElement.h 2013-08-26 02:21:32 UTC (rev 154585)
+++ trunk/Source/WebCore/html/HTMLFormElement.h 2013-08-26 06:05:05 UTC (rev 154586)
@@ -47,6 +47,7 @@
virtual ~HTMLFormElement();
PassRefPtr<HTMLCollection> elements();
+ bool hasNamedElement(const AtomicString&);
void getNamedElements(const AtomicString&, Vector<RefPtr<Node> >&);
unsigned length() const;
@@ -98,9 +99,6 @@
bool checkValidity();
- HTMLFormControlElement* elementForAlias(const AtomicString&);
- void addElementAlias(HTMLFormControlElement*, const AtomicString& alias);
-
CheckedRadioButtons& checkedRadioButtons() { return m_checkedRadioButtons; }
const Vector<FormAssociatedElement*>& associatedElements() const { return m_associatedElements; }
@@ -140,10 +138,14 @@
// are any invalid controls in this form.
bool checkInvalidControlsAndCollectUnhandled(Vector<RefPtr<FormAssociatedElement> >&);
- typedef HashMap<RefPtr<AtomicStringImpl>, RefPtr<HTMLFormControlElement> > AliasMap;
+ HTMLFormControlElement* elementFromPastNamesMap(const AtomicString&) const;
+ void addElementToPastNamesMap(HTMLFormControlElement*, const AtomicString& pastName);
+ // FIXME: This can leak HTMLFormControlElements.
+ typedef HashMap<RefPtr<AtomicStringImpl>, RefPtr<HTMLFormControlElement> > PastNamesMap;
+
FormSubmission::Attributes m_attributes;
- OwnPtr<AliasMap> m_elementAliases;
+ OwnPtr<PastNamesMap> m_pastNamesMap;
CheckedRadioButtons m_checkedRadioButtons;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
