Remove CVC reparenting from VrShell. Also fixes crash when exiting VR.

Remove CVC reparenting from VrShell. Also fixes crash when exiting VR.

Reparenting the content CVC isn't necessary anymore; it won't be receiving input as the presentation view now captures it, and we can set the CVC size directly, rather than having it inherit the size from its parent view. This was also adding a lot of complexity and makes switching tabs/WebContents difficult.

BUG=673536
Committed: https://crrev.com/a6fc3619d17754dbde5d727b460986a77dd7c3db
Cr-Commit-Position: refs/heads/master@{#439169}

[mthiesse, 2016-12-15 21:47:37]

mthiesse@chromium.org changed reviewers:
+ bshe@chromium.org

[mthiesse, 2016-12-15 21:47:38]

PTAL

[mthiesse, 2016-12-15 21:49:10]

Description was changed from

==========
Remove CVC reparenting from VrShell. Also fixes crash when exiting VR.

BUG=673536
==========

to

==========
Remove CVC reparenting from VrShell. Also fixes crash when exiting VR.

Reparenting the content CVC isn't necessary anymore, it won't be receiving input
as the presentation view now captures it, and we can set the CVC size directly,
rather than having it inherit the size from its parent view.

BUG=673536
==========

[mthiesse, 2016-12-15 21:54:07]

Description was changed from

==========
Remove CVC reparenting from VrShell. Also fixes crash when exiting VR.

Reparenting the content CVC isn't necessary anymore, it won't be receiving input
as the presentation view now captures it, and we can set the CVC size directly,
rather than having it inherit the size from its parent view.

BUG=673536
==========

to

==========
Remove CVC reparenting from VrShell. Also fixes crash when exiting VR.

Reparenting the content CVC isn't necessary anymore; it won't be receiving input
as the presentation view now captures it, and we can set the CVC size directly,
rather than having it inherit the size from its parent view. This was also
adding a lot of complexity and makes switching tabs/WebContents difficult.

BUG=673536
==========

[bshe, 2016-12-16 18:24:48]

lgtm with nits

https://codereview.chromium.org/2579163002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java
(right):

https://codereview.chromium.org/2579163002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java:477:
mTab.updateBrowserControlsState(BrowserControlsState.SHOWN, false);
seems like you also disabled animation here. can you also mention it in the bug
description?

https://codereview.chromium.org/2579163002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellImpl.java
(right):

https://codereview.chromium.org/2579163002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellImpl.java:141:
return true;
nit: remove this return.

[mthiesse, 2016-12-16 18:46:30]

https://codereview.chromium.org/2579163002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java
(right):

https://codereview.chromium.org/2579163002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java:477:
mTab.updateBrowserControlsState(BrowserControlsState.SHOWN, false);
On 2016/12/16 18:24:47, bshe wrote:
> seems like you also disabled animation here. can you also mention it in the
bug
> description?

Actually, now that we're calling setPositionsForTabToNonFullscreen, which is the
proper way to reset the omnibox position, etc. I've removed this call.

https://codereview.chromium.org/2579163002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellImpl.java
(right):

https://codereview.chromium.org/2579163002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellImpl.java:141:
return true;
On 2016/12/16 18:24:47, bshe wrote:
> nit: remove this return.

I meant to revert this change. Thanks for pointing this out.

[mthiesse, 2016-12-16 18:47:18]

The CQ bit was checked by mthiesse@chromium.org

[mthiesse, 2016-12-16 18:47:19]

The patchset sent to the CQ was uploaded after l-g-t-m from bshe@chromium.org
Link to the patchset: https://codereview.chromium.org/2579163002/#ps40001
(title: "Address comments")

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

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

[bshe, 2016-12-16 18:52:52]

lgtm

[commit-bot: I haz the power, 2016-12-16 19:43:37]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1481914038814830,
"parent_rev": "3d56c940d11ccdf02bd8d95a4a64aa7c5c6564e4", "commit_rev":
"80b6bc321c0985a42747c17103372d9971a78ede"}

[commit-bot: I haz the power, 2016-12-16 19:44:07]

Description was changed from

==========
Remove CVC reparenting from VrShell. Also fixes crash when exiting VR.

Reparenting the content CVC isn't necessary anymore; it won't be receiving input
as the presentation view now captures it, and we can set the CVC size directly,
rather than having it inherit the size from its parent view. This was also
adding a lot of complexity and makes switching tabs/WebContents difficult.

BUG=673536
==========

to

==========
Remove CVC reparenting from VrShell. Also fixes crash when exiting VR.

Reparenting the content CVC isn't necessary anymore; it won't be receiving input
as the presentation view now captures it, and we can set the CVC size directly,
rather than having it inherit the size from its parent view. This was also
adding a lot of complexity and makes switching tabs/WebContents difficult.

BUG=673536

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

[commit-bot: I haz the power, 2016-12-16 19:44:08]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-12-16 19:46:11]

Description was changed from

==========
Remove CVC reparenting from VrShell. Also fixes crash when exiting VR.

Reparenting the content CVC isn't necessary anymore; it won't be receiving input
as the presentation view now captures it, and we can set the CVC size directly,
rather than having it inherit the size from its parent view. This was also
adding a lot of complexity and makes switching tabs/WebContents difficult.

BUG=673536

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

to

==========
Remove CVC reparenting from VrShell. Also fixes crash when exiting VR.

Reparenting the content CVC isn't necessary anymore; it won't be receiving input
as the presentation view now captures it, and we can set the CVC size directly,
rather than having it inherit the size from its parent view. This was also
adding a lot of complexity and makes switching tabs/WebContents difficult.

BUG=673536

Committed: https://crrev.com/a6fc3619d17754dbde5d727b460986a77dd7c3db
Cr-Commit-Position: refs/heads/master@{#439169}
==========

[commit-bot: I haz the power, 2016-12-16 19:46:12]

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