832508 - Pausing and resuming fennec on the ouya reopens it with a blank screen

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Toolbar, defect)

Description

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

STR:
1. Start Fennec on the ouya, and load a page like http://planet-webgl.org/
2. Hit the "start" button on the gamepad to go back to the Ouya main screen
3. Re-enter fennec

Expected results:
You see planet webgl

Actual results:
white screen is shown; but as soon as you scroll it will paint as expected (sort of).


The problem here seems to be that planet-webgl gets rendered at a scale of 1.95 on the HD-resolution ouya. When Fennec is pushed to the background, the main GeckoApp activity is killed (similar to the infamous "don't keep activities checkbox in developer settings). Starting fennec again creates a fresh activity and java-side set of objects, including a new ImmutableViewportMetrics with 1.0 zoom. When the compositor resumes and tries to composite the content which it already has, the progressiveUpdateCallback aborts the composite because the content is drawn at 1.95 and the viewport metrics is at 1.0.

I can reproduce the "actual" behaviour on a GN as well with the "don't keep activities" checkbox set, but for some reason it's not hitting the debug log in progressiveUpdateCallback so maybe there's a different underlying cause here.

Comment 1

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

Actually commenting out that abort still makes no difference, so I'm not sure what's going on. This will need more investigation.

Comment 2

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

This seems to be timing related, because when I attach using gdb and just wait for a few seconds at a breakpoint in the resume code it doesn't happen. Will investigate more.

Comment 3

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

I think the reason this is happening is that when Fennec is reopened, the LayerRenderer.onTabChanged gets called *after* the composite happens, so a background color is set on the surface. On the next composite, the background color is cleared (at the end of endDrawing) and so it looks fine. If a composite doesn't happen then the background color is displayed indefinitely. This also explains why it's something of a race condition, since the composite and the tab changed notification happen on different threads.

Comment 4

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

Disregard comment 3. onTabChanged gets called before the composite, but the setFirstPaintViewport doesn't get called in between so the paint state doesn't get moved through the right cycle of values. Trying to figure why that doesn't happen.

Comment 5

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

Attachment: Patch

So the main problem here is that once we have re-created the new surface and java-side objects, we need to get an updated viewport from the compositor to replace the Java information that was thrown away. We could do this by forcing a full redraw of the content in the Viewport:Flush handler in browser.js, but that could potentially take a few seconds and nothing will be visible during that time. Telling the compositor to do it directly avoids that delay.

Comment 6

[Chris Lord [:cwiiis]]

Comment on attachment 709097 [details] [diff] [review]
Patch

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

Looks fine to me, a couple of small quibbles that would be nice to address.

::: gfx/layers/ipc/CompositorParent.h
@@ +74,5 @@
>  
>    virtual void ShadowLayersUpdated(ShadowLayersParent* aLayerTree,
>                                     const TargetConfig& aTargetConfig,
>                                     bool isFirstPaint) MOZ_OVERRIDE;
> +  void ForceSetIsFirstPaint() { mIsFirstPaint = true; }

Not that keen on the naming of this function - What's wrong with just SetIsFirstPaint with a boolean flag? Otherwise perhaps ForceIsFirstPaint? Also, while what this does at a micro level is obvious, it'd be good to have a comment here about the consequences of what this does are.

::: widget/android/nsWindow.h
@@ +155,5 @@
>                                mozilla::layers::CompositorChild* aCompositorChild);
>      static void ScheduleComposite();
>      static void SchedulePauseComposition();
>      static void ScheduleResumeComposition(int width, int height);
> +    static void ForceFirstPaint();

I think the name of this function and it's corresponding function in CompositorParent should match.

Comment 7

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

Attachment: Patch v2

Sorry about that, I think I was so happy to finally figure out what was going on I didn't bother cleaning up the patch afterwards. This one addresses your review comments but re-requesting review to make sure the comments are clear enough.

Comment 8

[Chris Lord [:cwiiis]]

Comment on attachment 709711 [details] [diff] [review]
Patch v2

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

np, I do that all the time :) LGTM.

Comment 9

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/04617b9d49b3

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/04617b9d49b3