1271399 - Need to shim protocol.js move for add-ons

Status: RESOLVED FIXED

(DevTools :: General, defect, P1)

Description

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

In bug 1270173, protocol.js was moved to shared.

However, there are also add-ons that load this module to define their own actors, including the WebSocket Monitor[1] that we suggest people use on hacks.m.o.

We should create a shim module so that is can still be loaded with the old module ID as well as the new one.

[1]: https://addons.mozilla.org/en-US/firefox/addon/websocket-monitor/

Comment 1

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

:ednapiranha, can you take a look at this?  Creating a small shim that lives in /devtools/server/shims and loads the module for the new ID should do it.

Comment 2

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

[Tracking Requested - why for this release]: Let's make sure we land something here in 49 to maintain add-on compat.

Comment 3

[Marcia Knous [:marcia]]

Tracking for 49 as add on compat is important.

Comment 4

[Jen Fong-Adwent [:ednapiranha]]

Attachment: Bug1271399.patch

So I think I have most of the setup, but existing add-ons still aren't able to detect the path. I did notice that this path (EXTRA_JS_MODULES.devtools.server) points to resource://gre/modules/devtools/server/protocol.js rather than resource://devtools/server/protocol.js. 

Any idea what I might be missing here to make it point to the correct location?

Comment 5

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

Comment on attachment 8750879 [details] [diff] [review]
Bug1271399.patch

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

::: devtools/server/shims/moz.build
@@ +15,5 @@
>  EXTRA_JS_MODULES.devtools += [
>      'dbg-server.jsm',
>  ]
> +
> +# Extra compatibility layer for transitional URLs used for part of 44 cycle

Update this comment to note that protocol.js was moved in 49

@@ +16,5 @@
>      'dbg-server.jsm',
>  ]
> +
> +# Extra compatibility layer for transitional URLs used for part of 44 cycle
> +EXTRA_JS_MODULES.devtools.server += [

Sorry, I gave you some bad info here! :(

Here's what should do the trick:

FINAL_TARGET_FILES.chrome.devtools.modules.devtools.server += [ ...

Now with more dots!

::: devtools/server/shims/protocol.js
@@ +33,5 @@
> +];
> +
> +const { require } =
> +  Cu.import("resource://devtools/shared/Loader.jsm", {});
> +const module = require("devtools/shared/protocol");

Since it's a CommonJS module, I think you can do:

module.exports = require("devtools/shared/protocol");

to export everything, and then you shouldn't need `EXPORTED_SYMBOLS` for this case.

Comment 6

[Jen Fong-Adwent [:ednapiranha]]

Attachment: Bug1271399.patch

Added your changes, but still having path issues.

Comment 7

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

Comment on attachment 8750914 [details] [diff] [review]
Bug1271399.patch

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

Looks like the DIST_SUBDIR in the moz.build is messing this up.  I'll attach my own attempt for you to check over.

Comment 8

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

Attachment: Shim for protocol.js

Comment 9

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

The existing files are moved into a new directory (the diff view is bad at showing this, can check the patch file directly at the attachment link).

Comment 10

[Jen Fong-Adwent [:ednapiranha]]

Comment on attachment 8750927 [details] [diff] [review]
Shim for protocol.js

Yep this works for me!

Comment 11

[Jen Fong-Adwent [:ednapiranha]]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b7be1a39898a

Comment 12

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/fe3494949571

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/fe3494949571