530354 - No need to check for navigator:browser before initializing SessionStore

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

In browser.js (delayedStartup) we initialize SessionStore & init a window only if windowtype is navigator:browser. We don't need to do that. Perhaps this is just left over from a time we did? Or maybe a Seamonkey compatibility thing.

Comment 1

[Michael Kohler [:mkohler]]

Attachment: patch v1

I don't know if this is a Seamonkey compatibility thing, so before we land the patch, we should be sure. Otherwise we'd perhaps break Seamonkey.

Comment 2

[Dão Gottwald [:dao]]

Comment on attachment 413925 [details] [diff] [review]
patch v1

>-  // initialize the session-restore service (in case it's not already running)
>+  // initialize the session-restore service

What was wrong with the comment as it was? I think this isn't really needed for subsequent windows, so the comment seems correct.

>+  try {
>+    var ss = Cc["@mozilla.org/browser/sessionstore;1"].
>+             getService(Ci.nsISessionStore);
>+    ss.init(window);

I'd prefer this:

Cc["@mozilla.org/browser/sessionstore;1"]
  .getService(Ci.nsISessionStore)
  .init(window);

>+  } catch(ex) {

nit: catch (

Comment 3

[Michael Kohler [:mkohler]]

Attachment: patch v2

nits addressed

Comment 4

[Phil Ringnalda (:philor)]

http://hg.mozilla.org/mozilla-central/rev/10c98bccd40b