There's a few different cases where we want to be able to create a new tab in Java (and have the UI updated accordingly) before a message is received from Gecko. We can accomplish this by stubbing tabs when we create them.

This patch cleans up the following: - consolidates Tab:Add and Tab:Load into a single message (Tab:Load) - factors out the new tab logic into a single function (needed for bug 797075) - moves loadUrl() and loadUrlInTab() to Tabs.java to remove some references to mAppContext One thing this patch is missing is setting the title of the tab immediately after loadUrl() is called, but this is fixed with the tab stubs in part 2.

Stubs tabs before receiving Tab:Added from Gecko. The easiest way to do this was to use negative IDs for stubbed tabs so that we could still use the mTabs id->tab mapping.

Comment on attachment 667205 [details] [diff] [review] Part 1: loadUrl() cleanup >+ public void loadUrl(String url, boolean newTab) { >+ loadUrl(url, newTab, null, false, -1); >+ } >+ >+ /** >+ * Loads a tab with the given URL. >+ * >+ * @param url URL of page to load, or search term used if searchEngine is given >+ * @param newTab if true, a new tab will be opened; otherwise, the selected tab is used >+ * @param searchEngine if given, the search engine with this name is used >+ * to search for the url string; if null, the URL is loaded directly >+ * @param userEntered whether the user manually typed the URL (used for disabling javascript: URIs) >+ * @param parentId ID of this tab's parent, or -1 if it has no parent >+ */ >+ public void loadUrl(String url, boolean newTab, String searchEngine, boolean userEntered, int parentId) { I like almost everything about this patch except the booleans used for newTab and userEntered, although newTab put me over the edge. Can we use integer flags for this? Create a flags param to the methods and Tabs.LOADURL_NEW_TAB and Tabs.LOADURL_USER_ENTERED flag values? I based the names on the AWESOMEBAR values I saw.

Changed to use flags.

Comment on attachment 667319 [details] [diff] [review] Part 1: loadUrl() cleanup, v2 Nice refactor

This patch uses a different approach, which stores the tab counter in Java instead of Gecko. This allows Java to simply pass along the tab ID when creating a tab without any special stubbing logic. When creating a tab in Gecko, we can use JNI to pull the next ID.

http://hg.mozilla.org/integration/mozilla-inbound/rev/454ec577c741 http://hg.mozilla.org/integration/mozilla-inbound/rev/c644dacaf9a9

Backed out for Android test failures: https://hg.mozilla.org/integration/mozilla-inbound/rev/0c5a32305677

This popped up in most of the failed tests on TBPL: 10-05 16:32:02.800 W/dalvikvm( 1712): JNI WARNING: illegal class name 'org.mozilla.gecko.Tabs' (Check_FindClass) 10-05 16:32:02.800 W/dalvikvm( 1712): (should be formed like 'java/lang/String') It looks like "/", not ".", should be used as the delimiter for findClass() strings. This change passes try: https://tbpl.mozilla.org/?tree=Try&rev=0a2b9242933b I filed bug 799015 to fix our other findClass() call.

Comment on attachment 669024 [details] [diff] [review] Part 2: Implement Java-side tab stubs, v3 >+ let jni = new JNI(); >+ let cls = jni.findClass("org/mozilla/gecko/Tabs"); >+ let method = jni.getStaticMethodID(cls, "getNextTabId", "()I"); >+ this.id = jni.callStaticIntMethod(cls, method); >+ jni.close(); I wonder if it would be worth create a long-lived function for this, instead of creating the JNI stuff each time. Could be a followup bug

http://hg.mozilla.org/integration/mozilla-inbound/rev/3e0deee3c6fd http://hg.mozilla.org/integration/mozilla-inbound/rev/b4989f34fdf7

https://hg.mozilla.org/mozilla-central/rev/3e0deee3c6fd https://hg.mozilla.org/mozilla-central/rev/b4989f34fdf7