Closed Bug 1676665 Opened 4 years ago Closed 4 years ago

Commands should not call "this.listener.switchToFrame" and "this.listener.setBrowsingContextId" unconditionally

Categories

(Remote Protocol :: Marionette, task, P1)

Product:
Remote Protocol
Component:
Marionette
Version:
Default
Type:
task

Tracking

(Fission Milestone:M7, firefox85 fixed)

Status:
RESOLVED FIXED
Milestone:
85 Branch
Project Flags:
Fission Milestone M7
Tracking Flags:
Tracking Status
firefox85 --- fixed

People

(Reporter: whimboo, Assigned: whimboo)

References

Details

(Whiteboard: [marionette-fission-mvp][simple])

Attachments

(2 files)

Bug 1676665 - [marionette] Conditionally call switchToFrame for actor or framescript.
(deleted), text/x-phabricator-request
Details
Bug 1676665 - [marionette] Remove setBrowsingContextId from framescript.
(deleted), text/x-phabricator-request
Details

Right now WebDriver:NavigateTo and WebDriver:Refresh update the browsing context to the top-level one before triggering the navigation. Right now this gets done unconditionally via this.listener.switchToFrame even when actors are enabled:

https://searchfox.org/mozilla-central/rev/02cb78667e87ccc42fea5edc6f3f2dd2edd6ecd5/testing/marionette/driver.js#1193

For those two calls we add the condition for MarionettePrefs.useActors and call into the actor specific method instead.

Further WebDriver:SwitchToFrame and WebDriver:SwitchToParentFrame are setting the id of the content browsing context:

https://searchfox.org/mozilla-central/rev/02cb78667e87ccc42fea5edc6f3f2dd2edd6ecd5/testing/marionette/driver.js#1813

Here we might even not need the calls anymore given that all the commands including the page load events are now implemented via actors. So these lines might be removed completely.

Whiteboard: [marionette-fission-mvp][simple]
Priority: P3 → P2

Tracking marionette-fission-mvp bugs for Fission Beta milestone (M7).

Fission Milestone: --- → M7
Assignee: nobody → hskupin
Status: NEW → ASSIGNED
Priority: P2 → P1

With all the commands implemented by the MarionetteCommands
JSWindowActor pair there is no need anymore to update the
framescript when actors are enabled.

Depends on D97177

Pushed by hskupin@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/49e3414e9200 [marionette] Conditionally call switchToFrame for actor or framescript. r=marionette-reviewers,jdescottes https://hg.mozilla.org/integration/autoland/rev/311ceb127ade [marionette] Remove setBrowsingContextId from framescript. r=marionette-reviewers,jdescottes

https://hg.mozilla.org/mozilla-central/rev/49e3414e9200
https://hg.mozilla.org/mozilla-central/rev/311ceb127ade

Status: ASSIGNED → RESOLVED
Closed: 4 years ago
status-firefox85: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 85 Branch

== Change summary for alert #27776 (as of Fri, 20 Nov 2020 10:39:15 GMT) ==

Improvements:

Ratio Suite Test Platform Options Absolute values (old vs new)
1% Base Content JS windows10-64-shippable 2,789,792.00 -> 2,772,101.33
1% Base Content JS windows10-64-shippable 2,784,627.00 -> 2,767,590.00

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=27776

Regressions: 1691348
Product: Testing → Remote Protocol
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: