Closed Bug 1423282 Opened 6 years ago Closed 6 years ago

Remove OOP frame manager and targetted IPC messages

Categories

(Remote Protocol :: Marionette, enhancement)

Version 3
enhancement
Not set
normal

Tracking

(firefox59 fixed)

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: ato, Assigned: ato)

References

Details

Attachments

(11 files)

59 bytes, text/x-review-board-request
automatedtester
: review+
Details
59 bytes, text/x-review-board-request
automatedtester
: review+
Details
59 bytes, text/x-review-board-request
automatedtester
: review+
Details
59 bytes, text/x-review-board-request
automatedtester
: review+
Details
59 bytes, text/x-review-board-request
automatedtester
: review+
impossibus
: review+
Details
59 bytes, text/x-review-board-request
automatedtester
: review+
impossibus
: review+
Details
59 bytes, text/x-review-board-request
automatedtester
: review+
Details
59 bytes, text/x-review-board-request
automatedtester
: review+
impossibus
: review+
Details
59 bytes, text/x-review-board-request
automatedtester
: review+
impossibus
: review+
Details
59 bytes, text/x-review-board-request
automatedtester
: review+
Details
59 bytes, text/x-review-board-request
automatedtester
: review+
Details
Building on https://bugzil.la/1423209#c2, we want to remove
the mechanism for doing IPC messages to a frame script using the
frame’s targetted message manager.  Although this in theory is a
better approach than broadcasting [1], the targetted message manager
technique (implemented in GeckoDriver#sendTargettedAsyncMessage_
[2]) is currently not used at all and relies on a lot of old and
unused code in testing/marionette/frame.js.  This theory is supported
by code coverage [3].

Once https://bugzil.la/marionette-window-tracking is done we can
move the IPC system back to using targetted message managers because
we will be able to reliably trust the reference we hold to the
current browser.  The problem at the moment is that we identify
browsers by their outerWindowID (and not their permanentKey),
which changes when a remoteness change occurs.

  [1] The ‘broadcasting’ technique consists of sending the
      IPC message with a name appended by the content browser’s
      outerWindowID so that it is only picked up by the desired
      browser.  This has obviously scalability problems, but is
      how Marionette currently functions.

  [2] https://searchfox.org/mozilla-central/source/testing/marionette/driver.js#364-380

  [3] https://codecov.io/gh/marco-c/gecko-dev/src/8faace152630a16ea00aa20337c8c132695e8f09/testing/marionette/driver.js#L347
Assignee: nobody → ato
Status: NEW → ASSIGNED
Depends on: 1423209
Blocks: 1423359
Blocks: 1384641
Comment on attachment 8934686 [details]
Bug 1423282 - Drop Marionette:switchToModalOrigin IPC message.

https://reviewboard.mozilla.org/r/205570/#review211488
Attachment #8934686 - Flags: review?(dburns) → review+
Comment on attachment 8934687 [details]
Bug 1423282 - Drop Marionette:log IPC message.

https://reviewboard.mozilla.org/r/205572/#review211490
Attachment #8934687 - Flags: review+
Comment on attachment 8934688 [details]
Bug 1423282 - Drop Marionette:shareData IPC message.

https://reviewboard.mozilla.org/r/205574/#review211492
Attachment #8934688 - Flags: review+
Comment on attachment 8934689 [details]
Bug 1423282 - Drop MarionetteFrame:handleModal IPC message.

https://reviewboard.mozilla.org/r/205576/#review211558
Attachment #8934689 - Flags: review?(dburns) → review+
Comment on attachment 8934690 [details]
Bug 1423282 - Drop MarionetteFrame:getInterruptedState IPC message.

https://reviewboard.mozilla.org/r/205578/#review211788
Attachment #8934690 - Flags: review?(dburns) → review+
Comment on attachment 8934691 [details]
Bug 1423282 - Drop MarionetteFrame:getCurrentFrameId IPC message.

https://reviewboard.mozilla.org/r/205580/#review211790
Attachment #8934691 - Flags: review?(dburns) → review+
Comment on attachment 8934692 [details]
Bug 1423282 - Remove aliveCheck to frame message manager.

https://reviewboard.mozilla.org/r/205582/#review211794
Attachment #8934692 - Flags: review?(dburns) → review+
Comment on attachment 8934693 [details]
Bug 1423282 - Remove legacy action chain browser close guard.

