547406 - Port Bug 485976 [Move writing sessionstore.js off the main thread] to SeaMonkey

Status: RESOLVED FIXED

(SeaMonkey :: Session Restore, defect)

Description

[Misak Khachatryan]

Attachment: patch

From parent bug:

Right now as far as I can tell session store writes data to disk synchronously
on the UI thread.  It might make sense to move this to a worker thread.

Comment 1

[[e-mail]]

Comment on attachment 427927 [details] [diff] [review]
patch

>+__defineGetter__("NetUtil", function() {
>+  delete this.NetUtil;
>+  Components.utils.import("resource://gre/modules/NetUtil.jsm");
>+  return NetUtil;
>+});
Don't bother with this; just import NetUtil after XPCOMUtils. (Unless for some strange reason you want to reimplement asyncCopy!)

>-
>+    // Asynchronously copy the data to the file.
Nit: don't delete that blank line

>+    var self = this;
>+    NetUtil.asyncCopy(istream, ostream, function(rc) {
>+      if (Components.isSuccessCode(rc)) {
>+        self._observerService.notifyObservers(null,
>+                                              "sessionstore-state-write-complete",
>+                                              "");
Nit: you only need a temporary for the observer service, not for this.
[I blame the callback mechanism for forcing this awkward workaround.]

Comment 2

[Misak Khachatryan]

I got error in console when used

        this._observerService.notifyObservers(null,


Error: this._observerService is undefined
Source File: file:///home/misak/workspace/src/suite-opt/mozilla/dist/bin/components/nsSessionStore.js
Line: 2686

Comment 3

[[e-mail]]

That's why you need a temporary for the observer service itself. (You only removed the "self" temporary for "this", which obviously doesn't work.)

Comment 4

[Misak Khachatryan]

Attachment: v2

Nits fixed, carrying forward r+ from Neil.

Comment 5

[[e-mail]]

Comment on attachment 428143 [details] [diff] [review]
v2

Almost there, but

>+    var self = this._observerService;
you need a new name for this variable

>+        self.notifyObservers(null,
>+                                              "sessionstore-state-write-complete",
>+                                              "");
and you need to realign these.

Comment 6

[Misak Khachatryan]

Attachment: final patch [Checkin: Comment 7]

renamed variable, carrying forward r+ from Neil.

Comment 7

[Serge Gautherie (:sgautherie)]

Comment on attachment 428177 [details] [diff] [review]
final patch
[Checkin: Comment 7]


http://hg.mozilla.org/comm-central/rev/37123caa84ab