Closed Bug 1741834 Opened 3 years ago Closed 2 years ago

MessageHandler: support transactions when using sessionData

Categories

(Remote Protocol :: WebDriver BiDi, task, P1)

task
Points:
5

Tracking

(firefox110 fixed)

RESOLVED FIXED
110 Branch
Tracking Status
firefox110 --- fixed

People

(Reporter: jdescottes, Assigned: Sasha)

References

(Blocks 1 open bug)

Details

(Whiteboard: [webdriver:m5], [wptsync upstream])

Attachments

(2 files)

This is a followup to the initial MessageHandler SessionData implementation added in Bug 1713443.

Once we support specific contexts for APIs such as session.subscribe, the SessionData will need to reflect for which contexts a given event is currently subscribed to. Given the specification, this might be difficult to implement efficiently with the current SessionData API.

For session.subscribe, if you:

  • subscribe to an event for a specific context
  • subscribe to the same event globally
  • unsubscribe globally

You should be fully unsubscribed from the event, meaning that the initial "context-specific" subscription was overridden by the global one.

If we use the current SessionData API, this would look as follows:

// !! THIS IS PSEUDOCODE !!

// 1. subscribe to an event for a specific context
addSessionData("log", "events", { type: "top-browsing-context", id: ${idForTopLevelBC} }, ["entryAdded"]);

// 2. subscribe to the same event globally
// 2.a. First need to remove all the specific contexts (assuming we'd have a way to list them)
removeSessionData("log", "events", { type: "top-browsing-context", id: ${idForTopLevelBC} }, ["entryAdded"]);
// 2.b. then add a global subscription
addSessionData("log", "events", { type: "all" }, ["entryAdded"]);

// 3. unsubscribe globally
removeSessionData("log", "events", { type: "all" }, ["entryAdded"]);

The issue with this is that the intermediary step 2.a. will lead to unsubscribe from entryAdded on the specific contexts which already had created a listener. And right after, we would re-subscribe for step 2.b. Meaning we could miss events in that window.

So instead we want step 2 to be atomic. Contexts which didn't previously subscribe to the event should create the listeners, and contexts which already had the listeners should not do anything. But we still want to reflect in SessionData the fact the only the "all" context remains, to be consistent with what the spec expects.

A suggestion to handle this is to allow transactions when interacting with SessionData, where you can call addSessionData/removeSessionData as many times as needed, which would then be committed as a single transaction.

This would avoid pushing more logic to the SessionData layer, because the way we want to handle contexts for session.subscribe might be different from what other Commands will need.

We don't need this to be in milestone 2. We will allow for to filter subscribe calls on specific browsing contexts in milestone 2, but we can limit this to new subscribe calls and say that this doesn't apply to new contexts yet.

Note that supporting transactions would be easier if we would simply broadcast the relevant sessionData to existing contexts instead of broadcasting a "diff". Right now the diff is very easy to compute because we broadcast for each individual add/removeSessionData call. But if we allowed transaction, computing the diff would be harder. If instead we broadcast the whole session data (limited to a given module/category to avoid broadcasting too much), then individual modules would be responsible for computing the diff which is relevant to them.

Points: --- → 8
Whiteboard: [webdriver:triage]
Priority: -- → P3
Whiteboard: [bidi-m3-mvp]

This is related to the full implementation for subscribe and unsubscribe (see bug 1723102).

Priority: P3 → P2
Whiteboard: [bidi-m3-mvp] → [webdriver:backlog]
Blocks: 1756610
Depends on: 1763137
Blocks: 1790362
Points: 8 → 5
Priority: P2 → P1
Whiteboard: [webdriver:backlog] → [webdriver:m5]
No longer blocks: 1756610
Assignee: nobody → aborovova
Status: NEW → ASSIGNED
Attachment #9306903 - Attachment description: WIP: Bug 1741834 - [messagehandler] Support transactions when using sessionData. → Bug 1741834 - [messagehandler] Support transactions when using sessionData.
Duplicate of this bug: 1803401
Attachment #9309339 - Attachment description: Bug 1741834 - [wdspec] Add test for browsingContext.contextCreated for a specific context → Bug 1741834 - [wdspec] Add test for browsingContext.contextCreated for a specific context.
Pushed by aborovova@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/bc541786a015 [messagehandler] Support transactions when using sessionData. r=webdriver-reviewers,jdescottes https://hg.mozilla.org/integration/autoland/rev/955a99216cfa [wdspec] Add test for browsingContext.contextCreated for a specific context. r=webdriver-reviewers,jdescottes
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/37626 for changes under testing/web-platform/tests
Whiteboard: [webdriver:m5] → [webdriver:m5], [wptsync upstream]
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 110 Branch
Upstream PR merged by moz-wptsync-bot
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: