Last Comment Bug 691534 - Use asyncFetch when loading files in aboutHome.xhtml
: Use asyncFetch when loading files in aboutHome.xhtml
Status: RESOLVED FIXED
[mobilestartupshrink][QA?]
: perf
Product: Fennec Graveyard
Classification: Graveyard
Component: General (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Firefox 10
Assigned To: Mark Finkle (:mfinkle) (use needinfo?)
:
:
Mentors:
Depends on: 692767
Blocks:
  Show dependency treegraph
 
Reported: 2011-10-03 14:57 PDT by Mark Finkle (:mfinkle) (use needinfo?)
Modified: 2011-11-08 17:30 PST (History)
4 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch (10.59 KB, patch)
2011-10-03 14:57 PDT, Mark Finkle (:mfinkle) (use needinfo?)
mbrubeck: review+
asa: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Mark Finkle (:mfinkle) (use needinfo?) 2011-10-03 14:57:37 PDT
Created attachment 564345 [details] [diff] [review]
patch

We are actually using sync file loading in aboutHome.xhtml - CRAZY!

This patch switches us to use asyncFetch to load the JSON data files. The patch also refactors the code to use a single _readFile method.

This should make trying to interact with Fennec easier _right_ after the home page is loaded.
Comment 1 Matt Brubeck (:mbrubeck) 2011-10-03 15:51:12 PDT
Comment on attachment 564345 [details] [diff] [review]
patch

r=mbrubeck with some minor nits:

>+    function _readFile(aFile, aCallback) {
>       try {

You can probably remove the try/catch

>+        let channel = NetUtil.newChannel(aFile);
>+        channel.contentType = "application/json";
>+        NetUtil.asyncFetch(channel, function(aStream, aResult) {
>+          if (!Components.isSuccessCode(aResult)) {
>+            Cu.reportError("AboutHome: Could not read from " + aFile.leafName);
>+            aCallback(null);
>+            return;
>+          }
> 
>+          let content = NetUtil.readInputStreamToString(aStream, aStream.available()) || "";
>+          aStream.close();
>+
>+          aCallback(content.replace(/\r\n?/g, "\n"));
>+          return;

No need for explicit "return" here.

>+        });
>       }
>       catch (ex) { Cu.reportError(ex); }
> 
>-      return null;
>+      aCallback(null);
>     }

Remove the aCallback(null) here.

>       loadFromCacheOrScheduleUpdate: function(aDelay) {
>+        let self = this;
>         let file = this._getFile();
>         if (file.exists()) {
>+          _readFile(file, function(aContent) {
>+            let json = JSON.parse(aContent);
>+            if (!json || json.addons.length == 0) {
>+              //noRecentTabs();

Remember to remove the commented-out line above.
Comment 2 Mark Finkle (:mfinkle) (use needinfo?) 2011-10-03 20:12:43 PDT
Made changes

https://hg.mozilla.org/integration/mozilla-inbound/rev/26d62a2d24cb
Comment 3 Marco Bonardo [::mak] 2011-10-04 02:09:04 PDT
https://hg.mozilla.org/mozilla-central/rev/26d62a2d24cb
Comment 4 Mark Finkle (:mfinkle) (use needinfo?) 2011-10-04 13:48:31 PDT
Comment on attachment 564345 [details] [diff] [review]
patch

Cheap, local fix for reading two data files using async methods. Should reduce the "stall" that can occur after loading the phone page on phones with slow file systems (still looking at you Galaxy S)
Comment 5 Matt Brubeck (:mbrubeck) 2011-10-07 10:41:21 PDT
We should hold off pushing this to Aurora unless bug 692767 is also fixed and approved for Aurora.
Comment 6 Mark Finkle (:mfinkle) (use needinfo?) 2011-10-13 06:34:48 PDT
https://hg.mozilla.org/releases/mozilla-aurora/rev/8b1171944849

Note You need to log in before you can comment on or make changes to this bug.