Closed Bug 624017 Opened 9 years ago Closed 9 years ago

Move more JavaScript into delay-loaded files

Categories

(Firefox for Android Graveyard :: General, defect)

defect
Not set

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: mbrubeck, Assigned: mbrubeck)

Details

(Keywords: perf)

Attachments

(1 file, 1 obsolete file)

Attached patch patch (obsolete) — Splinter Review
This moves several JavaScript objects from common-ui.js into individual files, reducing the size of common-ui.js by 800 lines.

This patch only affects objects that are loaded after startup.  The remaining objects in common-ui.js are all loaded at startup (during UIReadyDelayed), so there's no performance gain from moving them to separate files.

The attached patch is pure copy/paste, with no modifications in the JavaScript files themselves.
Attachment #502104 - Flags: review?(mark.finkle)
Attached patch patch v2Splinter Review
Forgot to update one line when I changed the file naming.
Attachment #502104 - Attachment is obsolete: true
Attachment #502107 - Flags: review?(mark.finkle)
Attachment #502104 - Flags: review?(mark.finkle)
Comment on attachment 502107 [details] [diff] [review]
patch v2

>+    {
>+      name: "Google Reader",
>+      callback: function callback(aURL, aTitle) {
>+        let url = "http://www.google.com/reader/link?url=" + encodeURIComponent(aURL) +
>+                  "&title=" + encodeURIComponent(aTitle);
>+        BrowserUI.addTab(url, Browser.selectedTab);
>+      }

Change to BrowserUI.newTab(url, Browser.selectedTab);

(I think this bug was filed somewhere)

I'm not the biggest fan of separate files, but it might be better for performance. I guess we'll find out.

We can always re-org later as needed.
Attachment #502107 - Flags: review?(mark.finkle) → review+
http://hg.mozilla.org/mobile-browser/rev/f8add7971e4b
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
I could only find this data about ts, to see if this patch could have influenced ts:
http://graphs.mozilla.org/graph.html#tests=[[16,11,621]]&sel=1294157766,1294675722
Perhaps it did, but I have no idea, really.
Is there any other way to verify this bug other than code inspection?
(In reply to comment #5)
> Is there any other way to verify this bug other than code inspection?

No, this change should be transparent to users (except for possible Ts improvement).
Then can somebody please verify this?
You need to log in before you can comment on or make changes to this bug.