VOOZH about

URL: https://bugzilla.mozilla.org/show_bug.cgi?id=1913666

⇱ 1913666 - Implement new Fullscreen API Architecture


Open Bug 1913666 Opened 1 year ago Updated 1 month ago

Implement new Fullscreen API Architecture

Implement new Fullscreen API Architecture
Core
DOM: Core & HTML
unspecified
Unspecified
Unspecified
task
Points:
---
ASSIGNED
ASSIGNED
---
Iteration:
---
a11y-review
Accessibility Severity
Performance Impact
Size Estimate
Webcompat Priority
Webcompat Score
Tracking Status
relnote-firefox
thunderbird_esr115
thunderbird_esr140
firefox-esr115
firefox-esr140
firefox-esr153
firefox152
firefox153
firefox154
---
QA Whiteboard:
---
Has STR:
---
Change Request:
---
Bug Flags:
Signature:
None
This bug is publicly visible.

 
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review

Move JSWindowActor implementation of Fullscreen to C++

Assignee

Updated

β€’
1 year ago
See Also: β†’ CVE-2025-1018, fullscreen-api
Assignee

Updated

β€’
1 year ago
See Also: β†’ 1621736
Assignee

Updated

β€’
1 year ago
See Also: β†’ CVE-2024-11698
Assignee

Updated

β€’
1 year ago
Blocks: 1910456
Attachment #9421750 - Attachment description: WIP: Bug 1913666 - Implement new Fullscreen API Architecture β†’ Bug 1913666 - Implement new Fullscreen API Architecture r=sefeng!
Attachment #9421750 - Attachment description: Bug 1913666 - Implement new Fullscreen API Architecture r=sefeng! β†’ Bug 1913666 - Part 1: Introduce Fullscreen Service r=sefeng!
Assignee

Comment 2

β€’
1 year ago

This patch introduces the FullscreenManager. A fullscreen manager
manages the queue of requests that corresponds to 1 app window (not a tab).
Each queue is processed in order (and only ever 1 at-a-time). This is possible
due to how the spec has a synchronous and "in parallell" block for both
entry and exit requests, thus, the synchronous parts of the algorithm can
run before posting a request to the Fullscreen service - a request
that does the remainder of the work necessary.

It also introduces the base class for all fullscreen service requests
FullscreenServiceRequestBase.

FullscreenServiceTimerObserver is responsible
for cancelling fullscreen API requests that may or may not have hung in their respective
content processess. We have several mochi tests that more or less test this scenario
(javascript code that while trues in some background window/tab where we need to be
able to terminate). This timer only affects the fullscreen service requests
that involve DOM Fullscreen.

This patch does not introduce any Fullscreen API request, but only
the BrowserRequest which is a request that processes things like
go chrome fullscreen using F11 (or cmd+shift+f on Mac), i.e. non-DOM fullscreen.
BrowserRequest as such never enters DOM fullscreen but it can make
a window exit out of DOM fullscreen. Being a non-DOM fullscreen request, it is
not cancellable.

Assignee

Comment 3

β€’
1 year ago

