Last Comment Bug 720494 - Move GeckoApp.handleAddTab logic into Tabs.addTab
: Move GeckoApp.handleAddTab logic into Tabs.addTab
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: Trunk
: ARM Android
P4 normal (vote)
: Firefox 12
Assigned To: :Margaret Leibovic
: Sebastian Kaspari (:sebastian)
Depends on:
Blocks: 718465
  Show dependency treegraph
Reported: 2012-01-23 13:35 PST by :Margaret Leibovic
Modified: 2012-01-26 12:29 PST (History)
4 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch (4.95 KB, patch)
2012-01-23 13:35 PST, :Margaret Leibovic
mbrubeck: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description User image :Margaret Leibovic 2012-01-23 13:35:41 PST
Created attachment 590851 [details] [diff] [review]

This patch just cleans up the code a little. I also added some logging with timestamps so that we can time this message path (as suggested in bug 719493 comment 7).

Fixing slowness in this loop would help us fix bug 718465.
Comment 2 User image :Ms2ger (⌚ UTC+1/+2) 2012-01-25 07:29:48 PST
Comment 3 User image :Margaret Leibovic 2012-01-25 10:33:31 PST
Comment on attachment 590851 [details] [diff] [review]

[Approval Request Comment]
Cleans up code and adds some logging. We'll probably write patches on top of this, so it would be good to be in Aurora to prevent conflicts. Landed on m-c this morning, so it will be in Nightly tomorrow.
Comment 4 User image :Ms2ger (⌚ UTC+1/+2) 2012-01-25 11:33:06 PST
Last time I checked, code cleanup was well on the not-approvable side.
Comment 5 User image :Margaret Leibovic 2012-01-25 11:42:38 PST
(In reply to Ms2ger from comment #4)
> Last time I checked, code cleanup was well on the not-approvable side.

Mobile has different approval requirements. Because we're landing the majority of the patches we write on aurora, we've also been landing bugs like this to prevent conflicts when trying to land future patches. We could wait until that conflict comes up and track down this patch then, but IMO it's easier to just land a low-risk patch like this, especially since a patch for bug 718465 would probably depend on this.
Comment 6 User image Alex Keybl [:akeybl] 2012-01-25 17:08:05 PST
Comment on attachment 590851 [details] [diff] [review]

[Triage Comment]
Mobile only - approved for Aurora.

Note You need to log in before you can comment on or make changes to this bug.