Closed Bug 726930 Opened 8 years ago Closed 8 years ago

speed up tab thumbnails

Categories

(Firefox for Android :: General, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 13

People

(Reporter: blassey, Assigned: blassey)

Details

Attachments

(1 file, 2 obsolete files)

Attached patch patch (obsolete) — Splinter Review
Here this patch has a couple improvements over the existing code. First it eliminates a bunch of string comparisons by not using the broadcast event. Next, it eliminates string manipulations and parsing of JSON data. Finally, by explicitly allocating and free'ing the buffer it allows us to copy the data out of the buffer on the background thread.
Attachment #596939 - Flags: review?(mark.finkle)
Attached patch patch, with comments (obsolete) — Splinter Review
Assignee: nobody → blassey.bugs
Attachment #596939 - Attachment is obsolete: true
Attachment #596939 - Flags: review?(mark.finkle)
Attachment #597136 - Flags: review?(mark.finkle)
Attached patch patchSplinter Review
that was the wrong patch for this bug
Attachment #597136 - Attachment is obsolete: true
Attachment #597136 - Flags: review?(mark.finkle)
Attachment #597171 - Flags: review?(mark.finkle)
Comment on attachment 597171 [details] [diff] [review]
patch


>diff --git a/mobile/android/chrome/content/browser.js b/mobile/android/chrome/content/browser.js
>--- a/mobile/android/chrome/content/browser.js
>+++ b/mobile/android/chrome/content/browser.js
>@@ -202,11 +202,12 @@ var BrowserApp = {
> 
>     getBridge().setDrawMetadataProvider(MetadataProvider);
> 
>+    getBridge().registerBrowserFrontend(this);
>+

Let's use an attribute-based style, like the nsIBrowserDOMWindow is used

getBridge().browserApp = this;

>+  getWindowForTab: function(tabId) {
>+      let tab = this.getTabForId(tabId);
>+      if (!tab.browser)
>+	  return null;
>+      return tab.browser.contentWindow;
>+  }, 

Add this at the bottom and use a comment above it like:

// nsIAndroidBrowserApp

>diff --git a/widget/android/AndroidBridge.cpp b/widget/android/AndroidBridge.cpp

>+/* void registerBrowserFrontend (in nsIBrowserFrontend aBrowserFrontend); */
>+NS_IMETHODIMP nsAndroidBridge::RegisterBrowserFrontend(nsIBrowserFrontend *aBrowserFrontend)
>+{
>+    if (nsAppShell::gAppShell)
>+        nsAppShell::gAppShell->SetBrowserFrontend(aBrowserFrontend);
>+    return NS_OK;
>+}

Changing to an attribute, this becomes: SetBrowserApp(nsIAndroidBrowserApp* aBrowserApp)

>diff --git a/widget/android/nsIAndroidBridge.idl b/widget/android/nsIAndroidBridge.idl

>+[scriptable, uuid(d10377b4-1c90-493a-a532-63cb3f16ee2b)]
>+interface nsIBrowserFrontend : nsISupports {
>+  nsIDOMWindow getWindowForTab(in PRInt32 tabId);
>+};

nsIBrowserFrontend -> nsIAndroidBrowserApp

> [scriptable, uuid(7dd8441a-4f38-49b2-bd90-da69d02a96cf)]
> interface nsIAndroidBridge : nsISupports
> {

>+  void registerBrowserFrontend(in nsIBrowserFrontend aBrowserFrontend);

Since this is a single use interface, let's use an attribute:

attribute nsIAndroidBrowserApp browserApp;

See: http://mxr.mozilla.org/mozilla-central/source/dom/interfaces/base/nsIDOMChromeWindow.idl#61

r+ with those changes
Attachment #597171 - Flags: review?(mark.finkle) → review+
https://hg.mozilla.org/mozilla-central/rev/40657c58d809
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 13
You need to log in before you can comment on or make changes to this bug.