Open Bug 1646544 Opened 4 years ago Updated 3 years ago

Track last focus time in the parent process

Categories

(Core :: DOM: Service Workers, task)

task

Tracking

()

ASSIGNED
Fission Milestone Future

People

(Reporter: kmag, Assigned: asuth)

References

(Blocks 1 open bug)

Details

It seems like it should at least walk all in-process ancestors regardless of OOP interstitials.

Severity: -- → N/A

Tracking "Figure out GetInProcessTop usage" bugs for Fission M6b.

Fission Milestone: --- → M6b

Henri, could you review the usage and see if this needs fixing for Fission?

Flags: needinfo?(hsivonen)

This code exists for ServiceWorker Clients.matchAll() from bug 1266747. It seems to me that we should at least do what comment 0 suggests. It's unclear to me if we should also do something cross-process and complicated. Needinfoing baku for that.

Flags: needinfo?(hsivonen) → needinfo?(amarchesini)
Component: DOM: Core & HTML → DOM: Service Workers
Flags: needinfo?(amarchesini) → needinfo?(bugmail)

Short answer: The current implementation should be sufficient to pass the WPT test with fission enabled. This is consistent with the test currently passing according to searchfox, I think...

Short plan: Add a comment to the existing code explaining the sufficiency of the current implementation but where it needs improvement and link to a spinoff bug tracking a longer term more fancy solution.

Longer Analysis, More Fancy Solution

There's the following levels of implementation fidelity possible:

  1. Same-origin direct-ancestor. "OOP interstitials" break the same-origin case, and cross-origin breaks.
  2. Same-origin compensating for OOP interstitials, but cross-origin breaks.
  3. Cross-origin works too.

The service-workers/service-worker/clients-matchall-order.https.html test only needs level 1 same-origin direct ancestor. This is because it generates only a single origin hierarchy with at most a parent and child iframe.

I understand the spec to be dictating that level 3 be implemented given that https://html.spec.whatwg.org/multipage/interaction.html#focusing-steps in step 8 invokes https://html.spec.whatwg.org/multipage/interaction.html#focus-update-steps which results in the entire browsing context hierarchy through the top-level browsing being focused
and bkelly's authorship intent. (Note that our current pre-fission implementation does treat the entire new chain as being focused at the same time, whereas "focus update steps" has a delta optimization so that elements in both the old chain and new chain don't experience a spurious blur/focus pair. I'm not sure the SW spec is as clear as it could be here.)

If the ClientManagerService understood the hierarchical relationship of Clients, it would potentially be straightforward for a Client to indicate that it is being focused and to atomically update the Client's focus time and that of all of its ancestors. The ClientManagerService currently doesn't know this, but for security/auditing purposes, it's something it wants to know. Specifically, (using DocumentChannel) the parent process already should know a priori the client id of ClientSources that should come into existence including their principal and the process they should be spawned in. The RemoteWorkerService covers the worker cases that create new agent clusters. All same-origin global creations (nested same-origin iframes, dedicated workers) can implicitly be trusted. As can null principal creation. (And client id's have explicit first-to-claim and hard-to-collide characteristics.)

Implementation-wise, this would imply creating a data structure that explicitly consists of data that's inductively known to be true by the parent process. Let's call it ClientTruth. These would be provided by DocumentChannel and the RemoteWorkerService prior to spawning globals in processes. Data such as the agentClusterId and principalInfo could be extracted into ClientTruth from IPCClientInfo.

For focus tracking, we'd potentially want an additional data-structure that would hold the focus state timestamps which is distinct from the existing IPCClientWindowState which is authored by a window about itself. Let's call this ClientRumors because we're allowing a client to assert facts about its potentially cross-origin ancestors which shouldn't be trusted. (And to round out the idiomatic naming, perhaps ClientAutobiography might cover the remainder of the Client-authored data.)

An alternate approach would be to track the focus time on the BrowsingContext in the parent process. But it's not clear anyone cares about these relative orderings apart from the Clients API and there are ordering/consistency issues that immediately show up thanks to communication happening via PContent for BrowsingContexts and PBackground for the clients API.

Assignee: nobody → bugmail
Status: NEW → ASSIGNED
Flags: needinfo?(bugmail)

(In reply to Andrew Sutherland [:asuth] (he/him) from comment #4)

Short plan: Add a comment to the existing code explaining the sufficiency of the current implementation but where it needs improvement and link to a spinoff bug tracking a longer term more fancy solution.

Neha, I assume, that this can wait for M6c ?

Flags: needinfo?(nkochar)

Sure, moved to M6c.

Fission Milestone: M6b → M6c
Flags: needinfo?(nkochar)

Renaming the bug and moving to Future based on asuth's findings in comment 4. Thanks Andrew!

Fission Milestone: M6c → Future
Summary: Figure out if GetInProcessParentDocument usage in nsFocusManager::SetFocusedWindowInternal is OK → Track last focus time in the parent process
You need to log in before you can comment on or make changes to this bug.