https://reviewboard.mozilla.org/r/205584/#review211796

::: commit-message-0c337:5
(Diff revision 2)
> +Bug 1423282 - Remove legacy action chain browser close guard. r?automatedtester,maja_zf
> +
> +It turns out that we no longer need to guard against the browser/frame
> +closing when using the legacy actions module.  This means we can
> +get rid of GeckoDriver#addFrameClsoeListener, which again populates

s/Clsoe/Close/
Attachment #8934693 - Flags: review?(dburns) → review+
Comment on attachment 8934694 [details]
Bug 1423282 - Drop Marionette:emitTouchEvent IPC message and related infra.

https://reviewboard.mozilla.org/r/205586/#review211910
Attachment #8934694 - Flags: review?(mjzffr) → review+
Comment on attachment 8934694 [details]
Bug 1423282 - Drop Marionette:emitTouchEvent IPC message and related infra.

https://reviewboard.mozilla.org/r/205586/#review212248
Attachment #8934694 - Flags: review?(dburns) → review+
Comment on attachment 8934695 [details]
Bug 1423282 - Remove unused IPC listener Marionette:getImportedScripts.

https://reviewboard.mozilla.org/r/205588/#review212250
Attachment #8934695 - Flags: review+
Comment on attachment 8934696 [details]
Bug 1423282 - Remove last remenants of frame.Manager.

https://reviewboard.mozilla.org/r/205590/#review212254
Attachment #8934696 - Flags: review?(dburns) → review+
Comment on attachment 8934690 [details]
Bug 1423282 - Drop MarionetteFrame:getInterruptedState IPC message.

https://reviewboard.mozilla.org/r/205578/#review212436
Attachment #8934690 - Flags: review?(mjzffr) → review+
Comment on attachment 8934691 [details]
Bug 1423282 - Drop MarionetteFrame:getCurrentFrameId IPC message.

https://reviewboard.mozilla.org/r/205580/#review212440
Attachment #8934691 - Flags: review?(mjzffr) → review+
Comment on attachment 8934693 [details]
Bug 1423282 - Remove legacy action chain browser close guard.

https://reviewboard.mozilla.org/r/205584/#review212442
Attachment #8934693 - Flags: review?(mjzffr) → review+
Attachment #8934687 - Flags: review?(hskupin)
Attachment #8934688 - Flags: review?(hskupin)
Attachment #8934689 - Flags: review?(hskupin)
Attachment #8934695 - Flags: review?(hskupin)
Attachment #8934696 - Flags: review?(hskupin)
Pushed by atolfsen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6ba05e4166e0
Drop Marionette:switchToModalOrigin IPC message. r=automatedtester
https://hg.mozilla.org/integration/autoland/rev/f3fbfc5bcd21
Drop Marionette:log IPC message. r=automatedtester
https://hg.mozilla.org/integration/autoland/rev/b25e29a946ce
Drop Marionette:shareData IPC message. r=automatedtester
https://hg.mozilla.org/integration/autoland/rev/152a9b346126
Drop MarionetteFrame:handleModal IPC message. r=automatedtester
https://hg.mozilla.org/integration/autoland/rev/f6846f826cb2
Drop MarionetteFrame:getInterruptedState IPC message. r=automatedtester,maja_zf
https://hg.mozilla.org/integration/autoland/rev/7bf4bf4d0938
Drop MarionetteFrame:getCurrentFrameId IPC message. r=automatedtester,maja_zf
https://hg.mozilla.org/integration/autoland/rev/70bcdce62f21
Remove aliveCheck to frame message manager. r=automatedtester
https://hg.mozilla.org/integration/autoland/rev/01b7837d7ec9
Remove legacy action chain browser close guard. r=automatedtester,maja_zf
https://hg.mozilla.org/integration/autoland/rev/60511611ac52
Drop Marionette:emitTouchEvent IPC message and related infra. r=automatedtester,maja_zf
https://hg.mozilla.org/integration/autoland/rev/bd6428b1fd5a
Remove unused IPC listener Marionette:getImportedScripts. r=automatedtester
https://hg.mozilla.org/integration/autoland/rev/b355b5aa1f37
Remove last remenants of frame.Manager. r=automatedtester
Product: Testing → Remote Protocol
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: