keeleysam / tenfourfox

Automatically exported from code.google.com/p/tenfourfox
0 stars 0 forks source link

Async session restore problems in 20 #210

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
We have no copyfile(3), and the JS copyfile fallback in osfile_unix_front.js is 
slow. This was faster, even if it was synchronous:

     this._restorePinnedTabsOnDemand =
       this._prefBranch.getBoolPref("sessionstore.restore_pinned_tabs_on_demand");
     this._prefBranch.addObserver("sessionstore.restore_pinned_tabs_on_demand", this, true);

-    // get file references
-    this._sessionFile = Services.dirsvc.get("ProfD", Ci.nsILocalFile);
-    this._sessionFileBackup = this._sessionFile.clone();
-    this._sessionFile.append("sessionstore.js");
-    this._sessionFileBackup.append("sessionstore.bak");

...

     if (this._resume_from_crash) {
-      // create a backup if the session data file exists
-      try {
-        if (this._sessionFileBackup.exists())
-          this._sessionFileBackup.remove(false);
-        if (this._sessionFile.exists())
-          this._sessionFile.copyTo(null, this._sessionFileBackup.leafName);
-      }
-      catch (ex) { Cu.reportError(ex); } // file was write-locked?
+      // Launch background copy of the session file. Note that we do
+      // not have race conditions here as _SessionFile guarantees
+      // that any I/O operation is completed before proceeding to
+      // the next I/O operation.
+      _SessionFile.createBackupCopy();
     }

The copy does work either way. However, the browser then isn't reading the 
file. The copy is good because if I restore the backup and launch 19, then the 
tabs do reappear.

At some irregular intervals this will succeed for 20, but I can't figure out 
why.

Original issue reported on code.google.com by classi...@floodgap.com on 5 Mar 2013 at 5:18

GoogleCodeExporter commented 9 years ago
Maybe make _SessionFile always do a syncRead?

Original comment by classi...@floodgap.com on 5 Mar 2013 at 5:20

GoogleCodeExporter commented 9 years ago
The browser actually is saving the state; if I copy the .bak over .js, it works 
in 19, but it won't work in 20.

Original comment by classi...@floodgap.com on 6 Mar 2013 at 3:32

GoogleCodeExporter commented 9 years ago
I don't like this fix, but throwing in a this._ensureInitialized() before 
SessionFile.read().then(...) makes everything work. Essentially it makes the 
whole thing synchronous as before.

I wonder if there is an OS issue with threads in 10.4. Keeping this open until 
I have a better idea how to fix it.

Might as well speed up the session file too.

Original comment by classi...@floodgap.com on 6 Mar 2013 at 3:48

GoogleCodeExporter commented 9 years ago
AuroraFox 20 suffers the same problem.

Original comment by Tobias.N...@gmail.com on 6 Mar 2013 at 7:21

GoogleCodeExporter commented 9 years ago
Does the same fix repair it? I'm thinking we have a race condition in here, 
either OS X or due to the slower system speed. I'm not sure of the underlying 
cause though.

I'm still dithering over what to do with copyfile(), but that doesn't apply to 
10.5. The pure-JS version is clearly too slow, however.

Original comment by classi...@floodgap.com on 6 Mar 2013 at 4:03

GoogleCodeExporter commented 9 years ago
Decided to go with the older native approach.

Leaving open until we find a better solution or give up.

Original comment by classi...@floodgap.com on 10 Mar 2013 at 11:54

GoogleCodeExporter commented 9 years ago
Temporarily had to revert for 24 since this code changed quite a bit.

Original comment by classi...@floodgap.com on 9 Jul 2013 at 2:02

GoogleCodeExporter commented 9 years ago
Fixed by Mozilla in Fx28. However, see the remaining note in 
nsSessionStartup.js in 10.4Fx 29 and 
https://bugzilla.mozilla.org/show_bug.cgi?id=952224. 

Wontfix for 24.

Original comment by classi...@floodgap.com on 19 Apr 2014 at 9:16