715836 - android.view.ViewRoot$CalledFromWrongThreadException: Only the original thread that created a view hierarchy can touch its views.

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect, P2)

Description

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Attachment: 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:[e-mail])]

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:[e-mail])]

Attachment: (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:[e-mail])]

Attachment: (2/2) Make sure repositionPluginViews is called on the UI thread

Comment 4

[Patrick Walton (:pcwalton)]

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)]

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 6

[Kartikaya Gupta (email:[e-mail])]

https://hg.mozilla.org/integration/mozilla-inbound/rev/317592537ff6
https://hg.mozilla.org/integration/mozilla-inbound/rev/157c86d30692

Comment 7

[Matt Brubeck (:mbrubeck)]

You can set the status-firefoxN and Target Milestone fields when landing on inbound; it saves some work for the inbound mergers.

Comment 8

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/317592537ff6
https://hg.mozilla.org/mozilla-central/rev/157c86d30692

Comment 9

[Kartikaya Gupta (email:[e-mail])]

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]]

Comment on attachment 588177 [details] [diff] [review]
(1/2) Remove unused functions that call repositionPluginViews

[Triage Comment]
Mobile only - approved for Aurora.

Comment 11

[Kartikaya Gupta (email:[e-mail])]

https://hg.mozilla.org/releases/mozilla-aurora/rev/bd8a953a2717
https://hg.mozilla.org/releases/mozilla-aurora/rev/524dcbfd9363