758812 - Break up browser.js into smaller pieces, Phase 1

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Justin Dolske [:Dolske]]

It is widely acknowledged that browser.js is too damn bug. The in-tree version is currently 9431 lines long. (It's actually 13,193 lines if you count the 6 files that are already #included)

The first step to fixing our problem is to blast out some of the big and obvious chunks into separate files and #include them. If nothing else, this starts to make looking at code history easier (because history for browser-foo.js will be separate from browser-bar.js).

My plan is to start with splitting out things like "var gFoo = { ... }" in a few steps. Later on, when the dust settles, we can look at moving related global functions into their own files as well.

Comment 1

[Justin Dolske [:Dolske]]

Attachment: Patch for gPluginHandler

(I'm only going to set the review flag on this first patch, because multiple flags are so annoying. The intention is for all to be reviewed. Note that there are no code changes here -- just moving chunks and adding #includes)

Comment 2

[Justin Dolske [:Dolske]]

Attachment: Patch for LightWeightThemeInstaller

Comment 3

[Justin Dolske [:Dolske]]

Attachment: Patch for FeedHandler

Comment 4

[Justin Dolske [:Dolske]]

Attachment: Patch for FullScreen and feeds

Comment 5

[Justin Dolske [:Dolske]]

Attachment: Patch for 2 addon things

Comment 6

[Justin Dolske [:Dolske]]

Comment on attachment 627397 [details] [diff] [review]
Patch for FeedHandler

Oops, I seem to have merged the FeedHandler and FullScreen changes. Meh.

Comment 7

[Justin Dolske [:Dolske]]

I did a try run just to make sure nothing unexpected broke. It's ok.

https://tbpl.mozilla.org/?tree=Try&rev=532c60405f19

It also occurred to me that browser-lwt.js could be grouped into browser-addons.js.

Comment 9

[Dão Gottwald [:dao]]

Any reason why LightWeightThemeInstaller isn't in browser-addons.js?

Comment 10

[:Gavin Sharp [email: [e-mail]]]

I don't feel strongly; it seems pretty self-contained and large enough to merit its own file.

Comment 11

[Dão Gottwald [:dao]]

Well, gXPInstallObserver and AddonsMgrListener are similarly self-contained.

Comment 12

[Justin Dolske [:Dolske]]

(In reply to Dão Gottwald [:dao] from comment #9)
> Any reason why LightWeightThemeInstaller isn't in browser-addons.js?

From comment 7:

> It also occurred to me that browser-lwt.js could be grouped into browser-addons.js.

I was just going to do that for checkin.

Comment 13

[Justin Dolske [:Dolske]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2ce67e30d650
https://hg.mozilla.org/integration/mozilla-inbound/rev/cc6ca4cba764
https://hg.mozilla.org/integration/mozilla-inbound/rev/9bb221edc54e
https://hg.mozilla.org/integration/mozilla-inbound/rev/729a6b7922cd

Comment 14

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/729a6b7922cd
https://hg.mozilla.org/mozilla-central/rev/9bb221edc54e
https://hg.mozilla.org/mozilla-central/rev/cc6ca4cba764
https://hg.mozilla.org/mozilla-central/rev/2ce67e30d650

Comment 15

[Eric Shepherd [:sheppy]]

This will need to be mentioned to devs that poke around within the product; most already know, but it's still worth noting on Firefox 15 for developers.

Comment 16

[Justin Dolske [:Dolske]]

Sheppy: doesn't hurt, but I'd be careful to note that there's essentially no change to the browser.js that _ships_ with Firefox, it's only the files as they live in mozilla-central that have been split. (The magic of #include.)