824448 - Convert HTMLHeadingElement to WebIDL

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Boris Zbarsky [:bzbarsky]]

Dromaeo's dom-attr test uses an <h2>.

You can see a before/after at http://dromaeo.com/?id=186938,186941 (esp. the getAttribute and element.property lines).

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: part 1. Rename nsHTMLHeadingElement to mozilla::dom::HTMLHeadingElement.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: part 2. Switch HTMLHeadingElement to WebIDL bindings.

Comment 3

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 695425 [details] [diff] [review]
part 1.  Rename nsHTMLHeadingElement to mozilla::dom::HTMLHeadingElement.

Review of attachment 695425 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/html/content/src/nsHTMLHeadingElement.cpp
@@ +9,5 @@
>  #include "nsStyleConsts.h"
>  #include "nsMappedAttributes.h"
>  #include "nsRuleData.h"
>  #include "mozAutoDocUpdate.h"
> +#include "mozilla/dom/HTMLHeadingElement.h"

Include this first, to make sure the header doesn't depend on anything included before it.

::: content/html/content/src/HTMLHeadingElement.h
@@ +36,5 @@
> +
> +  virtual bool ParseAttribute(int32_t aNamespaceID,
> +                                nsIAtom* aAttribute,
> +                                const nsAString& aValue,
> +                                nsAttrValue& aResult);

Indentation

@@ +39,5 @@
> +                                const nsAString& aValue,
> +                                nsAttrValue& aResult);
> +  NS_IMETHOD_(bool) IsAttributeMapped(const nsIAtom* aAttribute) const;
> +  nsMapRuleToAttributesFunc GetAttributeMappingFunction() const;
> +  virtual nsresult Clone(nsINodeInfo *aNodeInfo, nsINode **aResult) const;

* to the left

Comment 4

[Boris Zbarsky [:bzbarsky]]

> Include this first, to make sure the header doesn't depend on anything included before
> it.

Eh.  OK.

Will fix the ParseAttribute indent.  No plans to "fix" the copied code.  ;)

Comment 5

[Peter Van der Beken [:peterv]]

Hmm, need to add include guards to HTMLHeadingElement.h.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Good catch.  Done.

Comment 7

[Boris Zbarsky [:bzbarsky]]

   https://hg.mozilla.org/integration/mozilla-inbound/rev/dc6168662099
   https://hg.mozilla.org/integration/mozilla-inbound/rev/f5638aa67e23

Comment 8

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix for resulting bustage; curses upon partial interfaces

Comment 9

[Boris Zbarsky [:bzbarsky]]

Pushed the test fix as https://hg.mozilla.org/integration/mozilla-inbound/rev/21865911745c pending review.

Comment 10

[Boris Zbarsky [:bzbarsky]]

As expected, this gave us wins on dom-attr, ranging from 5% to 17% depending on the build configuration.

Comment 11

[Daniel Holbert [:dholbert] (vacation until Jun 23)]

https://hg.mozilla.org/mozilla-central/rev/dc6168662099
https://hg.mozilla.org/mozilla-central/rev/f5638aa67e23
https://hg.mozilla.org/mozilla-central/rev/21865911745c