1396620 - Remove created callback for custom elements (deprecated in v1)

Status: RESOLVED FIXED

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

Comment 1

[Edgar Chen [:edgar]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=80035a46f6932d5e41c9dd4c07d2fcf2f2a2f54f

Comment 2

[Edgar Chen [:edgar]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a20e9edd0e83ebf929a4b0cac1b41a7db4bf1fda&filter-tier=1&group_state=expanded

Comment 3

[Edgar Chen [:edgar]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a047b964b4a92db7c50d4330ee82ed885222bcf5&filter-tier=1&group_state=expanded

Comment 4

[Edgar Chen [:edgar]]

Attachment: Bug 1396620 - Part 1: Remove created callback for custom elements;

Review commit: https://reviewboard.mozilla.org/r/199570/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/199570/

Comment 5

[Edgar Chen [:edgar]]

Attachment: Bug 1396620 - Part 2: Fix compartment mismatch crash when doing old prototype swizzling for custom element;

Review commit: https://reviewboard.mozilla.org/r/199572/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/199572/

Comment 6

[John Dai[:jdai]]

Comment on attachment 8928363 [details]
Bug 1396620 - Part 1: Remove created callback for custom elements;

https://reviewboard.mozilla.org/r/199570/#review204774

Comment 7

[John Dai[:jdai]]

Comment on attachment 8928364 [details]
Bug 1396620 - Part 2: Fix compartment mismatch crash when doing old prototype swizzling for custom element;

https://reviewboard.mozilla.org/r/199572/#review204762

::: dom/base/Element.cpp:527
(Diff revision 1)
> +        // The custom element prototype could be in different compartment.
> +        if (!JS_WrapObject(aCx, &customProto)) {
> +          return nullptr;
> +        }

I guess this patch came from bug 1408828 comment 12[1]. Is this case still can happened in this bug? We won't enqueue an additional connectedCallback in BindToTree() which will get fired before createdCallback, right? Maybe I miss something...

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1408828#c12

Comment 8

[Edgar Chen [:edgar]]

Comment on attachment 8928364 [details]
Bug 1396620 - Part 2: Fix compartment mismatch crash when doing old prototype swizzling for custom element;

https://reviewboard.mozilla.org/r/199572/#review204796

::: dom/base/Element.cpp:527
(Diff revision 1)
> +        // The custom element prototype could be in different compartment.
> +        if (!JS_WrapObject(aCx, &customProto)) {
> +          return nullptr;
> +        }

Yeah, we still need this, though the prototype swizzling here might eventually be removed in bug 1416999.

We hit the compartment mismatch in https://searchfox.org/mozilla-central/rev/a662f122c37704456457a526af90db4e3c0fd10e/js/xpconnect/tests/mochitest/test_bug1094930.html.

Originally the Element::WrapObject happens during invoking createdCallback, which is invoked synchronously during `new f();`, and we are in the same compartment of `customProto`, a.k.a, the iframe one.

However, in part 1, we change test to use connectedCallback, the Element::WrapObject now happens during invoking connectedCallback, which is invoked during processing ElementQueue for `appendChild` and we are in the entry global's compartment, a.k.a. main window one. Then we hit the compartment mismatch in this case.

Comment 9

[Edgar Chen [:edgar]]

Comment on attachment 8928364 [details]
Bug 1396620 - Part 2: Fix compartment mismatch crash when doing old prototype swizzling for custom element;

Per comment #8.

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8928363 [details]
Bug 1396620 - Part 1: Remove created callback for custom elements;

https://reviewboard.mozilla.org/r/199570/#review205412

rs+

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8928364 [details]
Bug 1396620 - Part 2: Fix compartment mismatch crash when doing old prototype swizzling for custom element;

https://reviewboard.mozilla.org/r/199572/#review205416

Comment 12

[Edgar Chen [:edgar]]

Comment on attachment 8928363 [details]
Bug 1396620 - Part 1: Remove created callback for custom elements;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/199570/diff/1-2/

Comment 13

[Edgar Chen [:edgar]]

Comment on attachment 8928364 [details]
Bug 1396620 - Part 2: Fix compartment mismatch crash when doing old prototype swizzling for custom element;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/199572/diff/1-2/

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ed0cc9f86583
Part 1: Remove created callback for custom elements; r=jdai,smaug
https://hg.mozilla.org/integration/autoland/rev/b6b642efbb74
Part 2: Fix compartment mismatch crash when doing old prototype swizzling for custom element; r=smaug

Comment 15

[Arthur Iakab [arthur_iakab]]

Backed out changeset ed0cc9f86583::b6b642efbb74 (bug 1396620) for static failure at dom/base/CustomElementRegistry.cpp r=backout on a CLOSED TREE

This is the log of the failure https://treeherder.mozilla.org/logviewer.html#?job_id=145397725&repo=autoland&lineNumber=23154

Push failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=b6b642efbb746ef4b132a46bb9b85b155fd75115&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable

Backout: https://hg.mozilla.org/integration/autoland/rev/c8df3c4848c5b6cd2465e004ca985b4c4d5db7e9

Comment 16

[Edgar Chen [:edgar]]

Comment on attachment 8928363 [details]
Bug 1396620 - Part 1: Remove created callback for custom elements;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/199570/diff/2-3/

Comment 17

[Edgar Chen [:edgar]]

Comment on attachment 8928364 [details]
Bug 1396620 - Part 2: Fix compartment mismatch crash when doing old prototype swizzling for custom element;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/199572/diff/2-3/

Comment 18

[Edgar Chen [:edgar]]

Full try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2656e5802cbf3e516129d855c7a078525c335c08&filter-tier=1&group_state=expanded

Comment 19

[Edgar Chen [:edgar]]

Comment on attachment 8928363 [details]
Bug 1396620 - Part 1: Remove created callback for custom elements;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/199570/diff/3-4/

Comment 20

[Edgar Chen [:edgar]]

Comment on attachment 8928364 [details]
Bug 1396620 - Part 2: Fix compartment mismatch crash when doing old prototype swizzling for custom element;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/199572/diff/3-4/

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1d0330d6d96f
Part 1: Remove created callback for custom elements; r=jdai,smaug
https://hg.mozilla.org/integration/autoland/rev/010494388349
Part 2: Fix compartment mismatch crash when doing old prototype swizzling for custom element; r=smaug

Comment 22

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/1d0330d6d96f
https://hg.mozilla.org/mozilla-central/rev/010494388349

Comment 23

[Gerry Chang [:gchang]]

Mark 57/58 won't fix. Let it ride the train.