Closed Bug 797075 Opened 8 years ago Closed 8 years ago
Implement Java-side tab stubs
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.
Attachment #667205 - Flags: review?(mark.finkle)
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.
Attachment #667205 - Flags: review?(mark.finkle) → review-
Changed to use flags.
Comment on attachment 667319 [details] [diff] [review] Part 1: loadUrl() cleanup, v2 Nice refactor
Attachment #667319 - Flags: review?(mark.finkle) → review+
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.
Attachment #667561 - Flags: review?(mark.finkle) → review+
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
Attachment #669024 - Flags: review?(mark.finkle) → review+
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 19
You need to log in before you can comment on or make changes to this bug.