1503070 - Remove DOMRequest usage in mozbrowser API

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement, P2)

Description

[Boris Zbarsky [:bzbarsky]]

And in fact, we should remove as much of the mozbrowser API as we can get away with.

jryans, do you know how I can exercise the sendTouchEvent code in devtools/server/actors/emulation/touch-simulator.js?  Opening responsive design mode and clicking and whatnot doesn't seem to do it...

Comment 1

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

I believe you also need to enable touch simulation mode in RDM:

1. Go to some page
2. Open RDM
3. Click the hand icon in the toolbar so that it turns blue / enabled
4. Click in the viewport

Comment 2

[Boris Zbarsky [:bzbarsky]]

Thanks.  Looks like that codepath doesn't use the mozbrowser sendTouchEvent.

Comment 3

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Ah sorry, wasn't sure if you meant the touch simulator's `sendTouchEvent` function or the mozbrowser API `sendTouchEvent` that it could possibly call.  For DevTools, the touch simulator is running along side the content.  The mozbrowser path is a vestige from B2G that seems removable.

For the mozbrowser APIs more generally, RDM only uses a few, and mostly out of convenience.  (Mozbrowser _frames_ however we really do need, or at least some concept that creates top-level browsing contexts inside another document.)  Main usages of mozbrowser APIs in RDM:

* a few of the navigation methods[1]
* basic window.open handling[2]

These could all be converted to use other approaches that more closely match regular <xul:browser>s, assuming you wanted to entirely remove mozbrowser APIs.

[1]: https://searchfox.org/mozilla-central/rev/fc3d974254660b34638b2af9d5431618b191b233/devtools/client/responsive.html/browser/web-navigation.js
[2]: https://searchfox.org/mozilla-central/rev/fc3d974254660b34638b2af9d5431618b191b233/devtools/client/responsive.html/browser/tunnel.js#235-258

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: part 1. Remove next-paint listeners from BrowserElement

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: part 2. Remove download() method from BrowserElement

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: part 3. Remove purgeHistory() method from BrowserElement

Comment 7

[Boris Zbarsky [:bzbarsky]]

Attachment: part 4. Remove getScreenshot() method from BrowserElement

Comment 8

[Boris Zbarsky [:bzbarsky]]

Attachment: part 5. Remove zoom() method from BrowserElement

Comment 9

[Boris Zbarsky [:bzbarsky]]

Attachment: part 6. Remove getContentDimensions() method from BrowserElement

Comment 10

[Boris Zbarsky [:bzbarsky]]

Attachment: part 7. Remove find methods from BrowserElement

Comment 11

[Boris Zbarsky [:bzbarsky]]

Attachment: part 8. Remove executeScript API from BrowserElement

Comment 12

[Boris Zbarsky [:bzbarsky]]

Attachment: part 9. Remove getWebManifest() API from BrowserElement

Comment 13

[Boris Zbarsky [:bzbarsky]]

Attachment: part 10. Remove sendTouchEvent() API from BrowserElement

Comment 14

[Boris Zbarsky [:bzbarsky]]

Attachment: part 11. Remove some dead gotDOMRequestResult bits in BrowserElementParent.js

Nothing sends the 'got-visible' or 'got-set-input-method-active' messages.

Comment 15

[Boris Zbarsky [:bzbarsky]]

Attachment: part 12. Stop using DOMRequest in BrowserElement getCanGoForward/Back APIs

Comment 16

[Boris Zbarsky [:bzbarsky]]

