Last Comment Bug 715836 - android.view.ViewRoot$CalledFromWrongThreadException: Only the original thread that created a view hierarchy can touch its views.
: android.view.ViewRoot$CalledFromWrongThreadException: Only the original threa...
Status: RESOLVED FIXED
: regression
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: 12 Branch
: Other Linux
: P2 normal (vote)
: Firefox 12
Assigned To: Kartikaya Gupta (email:kats@mozilla.com)
:
Mentors:
http://www.wechoosethemoon.org
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-01-06 00:58 PST by Naoki Hirata :nhirata (please use needinfo instead of cc)
Modified: 2012-01-19 13:27 PST (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
fixed
11+


Attachments
logcat (38.62 KB, text/plain)
2012-01-06 00:58 PST, Naoki Hirata :nhirata (please use needinfo instead of cc)
no flags Details
(1/2) Remove unused functions that call repositionPluginViews (4.02 KB, patch)
2012-01-12 13:54 PST, Kartikaya Gupta (email:kats@mozilla.com)
pwalton: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Review
(2/2) Make sure repositionPluginViews is called on the UI thread (1.59 KB, patch)
2012-01-12 13:54 PST, Kartikaya Gupta (email:kats@mozilla.com)
pwalton: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Review

Description Naoki Hirata :nhirata (please use needinfo instead of cc) 2012-01-06 00:58:16 PST
Created attachment 586357 [details]
logcat

1. go to http://www.wechoosethemoon.org
2. look at logcat

Expected: no errors
Actual: 
01-06 08:39:10.625: W/System.err(381): android.view.ViewRoot$CalledFromWrongThreadException: Only the original thread that created a view hierarchy can touch its views.
01-06 08:39:10.636: W/System.err(381): 	at android.view.ViewRoot.checkThread(ViewRoot.java:2932)

Note:
1. It's not crashing, just an error
2. nexus s, 2.3.1; flash 11, 20120105
3. bug 703256 fixed the crash
Comment 1 Kartikaya Gupta (email:kats@mozilla.com) 2012-01-06 06:21:42 PST
Regression introduced in 58a278555680 (bug 710096). It shouldn't be hard to fix. I'm going to take some time to re-review the threading model of all the pan/zoom code though, there are definitely more bugs lurking there.
Comment 2 Kartikaya Gupta (email:kats@mozilla.com) 2012-01-12 13:54:05 PST
Created attachment 588177 [details] [diff] [review]
(1/2) Remove unused functions that call repositionPluginViews

Just in case somebody calls these in the future from the wrong thread.
Comment 3 Kartikaya Gupta (email:kats@mozilla.com) 2012-01-12 13:54:55 PST
Created attachment 588178 [details] [diff] [review]
(2/2) Make sure repositionPluginViews is called on the UI thread
Comment 4 Patrick Walton (:pcwalton) 2012-01-12 18:14:15 PST
Comment on attachment 588177 [details] [diff] [review]
(1/2) Remove unused functions that call repositionPluginViews

Review of attachment 588177 [details] [diff] [review]:
-----------------------------------------------------------------

Nice cleanup catch.
Comment 5 Patrick Walton (:pcwalton) 2012-01-12 18:14:46 PST
Comment on attachment 588178 [details] [diff] [review]
(2/2) Make sure repositionPluginViews is called on the UI thread

Review of attachment 588178 [details] [diff] [review]:
-----------------------------------------------------------------

r=me
Comment 7 Matt Brubeck (:mbrubeck) 2012-01-13 07:50:48 PST
You can set the status-firefoxN and Target Milestone fields when landing on inbound; it saves some work for the inbound mergers.
Comment 9 Kartikaya Gupta (email:kats@mozilla.com) 2012-01-19 02:29:22 PST
Comment on attachment 588177 [details] [diff] [review]
(1/2) Remove unused functions that call repositionPluginViews

[Approval Request Comment (for both patches)]
Regression caused by (bug #): bug 710096
User impact if declined: possible races and errors as UI things are accessed from a non-UI thread
Testing completed (on m-c, etc.): on m-c
Risk to taking this patch (and alternatives if risky): pretty low-risk; one patches removes dead code (errors will be caught by compiler) and the other patch moves a chunk of code to another thread
Comment 10 Alex Keybl [:akeybl] 2012-01-19 12:04:07 PST
Comment on attachment 588177 [details] [diff] [review]
(1/2) Remove unused functions that call repositionPluginViews

[Triage Comment]
Mobile only - approved for Aurora.

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