Closed Bug 1505916 Opened 6 years ago Closed 5 years ago

[Fission] Make the fullscreen code Fission-aware

Categories

(Firefox :: General, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
Firefox 71
Fission Milestone M4
Tracking Status
firefox71 --- fixed

People

(Reporter: Felipe, Assigned: ablayelyfondou)

References

Details

Attachments

(2 files)

I saw some failures related to fullscreen on a try run using browser.fission.simulate = true, and also by inspection of the DOMFullscreenChild.jsm code we can see that it is handling fullscreen events from all frames, so it will need to be adapted to handle oop frames
Is there a more specific component for this issue? I don't have the necessary knowledge to set it myself.
Thanks!
Flags: needinfo?(felipc)
I think we can put it in Core::DOM
Component: General → DOM
Flags: needinfo?(felipc)
Product: Firefox → Core
Fission Milestone: --- → Future
Component: DOM → DOM: Core & HTML
Fission Milestone: Future → M4
Component: DOM: Core & HTML → XUL Widgets
Product: Core → Toolkit
Assignee: nobody → ablayelyfondou
Depends on: 1561420
No longer depends on: 1561420
Attachment #9072170 - Attachment description: Bug 1505916 - [Fission] Make the fullscreen code Fission-aware. r?NeilDeakin → Bug 1505916 - [Fission] Make the fullscreen code Fission-aware. r?NeilDeakin,smaug
Attachment #9072170 - Attachment description: Bug 1505916 - [Fission] Make the fullscreen code Fission-aware. r?NeilDeakin,smaug → Bug 1505916 - [Fission] Make the fullscreen code Fission-aware. r?NeilDeakin,smaug,dao

Hi Abdoulaye, were you still working on this?

Flags: needinfo?(ablayelyfondou)

Hi Mike. Yes, I am.

Flags: needinfo?(ablayelyfondou)
Attachment #9072170 - Attachment description: Bug 1505916 - [Fission] Make the fullscreen code Fission-aware. r?NeilDeakin,smaug,dao → Bug 1505916 - [Fission] Part 1: Make the fullscreen code work with JSWindowActors. r?NeilDeakin,smaug,dao

e10s scenario:

  1. A DOM element request fullscreen mode.
  2. The request is redirected to the parent.
  3. Parent enters fullscreen.
  4. Parent notifies child that it has finished entering fullscreen.
  5. Child goes fullscreen.
  6. Then, child notifies parent that it has finished transitioning to fullscreen.
  7. Finally, parent notifies observers that fullscreen paint has finished.

Let's go into the details of how step 5 works in the above scenario.
5.a The element that made the request is set to fullscreen.
5.b Then, the document where that element lives is set to fullscreen as well as all of its ancestors until we reach the top-level document. (see Document::ApplyFulscreen method)

Now in Fission world, we may have a request coming from an oop iframe. And it that case since we won't be able to directly access to ancestor documents living in different content processes anymore, we will first notify those content processes (one after the other from bottom to top) to go fullscreen. Once they all do, the content process where the request originated from will be told to enter fullscreen. And then the process continues to step 6...

Attachment #9092954 - Attachment description: Bug 1505916 - [Fission] Part 2: Make the fullscreen code work with oop iframes. → Bug 1505916 - [Fission] Part 2: Make the fullscreen code work with oop iframes. r?NeilDeakin,smaug,dao

Dao and smaug, please review the patches.

Flags: needinfo?(dao+bmo)
Flags: needinfo?(bugs)

No need to needinfo when the patch is in review queue.

Flags: needinfo?(bugs)
Blocks: 1585074

Hey Abdoulaye - I notice you've got prior approval on the reviews for Part 1, and just got approval on Part 2 yesterday. Is this pretty close to being landable?

Flags: needinfo?(ablayelyfondou)

Yes it is, I am just waiting for dao approval here.

Flags: needinfo?(ablayelyfondou)
Flags: needinfo?(dao+bmo)
Flags: needinfo?(dao+bmo)
Flags: needinfo?(dao+bmo)
Flags: needinfo?(dao+bmo)
Blocks: 1587521

Are these patches ready to land?

Flags: needinfo?(ablayelyfondou)

I conferred with Abdoulaye outside of Bugzilla - it sounds like he's sorted things out with Dao in Phabricator, and try looks okay (https://treeherder.mozilla.org/#/jobs?repo=try&revision=18dd6d6f49252a373d35cb12f7d53e86d102a540), so going to try to land these now.

Flags: needinfo?(dao+bmo)
Flags: needinfo?(ablayelyfondou)
Pushed by mconley@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8b47aa47a640
[Fission] Part 1: Make the fullscreen code work with JSWindowActors. r=NeilDeakin,dao,smaug
https://hg.mozilla.org/integration/autoland/rev/fcaa0242c204
[Fission] Part 2: Make the fullscreen code work with oop iframes. r=NeilDeakin,smaug
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla71
Regressions: 1588386
QA Contact: vlad.lucaci
Regressions: 1589295
Component: XUL Widgets → General
Product: Toolkit → Firefox
Target Milestone: mozilla71 → Firefox 71
No longer regressions: 1589295
Regressions: 1590138
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: