Fix a bug displaying gray patch upon device rotation

Fix a bug displaying gray patch upon device rotation

Desired size of content view added in https://crrev.com/2536223003
is supposed to be used for a particular instance like overlay panel.
This should be initialized back to default values after it was used
in adjusting backing size of the content in order not to affect
the size of other content views.

BUG=671967
Committed: https://crrev.com/2fc6c23ea651a3b7252399e4f231baede32c7059
Cr-Commit-Position: refs/heads/master@{#438994}

[Jinsuk Kim, 2016-12-13 07:54:11]

Description was changed from

==========
Fix a bug display gray patch upon device rotation

Desired size of content view added in https://crrev.com/2536223003
is supposed to be used for a particular instance like overlay panel.
This should be initialized back to default values after it was used
in adjusting backing size of the content in order not to affect
the size of other content views.

BUG=671967
==========

to

==========
Fix a bug displaying gray patch upon device rotation

Desired size of content view added in https://crrev.com/2536223003
is supposed to be used for a particular instance like overlay panel.
This should be initialized back to default values after it was used
in adjusting backing size of the content in order not to affect
the size of other content views.

BUG=671967
==========

[Jinsuk Kim, 2016-12-13 07:56:45]

jinsukkim@chromium.org changed reviewers:
+ mdjones@chromium.org

[Jinsuk Kim, 2016-12-13 07:56:46]

Alternatively the issue can be addressed like in
https://codereview.chromium.org/2536223003/diff/80001/chrome/android/java/src...
where each ContentView stores its own desired size and expose getter API. I'm
trying to do this without modifying content layer first if possible.

[Jinsuk Kim, 2016-12-13 08:00:55]

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

[commit-bot: I haz the power, 2016-12-13 08:01:16]

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-12-13 08:38:38]

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

[commit-bot: I haz the power, 2016-12-13 08:38:39]

Dry run: This issue passed the CQ dry run.

[Jinsuk Kim, 2016-12-14 23:13:29]

On 2016/12/13 07:56:46, Jinsuk Kim wrote:
> Alternatively the issue can be addressed like in
>
https://codereview.chromium.org/2536223003/diff/80001/chrome/android/java/src...
> where each ContentView stores its own desired size and expose getter API. I'm
> trying to do this without modifying content layer first if possible.

ping

[mdjones, 2016-12-15 16:59:40]

On 2016/12/13 07:56:46, Jinsuk Kim wrote:
> Alternatively the issue can be addressed like in
>
https://codereview.chromium.org/2536223003/diff/80001/chrome/android/java/src...
> where each ContentView stores its own desired size and expose getter API. I'm
> trying to do this without modifying content layer first if possible.

I think I'd rather see the ContentViewCore track it's own sizing requirements
mostly due to the state we now have here. If you want to keep the code out of
content, do you think it might be worth having a map of
ContentViewCore->MeasureSpec in CompositorViewHolder? I think OverlayPanel might
be the only case where we show multiple ContentViewCores so it might not be
worth the extra work.

[Jinsuk Kim, 2016-12-15 22:09:52]

> I think I'd rather see the ContentViewCore track it's own sizing requirements
> mostly due to the state we now have here. If you want to keep the code out of
> content, do you think it might be worth having a map of
> ContentViewCore->MeasureSpec in CompositorViewHolder? I think OverlayPanel
might
> be the only case where we show multiple ContentViewCores so it might not be
> worth the extra work.

I agree that the size info better be kept together with the corresponding
content.
The reason I preferred not altering content layer was because there is no other
embedder that needs this adjustment. Will be happy to move it to content once it
is worth sharing among other embedders. Then ContentView will be the better
place
to keep the size than ContentViewCore. Otherwise it would require another
refactoring since CVC is on its way to deletion.

I wonder if we need a map for this... though there are multiple (types of)
overlay
panels, only one needs the adjustment on the backing size at a time. Please 
take another look at the updated patch where I pass the overlay content view
to CompositorViewHolder to tell the content for overlay from the other. Feel
free to let me know what you think. I'm okay to make changes to content if
you do want it that way.

