491083 - Avoid null storage exceptions

Status: RESOLVED FIXED

(Firefox :: Session Restore, defect)

Description

[John J. Barton]

I am running FF 3.6 built via
hg up edbe350e1d81

Line 1287 of nsSessionStore.js give an exception because storage is null.

---
try {
        storage = aDocShell.getSessionStorageForURI(uri);
        storageItemCount = storage.length;   
      }
      catch (ex) { /* sessionStorage might throw if it's turned off, see bug 458954 */ }
----
should be
----
try {
        storage = aDocShell.getSessionStorageForURI(uri);
        if (storage)
            storageItemCount = storage.length; 
        else 
            return;
      }
      catch (ex) { /* sessionStorage might throw if it's turned off, see bug 458954 */ }
----

I also do not understand why the catch block falls through. If the storage turned off, return you are done.

Comment 1

[Simon Bünzli]

(In reply to comment #0)
> Line 1287 of nsSessionStore.js give an exception because storage is null.

An exception which is immediately caught and discarded. What's the issue?

>         storage = aDocShell.getSessionStorageForURI(uri);
>         if (storage)
>             storageItemCount = storage.length; 
>         else 
>             return;

Actually, storage can be null for various reasons. IIRC we stumbled over this when passing in an "about:" uri. In that case, returning would be wrong.

Comment 2

[John J. Barton]

(In reply to comment #1)
> An exception which is immediately caught and discarded. What's the issue?

The exception is thrown, enters the error handling code, sees that an error handler from chromebug is installed, issues a error message, returns through the error handler, enter the catch block....

So my issue is that I trap on errors to improve my code.
Your issue could be performance.

> 
> Actually, storage can be null for various reasons. IIRC we stumbled over this
> when passing in an "about:" uri. In that case, returning would be wrong.

How can continuing with a null storage object be right?

Comment 3

[Simon Bünzli]

(In reply to comment #2)
> So my issue is that I trap on errors to improve my code.

So, will you start filing bugs against all try-catch blocks or just the ones where you happen to need the catch yourself? ;-)

> Your issue could be performance.

Feel free to post a patch. You should still be able to make Firefox 3.5.

> How can continuing with a null storage object be right?

If you read on, you'll note that the very next thing we do is |continue| the loop from the start (and thus potentially getting a different storage object) if there's either no storage at all or when it's empty.

Comment 4

[Simon Bünzli]

FIXED in bug 494543.