1250832 - Remove devtools globals from browser.xul

Status: RESOLVED FIXED

(DevTools :: General, defect, P2)

Description

[Alexandre Poirot [:ochameau]]

Here is the current list of globals:

gDevTools, gDevToolsBrowser, HUDService:
http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#7544

ScratchPad, ResponsiveUI, EyeDropper:
http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#7616

DeveloperToolbar, BrowserToolboxProcess:
http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#160

Most of them are used to implement hardcoded menus.
Once we insert menus dynamically (bug 124860), we won't need these globals.
None of browser code actually use any of these globals.
But I would have to check for any of them if some addons are using them.
So we will most likely keep some for compatiblity, like gDevTools.

Comment 1

[Alexandre Poirot [:ochameau]]

I opened bug 1250833 in order to stop using these globals from devtools codebase.

Comment 2

[Alexandre Poirot [:ochameau]]

Attachment: patch v1

Comment 3

[Alexandre Poirot [:ochameau]]

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

Comment 4

[Alexandre Poirot [:ochameau]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0813c2922a34

Comment 5

[Alexandre Poirot [:ochameau]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=11d3a3a72030

Comment 6

[Alexandre Poirot [:ochameau]]

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

Comment 7

[Alexandre Poirot [:ochameau]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1c9c59f72b81

Comment 8

[Alexandre Poirot [:ochameau]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9ae1599eaa70

Comment 9

[Alexandre Poirot [:ochameau]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6862b94fbc6c

Comment 10

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

The `ResponsiveUI` bit can surely be removed now, since the lazy getter was already removed in bug 1399449.

Comment 11

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1250832 - Remove DevTools globals from browser.xul scope.

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

Comment 12

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

Comment on attachment 8911806 [details]
Bug 1250832 - Remove DevTools globals from browser.xul scope.

https://reviewboard.mozilla.org/r/183232/#review188466

Thanks for the cleanup! :)

::: browser/base/content/browser.js:8562
(Diff revision 1)
>        gBrowser.selectedTab = newTab;
>        break;
>    }
>  }
>  
>  var Scratchpad = {

I believe this `var Scratchpad` should also be removed.

Comment 13

[Alexandre Poirot [:ochameau]]

Comment on attachment 8911806 [details]
Bug 1250832 - Remove DevTools globals from browser.xul scope.

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

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/418e79946237
Remove DevTools globals from browser.xul scope. r=jryans

Comment 15

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for failing devtools/client/commandline/test/browser_cmd_csscoverage_startstop.js on Windows 7 debug:

https://hg.mozilla.org/integration/autoland/rev/d4a29741c62807f145203dc5689cb74bfbd83daa

Push which ran failing test: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=f8b6f4681fb8830cbdb904c0ee77313ebfebc31e&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=133305665&repo=autoland
> TEST-UNEXPECTED-FAIL | devtools/client/commandline/test/browser_cmd_csscoverage_startstop.js | Uncaught exception - at chrome://mochitests/content/browser/devtools/client/commandline/test/browser_cmd_csscoverage_startstop.js:53 - ReferenceError: gDevTools is not defined

Comment 16

[Alexandre Poirot [:ochameau]]

Comment on attachment 8911806 [details]
Bug 1250832 - Remove DevTools globals from browser.xul scope.

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

Comment 17

[Alexandre Poirot [:ochameau]]

Green try on windows:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a5c3702943e515f7a9f621f06549752f8b8dd913

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5ab65f749652
Remove DevTools globals from browser.xul scope. r=jryans

Comment 19

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

https://hg.mozilla.org/mozilla-central/rev/5ab65f749652