Remove created callback for custom elements (deprecated in v1)

RESOLVED FIXED in Firefox 59

Status

()

Core
DOM
P2
normal
RESOLVED FIXED
11 months ago
8 months ago

People

(Reporter: jessica, Assigned: edgar)

Tracking

Trunk
mozilla59
Points:
---

Firefox Tracking Flags

(firefox57 wontfix, firefox58 wontfix, firefox59 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

Comment hidden (empty)

Updated

11 months ago
Blocks: 1396765
No longer blocks: 889230
Priority: -- → P2
(Assignee)

Updated

10 months ago
Assignee: nobody → echen
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

8 months ago
Attachment #8928363 - Flags: feedback?(jdai)
(Assignee)

Updated

8 months ago
Attachment #8928364 - Flags: feedback?(jdai)

Comment 6

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

https://reviewboard.mozilla.org/r/199570/#review204774
Attachment #8928363 - Flags: review+

Comment 7

8 months ago
mozreview-review
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

Updated

8 months ago
Attachment #8928363 - Flags: feedback?(jdai) → feedback+

Updated

8 months ago
Attachment #8928363 - Flags: review+

Updated

8 months ago
Attachment #8928364 - Flags: feedback?(jdai)
(Assignee)

Comment 8

8 months ago
mozreview-review
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.
(Assignee)

Comment 9

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

Per comment #8.
Attachment #8928364 - Flags: feedback?(jdai)

Updated

8 months ago
Attachment #8928364 - Flags: feedback?(jdai) → feedback+
(Assignee)

Updated

8 months ago
Attachment #8928363 - Flags: review?(bugs)
Attachment #8928364 - Flags: review?(bugs)

Comment 10

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

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

rs+
Attachment #8928363 - Flags: review?(bugs) → review+

Comment 11

8 months ago
mozreview-review
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
Attachment #8928364 - Flags: review?(bugs) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 14

8 months ago
Pushed by echen@mozilla.com:
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 hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 21

8 months ago
Pushed by echen@mozilla.com:
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

8 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/1d0330d6d96f
https://hg.mozilla.org/mozilla-central/rev/010494388349
Status: NEW → RESOLVED
Last Resolved: 8 months ago
status-firefox59: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Mark 57/58 won't fix. Let it ride the train.
status-firefox57: affected → wontfix
status-firefox58: --- → wontfix
You need to log in before you can comment on or make changes to this bug.