Android: Trigger tab placeholder update when activity is paused

Android: Trigger tab placeholder update when activity is paused

This makes sure we have a placeholder bitmap when we resume
and don't have a frame from the renderer yet.

BUG=636630
Committed: https://crrev.com/0a1f48d5cb6a5d263f85bc5322912b7f703c40ac
Cr-Commit-Position: refs/heads/master@{#414822}

[no sievers, 2016-07-27 00:44:18]

Description was changed from

==========
Android: Update placeholder when activity is stopped

This makes sure we have a placeholder bitmap when we resume
and don't have a frame from the renderer yet.
==========

to

==========
Android: Update placeholder when activity is stopped

This makes sure we have a placeholder bitmap when we resume
and don't have a frame from the renderer yet.
==========

[no sievers, 2016-07-27 00:44:18]

sievers@chromium.org changed reviewers:
+ dtrainor@chromium.org

[no sievers, 2016-07-27 00:45:23]

dtrainor, is this reasonable?

And how do I convince myself that this bitmap will be stored persistently even
if all UI resources get evicted around the time this readback request happens?
(Because the surface and context go away.)

I want us to resume and then have the bitmap available. (In practice it *seems*
to work.)

[no sievers, 2016-07-27 00:45:36]

The CQ bit was checked by sievers@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-27 00:46:01]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[no sievers, 2016-07-27 00:57:06]

This is flaky for other reasons:

- relies on certain order of window visibility and activity change notifications
between RWHV which might drop the frontbuffer and ChromeActivity.java to trigger
the readback

- we might not composite again if we are tearing down the Surface (so would need
the logic CompositorImpl had at one point to check for pending readback requests
before tearing down, but that was removed)

[commit-bot: I haz the power, 2016-07-27 01:44:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-27 01:44:02]

Dry run: This issue passed the CQ dry run.

[David Trainor- moved to gerrit, 2016-07-27 19:43:47]

On 2016/07/27 00:45:23, sievers wrote:
> dtrainor, is this reasonable?

Kind of? :)
 
> And how do I convince myself that this bitmap will be stored persistently even
> if all UI resources get evicted around the time this readback request happens?
> (Because the surface and context go away.)

IIUC dropping the UI resource just means we won't have a texture for it, but it
still goes through the process of getting compressed and flushed to disk.  We do
have a problem where if the activity gets destroyed we might not have actually
gotten it to disk yet, so it might not be there when we restart.
 
> I want us to resume and then have the bitmap available. (In practice it
*seems*
> to work.)

That should (for the most part) work.
in_reply_to: 5649050225344512
send_mail: 1
subject: Android: Update placeholder when activity is stopped

----------
Please reload the previous page and post again.

[no sievers, 2016-08-04 19:33:16]

On 2016/07/27 19:43:47, David Trainor wrote:
> On 2016/07/27 00:45:23, sievers wrote:
> > dtrainor, is this reasonable?
> 
> Kind of? :)
>  
> > And how do I convince myself that this bitmap will be stored persistently
even
> > if all UI resources get evicted around the time this readback request
happens?
> > (Because the surface and context go away.)
> 
> IIUC dropping the UI resource just means we won't have a texture for it, but
it
> still goes through the process of getting compressed and flushed to disk.  We
do
> have a problem where if the activity gets destroyed we might not have actually
> gotten it to disk yet, so it might not be there when we restart.
>  

I was actually hoping to keep it in memory as a bitmap and mostly care about the
case where the activity stops and resumes.

But all of that is racy:
1) We might tear down the current frame (SurfaceLayer) before we do the readback
2) If we get surfaceDestroyed() the cc::Display will be torn down before we
might have committed and drawn and finished the readback
3) CreateUIResource() happens eagerly from ThumbnailCache::PostCompressionTask()
which means we might upload it and drop the bitmap before the context goes away.
4) surfaceDestroyed() currently triggers
ThumbnailCache::OnUIResourcesWereEvicted() which drops all Thumbnails

I'll file a bug since it sounds like in practice this is unlikely to result in a
bitmap without a bunch of other changes.

[no sievers, 2016-08-13 00:53:44]

Description was changed from

==========
Android: Update placeholder when activity is stopped

This makes sure we have a placeholder bitmap when we resume
and don't have a frame from the renderer yet.
==========

to

==========
Android: Update placeholder when activity is paused

This makes sure we have a placeholder bitmap when we resume
and don't have a frame from the renderer yet.
==========

[no sievers, 2016-08-13 00:54:33]

Ok I'm landing (and have been landing) a bunch of patches to fix all the races,
so that this actually succeeds (not only sometimes).

And I've moved it to take the snapshot a bit earlier onPause() instead.

ptal

[no sievers, 2016-08-13 00:54:54]

Description was changed from

==========
Android: Update placeholder when activity is paused

This makes sure we have a placeholder bitmap when we resume
and don't have a frame from the renderer yet.
==========

to

==========
Android: Update placeholder when activity is paused

This makes sure we have a placeholder bitmap when we resume
and don't have a frame from the renderer yet.

BUG=636630
==========

[no sievers, 2016-08-13 00:55:31]

Description was changed from

==========
Android: Update placeholder when activity is paused

This makes sure we have a placeholder bitmap when we resume
and don't have a frame from the renderer yet.

BUG=636630
==========

to

==========
Android: Trigger tab placeholder update when activity is paused

This makes sure we have a placeholder bitmap when we resume
and don't have a frame from the renderer yet.

BUG=636630
==========

[no sievers, 2016-08-13 00:58:56]

The CQ bit was checked by sievers@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-13 00:59:30]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-13 02:07:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-13 02:07:33]

Dry run: This issue passed the CQ dry run.

[David Trainor- moved to gerrit, 2016-08-16 02:49:28]

lgtm!

[no sievers, 2016-08-26 20:54:19]

The CQ bit was checked by sievers@chromium.org

[commit-bot: I haz the power, 2016-08-26 20:54:59]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-26 21:47:23]

Description was changed from

==========
Android: Trigger tab placeholder update when activity is paused

This makes sure we have a placeholder bitmap when we resume
and don't have a frame from the renderer yet.

BUG=636630
==========

to

==========
Android: Trigger tab placeholder update when activity is paused

This makes sure we have a placeholder bitmap when we resume
and don't have a frame from the renderer yet.

BUG=636630
==========

[commit-bot: I haz the power, 2016-08-26 21:47:25]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-08-26 21:50:38]

Description was changed from

==========
Android: Trigger tab placeholder update when activity is paused

This makes sure we have a placeholder bitmap when we resume
and don't have a frame from the renderer yet.

BUG=636630
==========

to

==========
Android: Trigger tab placeholder update when activity is paused

This makes sure we have a placeholder bitmap when we resume
and don't have a frame from the renderer yet.

BUG=636630

Committed: https://crrev.com/0a1f48d5cb6a5d263f85bc5322912b7f703c40ac
Cr-Commit-Position: refs/heads/master@{#414822}
==========

[commit-bot: I haz the power, 2016-08-26 21:50:39]

Patchset 3 (id:??) landed as
https://crrev.com/0a1f48d5cb6a5d263f85bc5322912b7f703c40ac
Cr-Commit-Position: refs/heads/master@{#414822}

[mustaq, 2016-09-01 15:11:41]

A revert of this CL (patchset #3 id:40001) has been created in
https://codereview.chromium.org/2306623002/ by mustaq@chromium.org.

The reason for reverting is: Seeing lots of memory regression since this landed.
Bisects point to this CL with high confidence. See crbug.com/641962..