1470017 - Consider to not change prototype when adopting nodes

Status: RESOLVED FIXED

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

Description

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

https://www.w3.org/Bugs/Public/show_bug.cgi?id=20567

Comment 1

[Boris Zbarsky [:bzbarsky]]

We should consider doing some measurements on the memory aspect here somehow...

Comment 2

[Bobby Holley (:bholley)]

(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #1)
> We should consider doing some measurements on the memory aspect here
> somehow...

I already did, in [1].

So I'm not really in a hurry to do this. If we're going to do anything here, we might as well start with adopting Blink's model for document.open(), since the rationale in [2] is that we can drop transplanting from the spec if we do both of these things.

[1] https://www.w3.org/Bugs/Public/show_bug.cgi?id=20567#c56
[2] https://www.w3.org/Bugs/Public/show_bug.cgi?id=20567#c74

Comment 5

[BugBot [:suhaib / :marco]]

The severity field for this bug is set to S3. However, the following bug duplicate has higher severity:

• Bug 1821790: S2

:hsinyi, could you consider increasing the severity of this bug to S2?

For more information, please visit auto_nag documentation.

Comment 6

[Hsin-Yi Tsai (she/her) [:hsinyi]]

According to the know webcompat report, I am not quite sure if I see the rationale of Bug 1821790 being S2. Kagami, would you kindly answer this needinfo from comment 5 for me, as you have better ideas about this issue? Thank you.

Comment 7

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

I have no idea, S3 looks fine to me. I'll pass that question to mccr8 who set it.

Comment 8

[Andrew McCreight [:mccr8]]

I thought it was a regression, but this is apparently old behavior.

Comment 11

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1470017 - Don't reparent prototype chains on adopt. r=smaug!

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/4521a4f9d7fa
https://hg.mozilla.org/integration/autoland/rev/d6aba05c3f76
Don't reparent prototype chains on adopt. r=smaug,devtools-reviewers,jdescottes,webidl,nchevobbe

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/f58d86ae2499
https://hg.mozilla.org/integration/autoland/rev/8fb39fd0ea23
Revert "Bug 1470017 - Don't reparent prototype chains on adopt. r=smaug,devtools-reviewers,jdescottes,webidl,nchevobbe" for causing mochitests assertion failures in HTMLMediaElement.cpp.

Comment 14

[Serban Stanca [:SerbanS]]

Reverted this because it was causing mochitests assertion failures in HTMLMediaElement.cpp.

• Revert link
• Push with failures
• Failure Log
• Failure line: Assertion failure: mMediaStreamRenderer, at ./../../../../../checkouts/gecko/dom/media/mediaelement/HTMLMediaElement.cpp:X

Also please check these mochitests failures and these ones.

Comment 15

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1470017 - Use ownerDocGlobal where appropriate in child actors and remote/. r=#firefox-desktop-core-reviewers!

Child actors generally deal with content nodes, and those can be
adopted, in order to preserve behavior, switch to the new
.ownerDocGlobal.

Fix + rename the use-ownerGlobal lint since now it's wrong.
.ownerDocument.defaultView is .ownerDocGlobal, not .ownerGlobal now.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/3ca434ce61bd
https://hg.mozilla.org/integration/autoland/rev/551161093ac6
Don't reparent prototype chains on adopt. r=smaug,devtools-reviewers,jdescottes,webidl,nchevobbe

Comment 17

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/551161093ac6

Comment 19

[Phabricator Automation]

Comment on attachment 9570852 [details]
Bug 1470017 - Use ownerDocGlobal where appropriate in child actors and remote/. r=#firefox-desktop-core-reviewers!

Revision D294805 was moved to bug 2033242. Setting attachment 9570852 [details] to obsolete.

Comment 20

[Alex Finder|PTO, back on September 30th]

(In reply to Pulsebot from comment #16)

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/3ca434ce61bd
https://hg.mozilla.org/integration/autoland/rev/551161093ac6
Don't reparent prototype chains on adopt.
r=smaug,devtools-reviewers,jdescottes,webidl,nchevobbe

Perfherder has detected a browsertime performance change from push 551161093ac62a253604afcf21977f4ecbb6f5cc.

No action is required from the author; this comment is provided for informational purposes only.

Improvement	Test	Platform	Options	Absolute values [old vs new]	Performance Profiles
6%	google-slides LastVisualChange (doc)	linux2404-64-shippable	bytecode-cached fission warm webrender	2,306.46 ms -> 2,168.27 ms	Before/After

Need Help or Information?

If you have any questions, please reach out to [e-mail]. Alternatively, you can find help on Slack by joining #perf-help, and on Matrix you can find help by joining #perftest.

Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests.