Sometimes fennec's window is black when started the DEBUG intent

VERIFIED FIXED

Status

VERIFIED FIXED
8 years ago
8 years ago

People

(Reporter: cjones, Assigned: blassey)

Tracking

Trunk
ARM
Android

Details

Attachments

(1 attachment, 1 obsolete attachment)

STR
 (1) Start fennec with am start -a org.mozilla.gecko.DEBUG -n org.mozilla.fennec/org.mozilla.fennec.App
 (2) Click "launch"
Most of the time fennec doesn't draw, just appears as a black window.

When this happens, I've found that changing orientation seems to make fennec re-render.  I don't much care about the DEBUG intent, but this seems to indicate a race condition that could bite us in normal operation.
The problem seems to be because of the different order of some function calls for DEBUG workflow.

Normally we launch Gecko from onCreate, then the GeckoApp activity handles onStart and onResume, where we raise the flag surfaceView.mSurfaceNeedsRedraw. After that at some point the system calls GeckoSurfaceView.surfaceChanged where according to the flag the redraw is scheduled.

In DEBUG mode the onStart, onResume, and GeckoSurfaceView.surfaceChanged calls happen before we launch Gecko (which is done later on a button click), and apparently this screws up the drawing. Another call to GeckoAppShell.scheduleRedraw() right after (or before) the launch doesn't seem to help. I don't see an easy fix at the moment - tried some hacks, but they didn't work right away. This needs to be investigated further to fix the DEBUG flow, but I believe we shouldn't have such problem in normal operation, where the functions are called in a proper order. They are in the same thread, so there is no race condition.
I just got a black screen when starting up normally on device.  Not sure of it's related to this.
This happened to me again on device.  I'm using a local build from m-c f9f10c04dceb and m-b cdb24af322a8.  I hadn't seen this happen during normal startup before new dlopen.
Chris: I think you're seeing bug 604090 unless you're starting in a DEBUG intent.
I'm worried these are symptoms of the same problem (comment 0).  Might be different problems though.
Created attachment 485307 [details] [diff] [review]
patch
Assignee: nobody → blassey.bugs
Attachment #485307 - Flags: review?(mwu)

Comment 7

8 years ago
Comment on attachment 485307 [details] [diff] [review]
patch

I like this approach. It's the right way to deal with the race between our surface getting changed and gecko starting. However, it doesn't make sense to queue all events. If the user is on a slow device and starts doing random stuff to our app during early loading, we don't really want those events being handled. Also, we should only care about the last size event unless fennec is depending on multiple resize events, so we don't need a list - just the last resize event.

Eliminating setInitialSize is good - you should also kill the wrapper for setInitialSize in other-licenses/android/APKOpen.cpp.
Attachment #485307 - Flags: review?(mwu)
(In reply to comment #7)
> Comment on attachment 485307 [details] [diff] [review]
> patch
> 
> I like this approach. It's the right way to deal with the race between our
> surface getting changed and gecko starting. However, it doesn't make sense to
> queue all events. If the user is on a slow device and starts doing random stuff
> to our app during early loading, we don't really want those events being
> handled. Also, we should only care about the last size event unless fennec is
> depending on multiple resize events, so we don't need a list - just the last
> resize event.
> 
> Eliminating setInitialSize is good - you should also kill the wrapper for
> setInitialSize in other-licenses/android/APKOpen.cpp.

I'm not sure why we would want to filter events on the java side of things. The platform should coalesce events where its appropriate. IMO we should send all events and let the platform handle it.

Comment 9

8 years ago
(In reply to comment #8)
> I'm not sure why we would want to filter events on the java side of things. The
> platform should coalesce events where its appropriate. IMO we should send all
> events and let the platform handle it.

It's not coalescing. It's just firing an move event when we gecko is ready to receive it. We don't care about any other events before gecko starts, so there's no point in queuing them.

Comment 10

8 years ago
(In reply to comment #9)
> It's not coalescing. It's just firing an move event when we gecko is ready to
> receive it. We don't care about any other events before gecko starts, so
> there's no point in queuing them.

And by move I mean resize.
Created attachment 485608 [details] [diff] [review]
patch
Attachment #485307 - Attachment is obsolete: true
Attachment #485608 - Flags: review?(mwu)

Comment 12

8 years ago
Comment on attachment 485608 [details] [diff] [review]
patch

Please remove the setInitialSize wrapper in APKOpen.cpp http://hg.mozilla.org/mozilla-central/file/392605694d4b/other-licenses/android/APKOpen.cpp#l220 . r=me with that.
Attachment #485608 - Flags: review?(mwu) → review+
(Assignee)

Updated

8 years ago
tracking-fennec: --- → 2.0b3+
pushed http://hg.mozilla.org/mozilla-central/rev/b77a54634c1b
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Mozilla/5.0 (Android; Linux armv71; rv2.0b8pre) Gecko/20101029 Firefox/4.0b8pre Fennec/4.0b2pre

Ran/tested : adb shell am start -a org.mozilla.gecko.DEBUG -n org.mozilla.fennec/org.mozilla.fennec.App

tried changing landscape/portrait w/ and w/o hardware keyboard.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.