Race condition when moving and updating textures

RESOLVED FIXED in Firefox 11

Status

()

Firefox for Android
General
P4
normal
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: pcwalton, Assigned: pcwalton)

Tracking

unspecified
Firefox 12
ARM
Android
Points:
---

Firefox Tracking Flags

(firefox11 fixed, fennec11+)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
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.
(Assignee)

Comment 1

6 years ago
Created attachment 582497 [details] [diff] [review]
Proposed patch.

Proposed patch attached.
Assignee: nobody → pwalton
Status: NEW → ASSIGNED
Attachment #582497 - Flags: review?(bugmail.mozilla)
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.
Attachment #582497 - Flags: review?(bugmail.mozilla) → review+
(Assignee)

Comment 3

6 years ago
(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 :)
Priority: -- → P4
(Assignee)

Comment 4

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/d4c526ba5343
https://hg.mozilla.org/mozilla-central/rev/d4c526ba5343
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 12
tracking-fennec: --- → 11+
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
Attachment #582497 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/6b5628b66ac9
status-firefox11: --- → fixed
You need to log in before you can comment on or make changes to this bug.