1359085 - Extract devtools/server/content-globals from devtools

Status: RESOLVED FIXED

(DevTools :: General, enhancement, P3)

Description

[Julian Descottes [:jdescottes]]

addon-sdk/source/lib/sdk/loader/sandbox.js relies on devtools/server/content-globals . It is actually the only consumer for this class in mozilla-central today (except for tests, including one in devtools: devtools/shared/tests/mochitest/test_devtools_extensions.html)

Since DevTools are being removed from mozilla central, content-globals.js should be moved outside of devtool to addon-sdk.

Comment 1

[Julian Descottes [:jdescottes]]

Simply moving the module to addon-sdk is not enough. 

content-globals.js relies on being loaded by the devtools loader in order to :
- be able to inject globals
- check if we are loaded in a worker (relying on the isWorker global)

Comment 2

[Julian Descottes [:jdescottes]]

I am actually not sure if the globals set using content-globals are ever used. The only call site for getContentGlobals used to be in devtools/server/actors/script.js but was removed in https://bugzilla.mozilla.org/show_bug.cgi?id=1039952

(I searched both in mc and addons code)

I will do a try run removing this code and see what happens.

Comment 3

[Julian Descottes [:jdescottes]]

Try seems green: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3864dc81e1063fc35cf26db29cf9f9e2af3d8340 

Alex, do you have any insights about content-globals? As I said getContentGlobals is never used, so I think we should consider removing this completely.

Comment 4

[Alexandre Poirot [:ochameau]]

This is related to Addon-SDK Content scripts. It was to allow debugging them. Seing them in the debugger. I'm not sure this feature still work. May be the platform code involved in bug 1039952 ensure they are still covered. It is possible this code replaces content-globals:
http://searchfox.org/mozilla-central/source/devtools/server/actors/tab.js#533-546

You can see this pattern appear in SDK test:
http://searchfox.org/mozilla-central/source/addon-sdk/source/test/test-page-mod-debug.js#35-37


In any case, I confirm, this code looks useless.
test-page-mode-debug.js be kept, but you can remove the logic around content-globals,
test_devtools_extensions.html could be completely removed with content-globals.js,
sandbox.js should be pruned from content-globals logic

Comment 5

[Patrick Brosset <:pbro>]

Triaging dt-addon blockers as P3/enhancement since we're not using priorities for this project (filter on CLIMBING SHOES).

Comment 6

[Julian Descottes [:jdescottes]]

Attachment: Bug 1359085 - remove unused devtools util content-globals.js;

Review commit: https://reviewboard.mozilla.org/r/135126/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/135126/

Comment 7

[Alexandre Poirot [:ochameau]]

Comment on attachment 8863362 [details]
Bug 1359085 - remove unused devtools util content-globals.js;

https://reviewboard.mozilla.org/r/135126/#review138932

Thanks for this patch!

Comment 8

[Mozilla Autoland]

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 04ac52fd322c -d a86584f1364d: rebasing 393597:04ac52fd322c "Bug 1359085 - remove unused devtools util content-globals.js;r=ochameau" (tip)
merging devtools/shared/tests/mochitest/chrome.ini
warning: conflicts while merging devtools/shared/tests/mochitest/chrome.ini! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment 9

[Julian Descottes [:jdescottes]]

rebased try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=984ad385480d7a9f013b08881438e8445f936d9a

Comment 10

[Julian Descottes [:jdescottes]]

Comment on attachment 8863362 [details]
Bug 1359085 - remove unused devtools util content-globals.js;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/135126/diff/1-2/

Comment 11

[Mozilla Autoland]

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 1d2200ea18a8 -d c8e67ddc6f34: rebasing 393895:1d2200ea18a8 "Bug 1359085 - remove unused devtools util content-globals.js;r=ochameau" (tip)
merging devtools/shared/tests/mochitest/chrome.ini
warning: conflicts while merging devtools/shared/tests/mochitest/chrome.ini! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment 12

[Julian Descottes [:jdescottes]]

Comment on attachment 8863362 [details]
Bug 1359085 - remove unused devtools util content-globals.js;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/135126/diff/2-3/

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d69be43cf123
remove unused devtools util content-globals.js;r=ochameau

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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