I kept getCanGoBack/Forward because they are needed for the one test we have that actually tests the history API bits on BrowserElement.  The devtools code does use those bits, but the devtools tests testing them are disabled, of course.  :(

Comment 17

[Boris Zbarsky [:bzbarsky]]

Oh, and I also kept the methods I think devtools actually uses.  Though come to think of it, maybe I can remove sendMouseEvent too...

Comment 18

[Boris Zbarsky [:bzbarsky]]

Oh, right, sendMouseEvent is being used in kinda critical-looking tests (browserElement_OpenTab.js) too.

Comment 19

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 9021398 [details] [diff] [review]
part 10.  Remove sendTouchEvent() API from BrowserElement

Review of attachment 9021398 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for working on this.  This change looks correct to me and shouldn't impact RDM.

(Sorry for the sad state of testing in this area... I agree it's quite bad.)

Comment 20

[Boris Zbarsky [:bzbarsky]]

It happens...  Thank you for the quick review!

Comment 21

[(no longer active)]

Comment on attachment 9021392 [details] [diff] [review]
part 4.  Remove getScreenshot() method from BrowserElement

Review of attachment 9021392 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/browser-element/mochitest/mochitest-oop.ini
@@ +1,3 @@
>  [DEFAULT]
> +# XXXbz now that we're not testing OpenMixedProcess, can we reenable
> +# these tests on Android and e10s?

Hmm, maybe file a bug on this and put the bug number here?

Comment 22

[Boris Zbarsky [:bzbarsky]]

> Hmm, maybe file a bug on this and put the bug number here?

Bug 1504026 filed.

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/29ffb117b6b0
part 1.  Remove next-paint listeners from BrowserElement.  r=ehsan
https://hg.mozilla.org/integration/mozilla-inbound/rev/9b28e56f5082
part 2.  Remove download() method from BrowserElement.  r=ehsan
https://hg.mozilla.org/integration/mozilla-inbound/rev/22f4a8229e00
part 3.  Remove purgeHistory() method from BrowserElement.  r=ehsan
https://hg.mozilla.org/integration/mozilla-inbound/rev/ce8b800b5c8f
part 4.  Remove getScreenshot() method from BrowserElement.  r=ehsan
https://hg.mozilla.org/integration/mozilla-inbound/rev/cf8001107012
part 5.  Remove zoom() method from BrowserElement.  r=ehsan
https://hg.mozilla.org/integration/mozilla-inbound/rev/ca81706ad834
part 6.  Remove getContentDimensions() method from BrowserElement.  r=ehsan
https://hg.mozilla.org/integration/mozilla-inbound/rev/67ae5d3fbc1d
part 7.  Remove find methods from BrowserElement.  r=ehsan
https://hg.mozilla.org/integration/mozilla-inbound/rev/a450aaa87495
part 8.  Remove executeScript API from BrowserElement. r=ehsan
https://hg.mozilla.org/integration/mozilla-inbound/rev/8c03e85d67a4
part 9.  Remove getWebManifest() API from BrowserElement. r=ehsan
https://hg.mozilla.org/integration/mozilla-inbound/rev/6290a4836442
part 10.  Remove sendTouchEvent() API from BrowserElement. r=jryans
https://hg.mozilla.org/integration/mozilla-inbound/rev/ad3f50ecca56
part 11.  Remove some dead gotDOMRequestResult bits in BrowserElementParent.js.  r=ehsan
https://hg.mozilla.org/integration/mozilla-inbound/rev/589a6293a3b7
part 12.  Stop using DOMRequest in BrowserElement getCanGoForward/Back APIs.  r=ehsan

Comment 24

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/29ffb117b6b0
https://hg.mozilla.org/mozilla-central/rev/9b28e56f5082
https://hg.mozilla.org/mozilla-central/rev/22f4a8229e00
https://hg.mozilla.org/mozilla-central/rev/ce8b800b5c8f
https://hg.mozilla.org/mozilla-central/rev/cf8001107012
https://hg.mozilla.org/mozilla-central/rev/ca81706ad834
https://hg.mozilla.org/mozilla-central/rev/67ae5d3fbc1d
https://hg.mozilla.org/mozilla-central/rev/a450aaa87495
https://hg.mozilla.org/mozilla-central/rev/8c03e85d67a4
https://hg.mozilla.org/mozilla-central/rev/6290a4836442
https://hg.mozilla.org/mozilla-central/rev/ad3f50ecca56
https://hg.mozilla.org/mozilla-central/rev/589a6293a3b7

Comment 25

[Chris Mills [:cmills]]

I've updated the docs for this:

I've collected all the methods that are being removed in Fx 65 in this section:

https://developer.mozilla.org/en-US/docs/Web/API/HTMLIFrameElement#Browser_API_methods_removed_in_Firefox_65

Updated all the pages to say that those features are being removed in 65

I've also updated the getCanGoForward() and getCanGoBack() pages to just show the promise-based syntax (previously we were showing the promise and the DOMRequest syntax)

I also submitted a PR to update the browser compat data: https://github.com/mdn/browser-compat-data/pull/3062

Comment 26

[Boris Zbarsky [:bzbarsky]]

Chris, thank you!

Comment 27

[[:fabrice] Fabrice Desré]

Seeing the bug title I thought that was about returning promises instead of DOMRequests, not the nuking that is happening there.

Comment 28

[Boris Zbarsky [:bzbarsky]]

The initial impetus was to do the Promise thing (and that did happen for getCanGo*), but I didn't really see a  point in keeping around a bunch of unused code either.