Closed
Bug 1423282
Opened 6 years ago
Closed 6 years ago
Remove OOP frame manager and targetted IPC messages
Categories
(Remote Protocol :: Marionette, enhancement)
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 | ||
Updated•6 years ago
|
Assignee | ||
Comment 1•6 years ago
|
||
WIP try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d919a1e9c5b1bc02f4b2be57d91b5d09705a66f0
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 13•6 years ago
|
||
try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5166cee0179669f69da343a58e86d186f8803dd4
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 25•6 years ago
|
||
mozreview-review |
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 26•6 years ago
|
||
mozreview-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 27•6 years ago
|
||
mozreview-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 28•6 years ago
|
||
mozreview-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 29•6 years ago
|
||
mozreview-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 30•6 years ago
|
||
mozreview-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 31•6 years ago
|
||
mozreview-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 32•6 years ago
|
||
mozreview-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 33•6 years ago
|
||
mozreview-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 34•6 years ago
|
||
mozreview-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 35•6 years ago
|
||
mozreview-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 36•6 years ago
|
||
mozreview-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 37•6 years ago
|
||
mozreview-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 38•6 years ago
|
||
mozreview-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 39•6 years ago
|
||
mozreview-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+
Assignee | ||
Updated•6 years ago
|
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)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 51•6 years ago
|
||
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
Comment 52•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/6ba05e4166e0 https://hg.mozilla.org/mozilla-central/rev/f3fbfc5bcd21 https://hg.mozilla.org/mozilla-central/rev/b25e29a946ce https://hg.mozilla.org/mozilla-central/rev/152a9b346126 https://hg.mozilla.org/mozilla-central/rev/f6846f826cb2 https://hg.mozilla.org/mozilla-central/rev/7bf4bf4d0938 https://hg.mozilla.org/mozilla-central/rev/70bcdce62f21 https://hg.mozilla.org/mozilla-central/rev/01b7837d7ec9 https://hg.mozilla.org/mozilla-central/rev/60511611ac52 https://hg.mozilla.org/mozilla-central/rev/bd6428b1fd5a https://hg.mozilla.org/mozilla-central/rev/b355b5aa1f37
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox59:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Updated•1 year ago
|
Product: Testing → Remote Protocol
You need to log in
before you can comment on or make changes to this bug.
Description
•