[mdjones, 2016-12-15 22:37:05]

On 2016/12/15 22:09:52, Jinsuk Kim wrote:
> > I think I'd rather see the ContentViewCore track it's own sizing
requirements
> > mostly due to the state we now have here. If you want to keep the code out
of
> > content, do you think it might be worth having a map of
> > ContentViewCore->MeasureSpec in CompositorViewHolder? I think OverlayPanel
> might
> > be the only case where we show multiple ContentViewCores so it might not be
> > worth the extra work.
> 
> I agree that the size info better be kept together with the corresponding
> content.
> The reason I preferred not altering content layer was because there is no
other
> embedder that needs this adjustment. Will be happy to move it to content once
it
> is worth sharing among other embedders. Then ContentView will be the better
> place
> to keep the size than ContentViewCore. Otherwise it would require another
> refactoring since CVC is on its way to deletion.
> 
> I wonder if we need a map for this... though there are multiple (types of)
> overlay
> panels, only one needs the adjustment on the backing size at a time. Please 
> take another look at the updated patch where I pass the overlay content view
> to CompositorViewHolder to tell the content for overlay from the other. Feel
> free to let me know what you think. I'm okay to make changes to content if
> you do want it that way.

I'm fine with waiting until there is a greater need for the cvc to hold this
information (especially since there is only this one case right now). I like the
current solution, it is more clear what is happening now. lgtm.

[Jinsuk Kim, 2016-12-16 00:03:55]

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

[commit-bot: I haz the power, 2016-12-16 00:05:09]

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-12-16 01:11:47]

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

[commit-bot: I haz the power, 2016-12-16 01:11:48]

Dry run: This issue passed the CQ dry run.

[Jinsuk Kim, 2016-12-16 01:41:03]

The CQ bit was checked by jinsukkim@chromium.org

[commit-bot: I haz the power, 2016-12-16 01:42:01]

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-12-16 03:06:14]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1481852463651220,
"parent_rev": "e0ba2981b51b119359437b3a399e6e58e9e30af2", "commit_rev":
"00b4c1890f88646178d0eabea18db9cecf31e33b"}

[commit-bot: I haz the power, 2016-12-16 03:06:44]

Description was changed from

==========
Fix a bug displaying gray patch upon device rotation

Desired size of content view added in https://crrev.com/2536223003
is supposed to be used for a particular instance like overlay panel.
This should be initialized back to default values after it was used
in adjusting backing size of the content in order not to affect
the size of other content views.

BUG=671967
==========

to

==========
Fix a bug displaying gray patch upon device rotation

Desired size of content view added in https://crrev.com/2536223003
is supposed to be used for a particular instance like overlay panel.
This should be initialized back to default values after it was used
in adjusting backing size of the content in order not to affect
the size of other content views.

BUG=671967

Review-Url: https://codereview.chromium.org/2574693002
==========

[commit-bot: I haz the power, 2016-12-16 03:06:45]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-12-16 03:09:07]

Description was changed from

==========
Fix a bug displaying gray patch upon device rotation

Desired size of content view added in https://crrev.com/2536223003
is supposed to be used for a particular instance like overlay panel.
This should be initialized back to default values after it was used
in adjusting backing size of the content in order not to affect
the size of other content views.

BUG=671967

Review-Url: https://codereview.chromium.org/2574693002
==========

to

==========
Fix a bug displaying gray patch upon device rotation

Desired size of content view added in https://crrev.com/2536223003
is supposed to be used for a particular instance like overlay panel.
This should be initialized back to default values after it was used
in adjusting backing size of the content in order not to affect
the size of other content views.

BUG=671967

Committed: https://crrev.com/2fc6c23ea651a3b7252399e4f231baede32c7059
Cr-Commit-Position: refs/heads/master@{#438994}
==========

[commit-bot: I haz the power, 2016-12-16 03:09:08]

Patchset 2 (id:??) landed as
https://crrev.com/2fc6c23ea651a3b7252399e4f231baede32c7059
Cr-Commit-Position: refs/heads/master@{#438994}