1601475 - [MSU Capstone] Remove ImageContentLoaded event and ImageDocumentLoaded message handling code across the tree

Status: RESOLVED FIXED

(Toolkit :: General, task, P1)

Description

[Mike Conley (:mconley) (:⚙️)]

This isn't a port from a Message Manager actor to a JSWindowActor - this is a conversion from an old-school frame script function to a JSWindowActor.

Here's the chunk of the frame script we need to convert: https://searchfox.org/mozilla-central/rev/efdf9bb55789ea782ae3a431bda6be74a87b041e/toolkit/content/browser-child.js#55-70

Here is where that message is received: https://searchfox.org/mozilla-central/rev/efdf9bb55789ea782ae3a431bda6be74a87b041e/toolkit/content/widgets/browser-custom-element.js#1513-1518

We should instead have a new JSWindowActorChild created that instantiates on the ImageContentLoaded event, and then sends a message to a JSWindowWindowActorParent that will find the appropriate browser element and update its ._imageDocument property.

We'll also want to remove this line from the RDM tunnel code: https://searchfox.org/mozilla-central/rev/efdf9bb55789ea782ae3a431bda6be74a87b041e/devtools/client/responsive/browser/tunnel.js#462

and do a outerBrowser check in the JSWindowActorParent instead, like we do here: https://searchfox.org/mozilla-central/rev/efdf9bb55789ea782ae3a431bda6be74a87b041e/browser/actors/ClickHandlerParent.jsm#55-60

Comment 1

[Chris Peterson [:cpeterson]]

Tracking for Fission Nightly (M6)

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

The product::component has been changed since the backlog priority was decided, so we're resetting it.
For more information, please visit auto_nag documentation.

Comment 3

[:Gijs (he/him)]

Came across this with fx:general triage... Copying question from IRC:

Mike: hey, I hate to undo your hard work finding staatsty a bug... but I'm triaging [this bug] and looking at it. Can't we just rip all that code out?
It looks like it was landed as part of https://bugzilla.mozilla.org/show_bug.cgi?id=905439 for add-on compat when we still had XPCOM add-ons, but we don't anymore. I don't see anything reading .imageDocument anywhere (which is the only thing the message seems to be used for, cf. events created in DOM and handled in browser-child.js to become a message sent to the parent, handled in the browser widget to set browser._imageDocument, only read from the imageDocument getter on the browser), so I think we can just get rid of the whole lot - but maybe I'm missing something?

Comment 4

[Mike Conley (:mconley) (:⚙️)]

Hum. Looking through hgweb, I think _imageDocument actually came from bug 691610, and got tossed into browser.xml when it merged with remote-browser.xml.

It looks like tabbrowser.xml used to access browser.imageDocument, and then that usage was removed in bug 1453751 (https://phabricator.services.mozilla.com/D1672).

So yeah! I think we can just flat out remove this stuff! That to me sounds like a success scenario!

Comment 5

[:Gijs (he/him)]

Alright, updating the summary a bit. I think the links in comment #3 should help figure out what bits need to go away; Tyler, do ping us if you have questions...

Comment 6

[Tyler]

Attachment: Bug 1601475 - Remove ImageContentLoaded event and ImageDocumentLoaded message handling code across the tree

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e65c1bcb013f
Remove ImageContentLoaded event and ImageDocumentLoaded message handling code across the tree r=Gijs

Comment 8

[Daniel Varga [:dvarga]]

Backed out changeset e65c1bcb013f (Bug 1601475) for causing build bustage.

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&selectedJob=280242704&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=e65c1bcb013fb1d12cc3ae3b7474add9d0d52e3b

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=280249228&repo=autoland&lineNumber=54016

Backout link: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&selectedJob=280242704&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=f96293f37fc39f98c4ea9706ddf59edc5a3831b5

[task 2019-12-09T10:48:36.278Z] 10:48:36     INFO -  TEST-PASS | check_networking | libgkrust.a
[task 2019-12-09T10:48:36.278Z] 10:48:36     INFO -  make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/toolkit/library/rust'
[task 2019-12-09T10:48:36.278Z] 10:48:36     INFO -  /builds/worker/workspace/build/src/config/recurse.mk:32: recipe for target 'compile' failed
[task 2019-12-09T10:48:36.278Z] 10:48:36    ERROR -  make[2]: *** [compile] Error 2
[task 2019-12-09T10:48:36.278Z] 10:48:36     INFO -  /builds/worker/workspace/build/src/config/rules.mk:394: recipe for target 'default' failed
[task 2019-12-09T10:48:36.278Z] 10:48:36    ERROR -  make[1]: *** [default] Error 2
[task 2019-12-09T10:48:36.278Z] 10:48:36     INFO -  client.mk:125: recipe for target 'build' failed
[task 2019-12-09T10:48:36.278Z] 10:48:36     INFO -  make: *** [build] Error 2
[task 2019-12-09T10:48:36.299Z] 10:48:36     INFO -  315 compiler warnings present.
[task 2019-12-09T10:48:36.415Z] 10:48:36     INFO -  Notification center failed: Install notify-send (usually part of the libnotify package) to get a notification when the build finishes.
[task 2019-12-09T10:48:36.499Z] 10:48:36    ERROR - Return code: 2
[task 2019-12-09T10:48:36.499Z] 10:48:36  WARNING - setting return code to 2
[task 2019-12-09T10:48:36.499Z] 10:48:36    FATAL - 'mach build -v' did not run successfully. Please check log for errors.
[task 2019-12-09T10:48:36.499Z] 10:48:36    FATAL - Running post_fatal callback...
[task 2019-12-09T10:48:36.499Z] 10:48:36    FATAL - Exiting -1
[task 2019-12-09T10:48:36.499Z] 10:48:36     INFO - [mozharness: 2019-12-09 10:48:36.499433Z] Finished build step (failed)
[task 2019-12-09T10:48:36.499Z] 10:48:36     INFO - Running post-run listener: _parse_build_tests_ccov
[task 2019-12-09T10:48:36.499Z] 10:48:36     INFO - Running command: ['/builds/worker/workspace/build/src/obj-x86_64-pc-linux-gnu/_virtualenvs/init/bin/python', 'mach', 'python', 'testing/parse_build_tests_ccov.py'] in /builds/worker/workspace/build/src
[task 2019-12-09T10:48:36.499Z] 10:48:36     INFO - Copy/paste: /builds/worker/workspace/build/src/obj-x86_64-pc-linux-gnu/_virtualenvs/init/bin/python mach python testing/parse_build_tests_ccov.py
[task 2019-12-09T10:48:36.501Z] 10:48:36     INFO - Using env: {'ACCEPTED_MAR_CHANNEL_IDS': 'firefox-mozilla-central',

Comment 9

[:Gijs (he/him)]

[task 2019-12-09T10:40:18.746Z] 10:40:18     INFO -  /builds/worker/fetches/binutils/bin/ld: libxul.so: hidden symbol `_ZN7mozilla3dom13ImageListener13OnStopRequestEP10nsIRequest8nsresult' isn't defined

looks like we need to either define the dummy OnStopRequest or not use the DECL macro to declare both OnStartRequest and OnStopRequest .

This is kind of odd, because phabricator no longer has treeherder links now that this has landed, but I checked those and they were green, and because this included compiled code I assumed we'd run builds and they passed...

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/658924dcdbf0
Remove ImageContentLoaded event and ImageDocumentLoaded message handling code across the tree r=Gijs,mccr8

Comment 11

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/658924dcdbf0

Comment 12

[Mark Banner (:standard8)]

Clearing NI of relanded patch.