451576 - uncaught exceptions on startup

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect, P2)

Description

[Daniel Brooks [:db48x]]

[Exception... "Component returned failure code: 0x80004003 (NS_ERROR_INVALID_POINTER) [nsINavBookmarksService.getBookmarkIdsForURI]"  nsresult: "0x80004003 (NS_ERROR_INVALID_POINTER)"  location: "JS frame :: chrome://browser/content/browser-ui.js :: anonymous :: line 319"  data: no]

and if I bandaid that, I get this:

[Exception... "'[JavaScript Error: "uri is null" {file: "chrome://browser/content/browser-ui.js" line: 343}]' when calling method: [nsIDOMEventListener::handleEvent]"  nsresult: "0x80570021 (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)"  location: "JS frame :: chrome://browser/content/deckbrowser.xml :: selectTab :: line 240"  data: yes]

They're both because browser.currentURI is an empty string.

Comment 1

[Daniel Brooks [:db48x]]

Attachment: 451576-1.diff

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 334907 [details] [diff] [review]
451576-1.diff

The cause of this is the fact that BrowserUI.selectTab is entered very early (from the bottom of deckbrowser.createBrowser

I'd like to make selectTab a bit more robust and not push the defenses down into other methods.

Can we find a way to bail out of selectTab?

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: Alternative patch

Calling SetURI when the browser.currentURI = null seems wrong, so this patch just bails from the function. This is different than Daniel's patch which will try/catch and tweak values.

Not sure which is more appropriate

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 334907 [details] [diff] [review]
451576-1.diff

This patch could still be appropriate

Comment 5

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 334994 [details] [diff] [review]
Alternative patch

When is currentURI ever null? AFAIK that's never the case in /browser's tabbrowser. Do we need to be triggering about:blank loads for new tabs?

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to comment #5)
> (From update of attachment 334994 [details] [diff] [review])
> When is currentURI ever null? AFAIK that's never the case in /browser's
> tabbrowser. Do we need to be triggering about:blank loads for new tabs?

The very first call to deckbrowser.newTab(), called from BrowserUI.init() - this ends up firing the "TabSelect" event from inside deckbrowser, which ends up calling BrowserUI.setURI()

I don't think tabbrowser fires "TabSelect" for new tabs. It fires "TabOpen" only.

Comment 7

[:Gavin Sharp [email: [e-mail]]]

Seems like we should be setting up deckbrowser to have a blank tab by default, like tabbrowser, and avoid calling newTab during startup.

Comment 8

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 334994 [details] [diff] [review]
Alternative patch

...but we can take this for now, I guess. Add a FIXME comment and file a followup?

Comment 9

[Mark Finkle (:mfinkle) (use needinfo?)]

filed bug 454028 as a followup.

changeset:   123:12dffbaf540f
tag:         tip
user:        Mark Finkle <[e-mail]>
date:        Sat Sep 06 23:06:04 2008 -0500
summary:     b=451576, r=gavin. uncaught exceptions on startup

Comment 10

[Joel Maher ( :jmaher ) (UTC -8)]

verfied with beta3