1662840 - Move SetOverrideDPPX to WindowContext or BrowsingContext

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect)

Description

[Kris Maglione [:kmag]]

The functions which set these flags currently try to propagate them to descendants using nsDocumentViewer::PropagateToPresContextsHelper, which only handle in-process children. We want to handle out-of-process children too, which means we need to use store them on WindowContext or BrowsingContext instead, and propagate them using something like BrowsingContext::PreOrderWalk.

We should then remove PropagateToPresContextsHelper.

Note that this involves adding DidSet* hooks to BrowsingContext or WindowContext for each of these flags to update the layout state of the child process they live in when the flags change.

Comment 1

[Chris Peterson [:cpeterson]]

Moving to Remote Protocol::Emulation component

Tracking for Fission M7 Beta miletone

Comment 2

[Neha Kochar [:neha]]

Honza said this is not part of dt-fission-m3-mvp and is not considered Fission M7 blocker.

Comment 3

[Chris Peterson [:cpeterson]]

dt-fission-future don't need to block Fission MVP.

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Kris, I'm fairly sure that this bug is in the wrong component, and doesn't belong to Remote Protocol. Can you please check? Thanks.

Comment 5

[Kris Maglione [:kmag]]

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+1] from comment #4)

Kris, I'm fairly sure that this bug is in the wrong component, and doesn't belong to Remote Protocol. Can you please check? Thanks.

All of the users of these APIs are in the remote protocol emulation code, and I can't think of any other use cases for them, so I think the remote protocol essentially owns them. Regardless, the emulation code is going to need to change in order to fix this. We probably want to move the setters to BrowsingContext or WindowContext, and access them only from the parent process, which I don't think anyone on the Fission team knows enough about the remote protocol code to do.

Comment 6

[Chris Peterson [:cpeterson]]

Honza, this bug is a candidate for dt-fission-m3-mvp. The Fission team is currently tracking it as a blocker for our Fission Release experiment (milestone M8).

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Kris Maglione [:kmag] from comment #5)

All of the users of these APIs are in the remote protocol emulation code, and I can't think of any other use cases for them, so I think the remote protocol essentially owns them.

APIs in this context are SetOverrideDPPX, EmulateMediumInternal, and EmulatePrefersColorSchemeInternal?

I can at least see an instance of using docShell.contentViewer.overrideDPPX and I'm happy to move that to the browsing context if it is available now.

But there is nothing related to EmulateMediumInternal, and EmulatePrefersColorSchemeInternal in remote protocol. Do you have some links to show that actually show the relationship to remote protocol?

Regardless, the emulation code is going to need to change in order to fix this. We probably want to move the setters to BrowsingContext or WindowContext, and access them only from the parent process, which I don't think anyone on the Fission team knows enough about the remote protocol code to do.

Code that needs to run in the parent process and is related to emulation is part of /remote/domains/parent/Emulation.jsm. Some of the API end-points already use the browsing context to set overrides like for touch events, as long as those were available.

Comment 8

[Kris Maglione [:kmag]]

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+1] from comment #7)

But there is nothing related to EmulateMediumInternal, and EmulatePrefersColorSchemeInternal in remote protocol. Do you have some links to show that actually show the relationship to remote protocol?

Sorry, you're right. Those are used by devtools emulation code outside of remote protocol. I'll split them off into another bug.

Code that needs to run in the parent process and is related to emulation is part of /remote/domains/parent/Emulation.jsm. Some of the API end-points already use the browsing context to set overrides like for touch events, as long as those were available.

OK, one of us may be able to take it, then. But regardless, I think this bug fundamentally belongs to the remote protocol emulation component.

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I cannot see that overrideDPPX or anything related is available on the browsing context nor window context yet. As such there is nothing we can do on our side unless the Fission team moved that property from the content viewer over to one of these context classes. Kris, who could work on that?

Comment 10

[Kris Maglione [:kmag]]

Attachment: Bug 1662840: Move `overrideDPPX` from nsIContentViewer to BrowsingContext. r=emilio,whimboo,mtigley

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Thanks Kris for working on it. Looks like this bug is now mostly about DOM navigation changes.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a9bc92e5b775
Move `overrideDPPX` from nsIContentViewer to BrowsingContext. r=whimboo,remote-protocol-reviewers,mtigley,emilio

Comment 13

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/a9bc92e5b775