1446246 - Common up the HTMLConstructor implementations

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement, P2)

Description

[Boris Zbarsky [:bzbarsky]]

We have a bunch of HTMLConstructor implementations that are all very similar.  The infrastructure from bug 1444286 lets us common them up into a single function.  This saves about 80KB of codesize, if I am measuring right.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Codegen output for the [HTMLConstructor]-generated constructor for HTMLDivElement

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: part 1. Use a single handwritten HTMLConstructor implementation, instead of code-generating lots of very similar implementations

The codegen changes are mostly a backout of the changes made in bug 1274159.

The HTMLConstructor implementation is mostly copied from one of the
code-generated ones, with a few modifications:

* Derive the interface name from the proto id instead of hardcoding it.
* Use the proto/constructor ids to get constructor and prototype objects.
* Use ErrorResult instead of FastErrorResult; we don't want the precedent of
  using FastErrorResult in non-generated code.

There will be further changes to combine HTMLConstructor and
CreateXULOrHTMLElement, in a separate changeset.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: part 2. Combine HTMLConstructor and CreateXULOrHTMLElement into a single function

This fixes an observable bug we had due to doing the steps in a different order
from the spec: the 'prototype' get can have side-effects so needs to happen
after some of the other sanity checks.

Comment 4

[Peter Van der Beken [:peterv]]

Comment on attachment 8959676 [details] [diff] [review]
part 2.  Combine HTMLConstructor and CreateXULOrHTMLElement into a single function

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

::: dom/bindings/BindingUtils.cpp
@@ +3748,5 @@
> +      }
> +    }
> +
> +    // Step 12.
> +    constructionStack.LastElement() = nullptr;

It might make things more understandable to use ALREADY_CONSTRUCTED_MARKER here?

Comment 5

[Boris Zbarsky [:bzbarsky]]

> It might make things more understandable to use ALREADY_CONSTRUCTED_MARKER here?

Yes, fixed.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b470762b4676
part 1.  Use a single handwritten HTMLConstructor implementation, instead of code-generating lots of very similar implementations. r=peterv
https://hg.mozilla.org/integration/mozilla-inbound/rev/9ed0718ede32
part 2.  Combine HTMLConstructor and CreateXULOrHTMLElement into a single function.  r=peterv

Comment 7

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/b470762b4676
https://hg.mozilla.org/mozilla-central/rev/9ed0718ede32

Comment 8

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/w3c/web-platform-tests/pull/10311 for changes under testing/web-platform/tests

Comment 9

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Can't merge web-platform-tests PR due to failing upstream checks:
Github PR https://github.com/w3c/web-platform-tests/pull/10311
* continuous-integration/travis-ci/pr (https://travis-ci.org/w3c/web-platform-tests/builds/362266178?utm_source=github_status&utm_medium=notification)

Comment 10

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream PR merged