This patch introduces the UserEnterFullscreen and SystemEnterFullscreen
requests which services a request to go DOM Fullscreen (or just simply add
a new element to an already fullscreened document's top layer).

We repurpose some of the previous implementation for configuring state
in the Document objects, so this logic really doesn't differ all that much except
that the control flow has been lifted out (since that is now FullscreenService's
responsibilities). So before, something like ApplyFullscreen would apply fs,
but also instruct the algorithm to keep going. Now, ApplyFullscreenState just
"does the thing".

Fullscreen API requests has a "in content process-representation" as well as the "service request"
and these are keyed by a unique monotonic id (uint64_t), these ID's are also used in the IPC
messaging scheme. The in-content-process data contains things like the document
where the request originated from, the element and the Javascript promise that was
handed out to the user. FullscreenServiceChange is the base class for these types
and in this patch, FullscreenServiceRequest is introduced for fullscreen-entry.

Assignee

Comment 4

β€’
1 year ago

This patch introduces UserExitFullscreen and SystemExitFullscreen
requests which services a request to perform the in-parallell parts of
the document.exitFullscreen() algorithm, for non-trusted as well as "chrome js"
javascript.

FullscreenServiceExit is introduced as the "in-content-process" representation
of a Fullscreen API exit-request.

Assignee

Comment 5

β€’
1 year ago

This patch introduces all the logic that involves the cancellation of
fullscreen requests (defined as "user agent deems it necessary to end
fullscreen session" in the spec). This is (probably, at least I think so)
the most complex part because there's quite a bit of edge cases that
needs handling.

Cancelling requests happen when we unload pages (or on page hide rather),
switch tabs (if the tab is in DOM Fullscreen) etc. So it needs to both
exit DOM fullscreen but also cancel any requests made by "that tab"
and it needs to be sure that the state we eventually end up in, is deterministic.

Assignee

Comment 6

β€’
1 year ago

This patch introduces the front end changes that needs to happen with this
new Fullscreen Architecture. One thing to note is that fullscreen-painted
might seem like it has something to do with painting, but that's not how this
notification message is actually used, it's just used as a sort-of "hey
we're done with the transition"-message. In the previous patches of this series
we can see this message be "simulated". This is (possibly) confusing, because we're not
actually waiting for a MozAfterPaint anywhere. Because the front ends,
both on desktop and android relies on seeing this message, we therefore
fire it at the end of a "Fullscreen Service request".

Depends on: 1943086

Does this mean 1943086 blocks this patch series?

The bug is specifically Fenix related and there's a pref that is set to off (default). Should I add a secondary issue, that depends on this issue 1913666, that aims to flip the pref on by default and have that one depend on 1943086?

Flags: needinfo?(echen)

(In reply to Simon Farre from comment #7)

Does this mean 1943086 blocks this patch series?

The bug is specifically Fenix related and there's a pref that is set to off (default). Should I add a secondary issue, that depends on this issue 1913666, that aims to flip the pref on by default and have that one depend on 1943086?

If we are not going to flip the perf here, yes, file a new bug which is for flipping the pref and having that one depend on bug 1943086 sounds good.

Flags: needinfo?(echen)
Assignee

Comment 9

β€’
1 year ago

This patch aims to re-use parts of what existed in
nsDOMWindowUtils.cpp, particularly OldWindowSize and
PrepareForFullscreenChange that tries to set the window dimensions in
advance.

I'm not entirely convinced yet, that the old code does what it think it
does, in a fission-like setting. Regardless, it will carry over to the
new architecture, so whatever it means to do, it should do here as well.

This also caches the window size and rotation, so that the old rotation
can be queried, which is relevant for things like Android, where we may
want to restore to landscape or portrait depending on what our original
rotation was.

Attachment #9433040 - Attachment description: Bug 1913666 - Part 3: Add Fullscreen API Service Requests (Enter) r=sefeng! β†’ Bug 1913666 - Part 3: Add Fullscreen API Service Requests (Enter) r=edgar!
Attachment #9433041 - Attachment description: Bug 1913666 - Part 4: Add Fullscreen API Service Requests (Exit) r=sefeng! β†’ Bug 1913666 - Part 4: Add Fullscreen API Service Requests (Exit) r=edgar!
Attachment #9433042 - Attachment description: Bug 1913666 - Part 5: Add cancelability to requests r=sefeng! β†’ Bug 1913666 - Part 5: Add cancelability to requests r=edgar!
Attachment #9433043 - Attachment description: Bug 1913666 - Part 6: Frontend additions to make them work with new arch r=sefeng! β†’ Bug 1913666 - Part 6: Frontend additions to make them work with new arch r=edgar!
Attachment #9466346 - Attachment description: Bug 1913666 - Part 7: Re-use of previous 'prepare fullscreen change' behavior r=sefeng! β†’ Bug 1913666 - Part 7: Re-use of previous 'prepare fullscreen change' behavior r=edgar!
Assignee

Comment 10

β€’
1 month ago

Introduces the fullscreen service which is just a skeleton implementation in this patch. In later patches
the fullscreen service will manage a collection of "fullscreen managers" which control
fullscreen requests on a per-app-window basis, since Firefox allows multiple windows
to go DOM fullscreen, but only 1 tab may be in DOM Fullscreen per window.

Assignee

Comment 11

β€’
1 month ago

Re-factored FullscreenRequest and Element::RequestFullscreen to not so
explicitly depend on FullscreenRequest : FullscreenChange, by having the
reject function be a public static method callable by anyone. The
fullscreen element ready check also gets a similar refactor so that the
logic specifically related to the algorithm can be directly re-used
later on.

Then in coming patches the success-path can be split up easily to either
be FullscreenService variant or the old JSWA variant.

Assignee

Comment 12

β€’
1 month ago

Same as with part 2, factor out spec algorithm parts so that they don't
get polluted by JSWA logic.

Assignee

Comment 13

β€’
1 month ago

Previously existed a browser-fullScreenAndPointerLock.js <-> DOMFullscreenParent.sys.mjs dependency.

This patch intends to split that and make it only uni-directional, so that reuse in browser-fullScreenAndPointerLock.js can be 100% for the coming Fullscreen IPC architectural rework.

This means things in browser-fullScreenAndPointerLock.js that involves themselves with caching actors, or walking actors or any logic involving IPC at all, should be removed and hoisted out into DOMFullscreenParent.sys.mjs instead.

This comes with some trickyness. browser-fullScreenAndPointerLock.js has direct access to window and document and because of that, these arguments needs to be passed into the functions. DOMFullscreenParent.sys.mjs does this in places already (see updateFullscreenWindowReference for example).

Assignee

Comment 14

β€’
1 month ago

The fullscreen-painted observer topic is named after its original implementation detail which is a paint observer inside FullscreenTransitionTask. But every external consumer (site-permission panel, WebAuthn prompt helper, MacTouchBar, test harnesses) seem to use it as a "fullscreen transition has settled" barrier. The "painted" guarantee is already best-effort: FullscreenTransitionTask falls back to a 1s timeout (full-screen-api.transition.timeout) and continues regardless. Over time the topic also acquired four manual re-broadcast sites (DOMFullscreenChild on MozAfterPaint, two paths in DOMFullscreenParent, and pictureinpicture/player.js against the PiP window) with subtly different triggers.

This patch does not rename or remove the topic, but the redesign collapses the redundant emit sites and removes the dead content-process broadcast and its parent→child Painted IPC loopback; with hopefully the result being that the Fullscreen IPC architecture rework, will out of the box provide the same side-effects in the UI, as the JSWA implementation does.

In the future, we may want to audit this and possibly remove it entirely.

Assignee

Comment 15

β€’
1 month ago

Like previous patches, this removes bi-directional JSWA dependencies, and separates work that involves the Fullscreen API from our current IPC architecture, to (attempt) to ensure parity in behavior for new architecture.

Assignee

Comment 17

β€’
1 month ago

This IPC message targets PBrowser that contains a subrange of documents that shall enter "remote frame fullscreen" (i.e. have iframe(s) enter fullscreen).

This can be sent in parallel, if we have origins A, B, C and C is entering fullscreen, we can target A and B with this IPC message, since they can't observe anything from each other.

The functionality for C entering/committing the fullscreen transaction and resolving the JS promise is in follow up patch.

Assignee

Comment 18

β€’
1 month ago

RestorePreviousFullscreenState runs the exit algorithm steps per Fullscreen API. Will be used by new Fullscreen IPC architecture.

Also exposed nsGlobalWindowOuter::FullscreenRequest, so that nsGlobalWindowOuter can be queried about whether or not it has an ongoing fullscreen transition. Will be useful to new Fullscreen Service as it can determine when it's safe to start a new request.

Assignee

Comment 19

β€’
1 month ago

Set keyboard lock status on the chrome document. JSWA implementation uses WGP which excludes chrome document from setting it.

Assignee

Comment 20

β€’
1 month ago

IPC Messages added:
CollectFullscreenDocsToUnfullscreen - is used to determine if the subtree for that PBrowser has any non-simple fullscreen docs by returning the subtree in fullscreen (if any) and metadata about it.

ExitFullscreenInRemote - Start the exit fullscreen steps at the target browsing context's document. See fullscreen API spec; if one does doc.exitFullscreen(), any documents below it must exit fully and then run the algorithm until it's first non-simple fullscreen document (if any). That's why we provide a browsing context here, so that the fullscreen service will be able to target a specific level to start from.

ExitFullscreenFullyForRemoteFrame - Completely exits the subtree out of fullscreen forcefully.

Assignee

Comment 21

β€’
1 month ago

Adds dom::FullscreenManager, a per-top-window queue that serially drives
EnterFullscreen / ExitFullscreen transitions. Each request mediates the
chrome OS-fullscreen transition, the OOP-frame notify round, and the
parent-side WindowGlobal state updates.

Also adds the FullscreenService glue that owns these managers and the
two chrome event helpers (MozDOMFullscreen:Entered /
MozDOMFullscreen:Exited).

nsGlobalWindowOuter::FinishDOMFullscreenChange now routes through
FullscreenService::FullscreenChanged when the service is enabled. No
managers are instantiated until the IPC trip (Part 13) queues a request,
so this patch is dormant at runtime.

Assignee

Comment 22

β€’
1 month ago

Three PWindowGlobal IPC messages introduced:

  1. RequestFullscreen - maps to element.requestFullscreen() and queuing a fullscreen request/transaction with the Fullscreen Service
  2. RequestExitFullscreen - maps to document.exitFullscreen() and queuing an exit of fullscreen request/transaction wiith the Fullscreen service
  3. FullscreenServiceTransactionComplete - The IPC message a request uses to complete a transaction with the fullscreen service, which informs the fullscreen service of the final result of the request and then allows for next Fullsreen API operation to start (if any is queued).

This transactional model, is what synchronizes access to fullscreen operations. Elements can still be removed mid-flight of one of these operations, and the behavior is best-effort and exiting if some unexpected result happens.

Assignee

Comment 23

β€’
1 month ago

Cancelling a request aborts mid-flight transactions as well as exits the browser out of fullscreen.

You need to log in before you can comment on or make changes to this bug.