711333 - Race condition when moving and updating textures

Status: RESOLVED FIXED

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

Description

[Patrick Walton (:pcwalton)]

It's possible for the following chain of events to occur:

(1) GeckoSoftwareLayerClient begins a transaction on the tile layer in beginDrawing().
(2) GSLC acquires the monitor on the layer controller in updateViewport(), which is called by endDrawing().
(3) GSLC moves the tile and the viewport around.
(4) GSLC releases the monitor.
(5) The GL rendering thread acquires the monitor on the layer controller.
(6) In the process of rendering a frame, the rendering thread tries to begin a transaction but fails and continues with the old image in the new position. Now the display is corrupt for one frame.
(7) GSLC ends the transaction.

This usually shows up as one of the flashes when navigating from page to page. It corrects itself after a frame or two but is quite unsightly. The solution should be for GSLC to always hold the monitor around the transaction. Will post a patch to this effect tomorrow.

Comment 1

[Patrick Walton (:pcwalton)]

Attachment: Proposed patch.

Proposed patch attached.

Comment 2

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

Comment on attachment 582497 [details] [diff] [review]
Proposed patch.

>diff --git a/widget/src/android/AndroidGraphicBuffer.cpp b/widget/src/android/AndroidGraphicBuffer.cpp
>   bool EnsureInitialized()
>   {
>     if (mInitialized) {
>       return true;
>     }
> 
>+    return false;
>+
>     void *handle = dlopen(ANDROID_EGL_PATH, RTLD_LAZY);

Testing code? r+ assuming this hunk was included accidentally.

Comment 3

[Patrick Walton (:pcwalton)]

(In reply to Kartikaya Gupta (:kats) from comment #2)
> Testing code? r+ assuming this hunk was included accidentally.

Actually, code needed to make my Droid X not crash (bug 711694), but good catch :)

Comment 4

[Patrick Walton (:pcwalton)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d4c526ba5343

Comment 5

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/d4c526ba5343

Comment 6

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 582497 [details] [diff] [review]
Proposed patch.

[Triage Comment]
just moves synchronization further up the stack. Also this has baked on m-c for 20 days

Comment 7

[Brad Lassey [:blassey] (use needinfo?)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/6b5628b66ac9