This is similar to bug 722661, but we can also stub the initial tab (about:home or the external URL being loaded) even if we aren't doing a session restore. This would also have the side effect of fixing bug 799977. Bug 799977 happens when clicking a "tabs from last time" entry before Gecko has loaded, and it fails when calling Tabs.getSelectedTab() (since there are no tabs at the time). Creating an about:home stub guarantees that a tab will exist here.

This patch removes some of the special case code we had for handling the initial tab at startup. Here's some explanations since some of these changes aren't obvious: > - passedUri = "about:empty"; > - if (url == "about:empty") > - loadParams.flags = Ci.nsIWebNavigation.LOAD_FLAGS_BYPASS_HISTORY; These changes revert bugs 743415 and 735838. We're triggering these tab loads directly from Java, so we shouldn't need to worry about the "about:home or null" weirdness described in https://bugzilla.mozilla.org/show_bug.cgi?id=735838#c5. > - if (!browser) > - return; If we're calling Tab:Load to create the initial tab, we can't have this check at the top of the observer since we won't have a selected browser yet. I don't think this check is necessary since we grey out all menu items that require a tab if we haven't received a Gecko:Ready yet. I've verified that this fixes bug 799977.

Comment on attachment 670197 [details] [diff] [review] Stub initial tab before Gecko starts >diff --git a/mobile/android/base/GeckoApp.java b/mobile/android/base/GeckoApp.java >+ // Tabs for external URLs are created in BrowserApp's initializeChrome(). >+ // Create them here if this is a webapp. >+ if (action != null && action.startsWith(GeckoApp.ACTION_WEBAPP_PREFIX)) { >+ Tabs.getInstance().loadUrl(uri, Tabs.LOADURL_NEW_TAB | Tabs.LOADURL_USER_ENTERED | Tabs.LOADURL_PINNED); >+ } Couldn't we move this to WebApp ? If so, file a followup. >diff --git a/mobile/android/chrome/content/browser.js b/mobile/android/chrome/content/browser.js >- if (url == "about:empty") >- loadParams.flags = Ci.nsIWebNavigation.LOAD_FLAGS_BYPASS_HISTORY; This makes me nervous. Let's really make sure we don't regress bug 743415 Overall I think the cleanup is good and hopefully the UI acts more responsive during startup

> >+ // Tabs for external URLs are created in BrowserApp's initializeChrome(). > >+ // Create them here if this is a webapp. > >+ if (action != null && action.startsWith(GeckoApp.ACTION_WEBAPP_PREFIX)) { > >+ Tabs.getInstance().loadUrl(uri, Tabs.LOADURL_NEW_TAB | Tabs.LOADURL_USER_ENTERED | Tabs.LOADURL_PINNED); > >+ } > > Couldn't we move this to WebApp ? If so, file a followup. Oops, thanks. It's not a big change, so I'll just do that now.

Added missing call to super method.

Comment on attachment 670858 [details] [diff] [review] Stub initial tab before Gecko starts, v3 >diff --git a/mobile/android/base/GeckoApp.java b/mobile/android/base/GeckoApp.java >- void initializeChrome(String uri, Boolean isExternalURL) { >+ protected void initializeChrome(String uri, Boolean isExternalURL) { > mDoorHangerPopup = new DoorHangerPopup(this, null); > } Not your problem, but the way we initialize mDoorHangerPopup here and in BrowserApp.initializeChrome, but not in WebApp.initializeChrome is weird and error-prone. We should file a bug to clean it up.

(In reply to Mark Finkle (:mfinkle) from comment #5) > Comment on attachment 670858 [details] [diff] [review] > Stub initial tab before Gecko starts, v3 > > >diff --git a/mobile/android/base/GeckoApp.java b/mobile/android/base/GeckoApp.java > > >- void initializeChrome(String uri, Boolean isExternalURL) { > >+ protected void initializeChrome(String uri, Boolean isExternalURL) { > > mDoorHangerPopup = new DoorHangerPopup(this, null); > > } > > Not your problem, but the way we initialize mDoorHangerPopup here and in > BrowserApp.initializeChrome, but not in WebApp.initializeChrome is weird and > error-prone. We should file a bug to clean it up. We don't override initializeChrome in WebApp.java, so GeckoApp takes care of it for us. We just override it in BrowserApp because we want to pass in an extra parameter for the anchor.

(In reply to Margaret Leibovic [:margaret] from comment #6) > (In reply to Mark Finkle (:mfinkle) from comment #5) > > Comment on attachment 670858 [details] [diff] [review] > > Stub initial tab before Gecko starts, v3 > > > > >diff --git a/mobile/android/base/GeckoApp.java b/mobile/android/base/GeckoApp.java > > > > >- void initializeChrome(String uri, Boolean isExternalURL) { > > >+ protected void initializeChrome(String uri, Boolean isExternalURL) { > > > mDoorHangerPopup = new DoorHangerPopup(this, null); > > > } > > > > Not your problem, but the way we initialize mDoorHangerPopup here and in > > BrowserApp.initializeChrome, but not in WebApp.initializeChrome is weird and > > error-prone. We should file a bug to clean it up. > > We don't override initializeChrome in WebApp.java, so GeckoApp takes care of > it for us. We just override it in BrowserApp because we want to pass in an > extra parameter for the anchor. Exactly. That is too fragile. I think we should consider making GeckoApp be able to handle a null mDoorHangerPopup and move all door hanger popup initialization to subclasses (BrowserApp and WebApp) which makes things seem more sane.

(In reply to Mark Finkle (:mfinkle) from comment #7) > (In reply to Margaret Leibovic [:margaret] from comment #6) > > (In reply to Mark Finkle (:mfinkle) from comment #5) > > > Comment on attachment 670858 [details] [diff] [review] > > > Stub initial tab before Gecko starts, v3 > > > > > > >diff --git a/mobile/android/base/GeckoApp.java b/mobile/android/base/GeckoApp.java > > > > > > >- void initializeChrome(String uri, Boolean isExternalURL) { > > > >+ protected void initializeChrome(String uri, Boolean isExternalURL) { > > > > mDoorHangerPopup = new DoorHangerPopup(this, null); > > > > } > > > > > > Not your problem, but the way we initialize mDoorHangerPopup here and in > > > BrowserApp.initializeChrome, but not in WebApp.initializeChrome is weird and > > > error-prone. We should file a bug to clean it up. > > > > We don't override initializeChrome in WebApp.java, so GeckoApp takes care of > > it for us. We just override it in BrowserApp because we want to pass in an > > extra parameter for the anchor. > > Exactly. That is too fragile. I think we should consider making GeckoApp be > able to handle a null mDoorHangerPopup and move all door hanger popup > initialization to subclasses (BrowserApp and WebApp) which makes things seem > more sane. Looks like we already do null checks whenever mDoorHangerPopup is used. I filed bug 801739.

Backed out for Android failures: http://hg.mozilla.org/integration/mozilla-inbound/rev/3ff5410852cc

This failed on TBPL because we create our telemetry prompt - which requires a tab - in startup(). This patch creates the initial tab immediately after startup(), so we need to delay showing the telemetry prompt until then. I also addressed comment 5 with this patch by calling mDoorHangerPopup.setAnchor() in the overridden initializeChrome(), allowing it to call the super method.