VR: Ensure initial paused state is set correctly for VrShellDelegate.

VR: Ensure initial paused state is set correctly for VrShellDelegate.

BUG=715674
Review-Url: https://codereview.chromium.org/2840353002
Cr-Commit-Position: refs/heads/master@{#467859}
Committed: https://chromium.googlesource.com/chromium/src/+/97681b526480ec96c009d815b5bd32899587ea60

[mthiesse, 2017-04-26 21:37:40]

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

[mthiesse, 2017-04-26 21:37:40]

PTAL

[mthiesse, 2017-04-26 21:38:01]

Description was changed from

==========
VR: Ensure initial paused state is set correctly.

BUG=715674
==========

to

==========
VR: Ensure initial paused state is set correctly for VrShellDelegate.

BUG=715674
==========

[bshe, 2017-04-27 15:08:15]

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

https://codereview.chromium.org/2840353002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java:353:
mPaused = ApplicationStatus.getStateForActivity(activity) !=
ActivityState.RESUMED;
Could this be:
mPaused = ApplicationStatus.getStateForActivity(activity) ==
ActivityState.PAUSED
If not, do you mind to add comment about why?

https://codereview.chromium.org/2840353002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java:379:
if (sInstance != null) assert sInstance.mPaused;
could sInstance be null? IIUC, this event listener is registered when sInstance
is created.

[mthiesse, 2017-04-27 15:31:14]

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

https://codereview.chromium.org/2840353002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java:353:
mPaused = ApplicationStatus.getStateForActivity(activity) !=
ActivityState.RESUMED;
On 2017/04/27 15:08:15, bshe wrote:
> Could this be:
> mPaused = ApplicationStatus.getStateForActivity(activity) ==
> ActivityState.PAUSED
> If not, do you mind to add comment about why?

No it couldn't be that, or if the activity was stopped we would think it was
resumed. I added a comment.

https://codereview.chromium.org/2840353002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java:379:
if (sInstance != null) assert sInstance.mPaused;
On 2017/04/27 15:08:15, bshe wrote:
> could sInstance be null? IIUC, this event listener is registered when
sInstance
> is created.

Done.

[bshe, 2017-04-27 22:17:20]

On 2017/04/27 15:31:14, mthiesse wrote:
>
https://codereview.chromium.org/2840353002/diff/1/chrome/android/java/src/org...
> File
>
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java
> (right):
> 
>
https://codereview.chromium.org/2840353002/diff/1/chrome/android/java/src/org...
>
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java:353:
> mPaused = ApplicationStatus.getStateForActivity(activity) !=
> ActivityState.RESUMED;
> On 2017/04/27 15:08:15, bshe wrote:
> > Could this be:
> > mPaused = ApplicationStatus.getStateForActivity(activity) ==
> > ActivityState.PAUSED
> > If not, do you mind to add comment about why?
> 
> No it couldn't be that, or if the activity was stopped we would think it was
> resumed. I added a comment.
> 
>
https://codereview.chromium.org/2840353002/diff/1/chrome/android/java/src/org...
>
chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java:379:
> if (sInstance != null) assert sInstance.mPaused;
> On 2017/04/27 15:08:15, bshe wrote:
> > could sInstance be null? IIUC, this event listener is registered when
> sInstance
> > is created.
> 
> Done.

lgtm

[mthiesse, 2017-04-27 22:22:41]

The CQ bit was checked by mthiesse@chromium.org

[commit-bot: I haz the power, 2017-04-27 22:30:04]

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, 2017-04-28 02:57:25]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1493331761550410,
"parent_rev": "d2f9f9ff225693ec2dc17919217abb1d284330f2", "commit_rev":
"97681b526480ec96c009d815b5bd32899587ea60"}

[commit-bot: I haz the power, 2017-04-28 02:57:34]

Description was changed from

==========
VR: Ensure initial paused state is set correctly for VrShellDelegate.

BUG=715674
==========

to

==========
VR: Ensure initial paused state is set correctly for VrShellDelegate.

BUG=715674

Review-Url: https://codereview.chromium.org/2840353002
Cr-Commit-Position: refs/heads/master@{#467859}
Committed:
https://chromium.googlesource.com/chromium/src/+/97681b526480ec96c009d815b5bd...
==========

[commit-bot: I haz the power, 2017-04-28 02:57:35]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/97681b526480ec96c009d815b5bd...