Exit VR when our main content tab is hidden (Usually due to links opening in new tab)

Exit VR when our main content tab is hidden (Usually due to links opening in new tab)

BUG=670437
Committed: https://crrev.com/f35768b1c5502ea9f88fe2b9ff56bb9f53869ceb
Cr-Commit-Position: refs/heads/master@{#436050}

[mthiesse, 2016-12-01 21:39:39]

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

[mthiesse, 2016-12-01 21:39:40]

PTAL

[cjgrant, 2016-12-02 15:20:08]

cjgrant@chromium.org changed reviewers:
+ cjgrant@chromium.org

[cjgrant, 2016-12-02 15:20:08]

We should flag stuff like this to the test team so they can add a test case for
it, right?

[mthiesse, 2016-12-02 15:49:47]

Description was changed from

==========
Exit VR when our main content tab is hidden (Usually due to links opening in new
tab)

BUG=670437
==========

to

==========
Exit VR when our main content tab is hidden (Usually due to links opening in new
tab)

BUG=670437
==========

[mthiesse, 2016-12-02 15:51:35]

On 2016/12/02 15:20:08, cjgrant wrote:
> We should flag stuff like this to the test team so they can add a test case
for
> it, right?

+cc bsheedy

Maybe manual tests? I don't know if it would be practical to test this flow with
instrumentation tests, as it's calling out to another activity (the DOFF flow),
and requires navigation to specific web pages.

[bshe, 2016-12-02 16:04:05]

On 2016/12/02 15:51:35, mthiesse wrote:
> On 2016/12/02 15:20:08, cjgrant wrote:
> > We should flag stuff like this to the test team so they can add a test case
> for
> > it, right?
> 
> +cc bsheedy
> 
> Maybe manual tests? I don't know if it would be practical to test this flow
with
> instrumentation tests, as it's calling out to another activity (the DOFF
flow),
> and requires navigation to specific web pages.

Yah. The transition becomes more and more complex with all different conditions
that we check.
I created a doc here:
https://docs.google.com/document/d/16E7M8q2lwRic5EsHzbgVPFBRe-fsKIKDk-X4mba3u...
We should have tests(probably manual tests first) to cover these cases.

[bshe, 2016-12-02 16:21:31]

On 2016/12/02 16:04:05, bshe wrote:
> On 2016/12/02 15:51:35, mthiesse wrote:
> > On 2016/12/02 15:20:08, cjgrant wrote:
> > > We should flag stuff like this to the test team so they can add a test
case
> > for
> > > it, right?
> > 
> > +cc bsheedy
> > 
> > Maybe manual tests? I don't know if it would be practical to test this flow
> with
> > instrumentation tests, as it's calling out to another activity (the DOFF
> flow),
> > and requires navigation to specific web pages.
> 
> Yah. The transition becomes more and more complex with all different
conditions
> that we check.
> I created a doc here:
>
https://docs.google.com/document/d/16E7M8q2lwRic5EsHzbgVPFBRe-fsKIKDk-X4mba3u...
> We should have tests(probably manual tests first) to cover these cases.

lgtm by the way. busy with promoting my doc forgot to do the real import thing
here :p

[mthiesse, 2016-12-02 16:27:05]

On 2016/12/02 16:21:31, bshe wrote:
> On 2016/12/02 16:04:05, bshe wrote:
> > On 2016/12/02 15:51:35, mthiesse wrote:
> > > On 2016/12/02 15:20:08, cjgrant wrote:
> > > > We should flag stuff like this to the test team so they can add a test
> case
> > > for
> > > > it, right?
> > > 
> > > +cc bsheedy
> > > 
> > > Maybe manual tests? I don't know if it would be practical to test this
flow
> > with
> > > instrumentation tests, as it's calling out to another activity (the DOFF
> > flow),
> > > and requires navigation to specific web pages.
> > 
> > Yah. The transition becomes more and more complex with all different
> conditions
> > that we check.
> > I created a doc here:
> >
>
https://docs.google.com/document/d/16E7M8q2lwRic5EsHzbgVPFBRe-fsKIKDk-X4mba3u...
> > We should have tests(probably manual tests first) to cover these cases.
> 
> lgtm by the way. busy with promoting my doc forgot to do the real import thing
> here :p

http://i.imgur.com/Jk8TfMO.jpg

[amp, 2016-12-02 16:28:48]

The CQ bit was checked by amp@chromium.org

[amp, 2016-12-02 16:28:59]

The CQ bit was unchecked by amp@chromium.org

[commit-bot: I haz the power, 2016-12-02 16:29:03]

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

[commit-bot: I haz the power, 2016-12-02 16:29:03]

This CL has an open dependency (Issue 2541243003 Patch 80001). Please resolve
the dependency and try again.
If you are sure that there is no real dependency, please use one of the options
listed in https://goo.gl/9Es4OR to land the CL.

[mthiesse, 2016-12-02 21:14:17]

The CQ bit was checked by mthiesse@chromium.org

[mthiesse, 2016-12-02 21:14:17]

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/2542293002/#ps40001
(title: "rebase")

[commit-bot: I haz the power, 2016-12-02 21:15:25]

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-02 22:18:18]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1480713257587070,
"parent_rev": "57a998f5db5b842cb388108c0f5c988e7f699f94", "commit_rev":
"dd864e78e324a16bffdece198b6df799712f53ef"}

[commit-bot: I haz the power, 2016-12-02 22:18:46]

Description was changed from

==========
Exit VR when our main content tab is hidden (Usually due to links opening in new
tab)

BUG=670437
==========

to

==========
Exit VR when our main content tab is hidden (Usually due to links opening in new
tab)

BUG=670437
==========

[commit-bot: I haz the power, 2016-12-02 22:18:47]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-12-02 22:23:02]

Description was changed from

==========
Exit VR when our main content tab is hidden (Usually due to links opening in new
tab)

BUG=670437
==========

to

==========
Exit VR when our main content tab is hidden (Usually due to links opening in new
tab)

BUG=670437

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

[commit-bot: I haz the power, 2016-12-02 22:23:03]

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