Megan, let's defer the promo until the user is not in VR.

How are you enabling VR?

This seems like an issue we'd have with the other promo dialogs and should come up with a uniform solution for. 

Why is this a P1 bug? Are we launching something in M61 that requires a fix?

The VR browser is (planning on) launching in M61. We should at the very least make sure these popups don't show up while in VR. This is one of the last remaining popups we haven't yet disabled while in VR.

I'm out all next week or I'd pick this up, but for anybody wanting to pick this up, checking if we're currently in VR is as simple as calling VrShellDelegate.isInVr().

Given that approximately zero users in the target market for this promo are going to be using Chrome in VR mode, my proposal is to simply block the promo altogether when in VR mode (rather than deferring it), assuming that deferring it is harder than blocking. Please do not spend more than 5 minutes on this.

It's trivial for us to defer showing the promo, I'll land a cl for that. This won't cover the case where the the promo shows, and then the user enters VR mode. Sometimes that might not be a conscious decision by the user, but a race case. I saw this when I couldn't get the VR mode button to show up for me. I was called VrShellDelegate.enterVrIfNecessary() to force it on, and I found even when I prevented showing the promo using VrShellDelegate.isInVr(), we would still get an overlaid promo.

We need more specific steps to reproduce this problem in order to fix it.

Assigning to VR to triage.

As megjablon@ mentioned in #9, we should be able to just simply defer the promo to the next start when not in VR by changing ChromeTabbedActivity#finishNativeInitialization:

https://chromium.googlesource.com/chromium/src/+/1e5c67abd6325c2c316fa9a9430b85079174f0d6/chrome/android/java/src/org/chromium/chrome/browser/ChromeTabbedActivity.java#478

We should find whatever signal that will tell us we are in VR (or the incoming intent will soon put us there) and add it to the list of signals that prevent promos in that session.

I do agree that this won't handle the case where you start chrome and then put it into the vr headset, but that is a larger issue where we might want to dismiss the promos but leave them able to be shown again, and I'm not sure if that works out of the box right now.

Sorry that this fell off my radar. The deferral sounds quite reasonable. Beyond this bug, I certainly agree that we need a more holistic solution for what to do when extant UI is up when the user enters VR mode.

Hey Amir, is this issue covered by any current ongoing efforts?

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/27258706427eb5343128460b0e6f52bbbd0bdb81

commit 27258706427eb5343128460b0e6f52bbbd0bdb81
Author: Amirhossein Simjour <asimjour@chromium.org>
Date: Thu Oct 12 19:52:34 2017

VR: Defer 2D Data Saver promo when in VR

This covers the case that we know that we are in VR when
ChromeTabbedActivity#finishNativeInitialization. This also covers the case
that Chrome is lunched from inside of VR.

BUG= 743250 

Change-Id: I5576a6ef5073fc8e1bbaa23585f11aa2de4540ed
Reviewed-on: https://chromium-review.googlesource.com/713719
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Amirhossein Simjour <asimjour@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508396}
[modify] https://crrev.com/27258706427eb5343128460b0e6f52bbbd0bdb81/chrome/android/java/src/org/chromium/chrome/browser/ChromeTabbedActivity.java

The race condition is still there, but the main problem is resolved. I change the priority and move it to M65. It should be resolved when we can move 2D ui elements to VR.

Any updates here?

Refreshed during triage.

is this similar to the approach we're using for Boris/Sogou?

Amir, can you answer #22?

This is similar, but we have more control over this one. We can defer it and show it next time that we are in 2D

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/17d6d246f25c79d658fc8ea4cb2575ae18d1882f

commit 17d6d246f25c79d658fc8ea4cb2575ae18d1882f
Author: Biao She <bshe@chromium.org>
Date: Thu Apr 19 17:02:12 2018

Fix potential race when decide if data reduction promo should show up in VR

Bug:  743250 
Change-Id: I34f0f60d52f3f5a72bc7dbb1c0647d737137180f
Reviewed-on: https://chromium-review.googlesource.com/1019541
Reviewed-by: Amirhossein Simjour <asimjour@chromium.org>
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Commit-Queue: Biao She <bshe@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552054}
[modify] https://crrev.com/17d6d246f25c79d658fc8ea4cb2575ae18d1882f/chrome/android/java/src/org/chromium/chrome/browser/ChromeTabbedActivity.java

The above Cl should make sure that the 2D data saver promo never shows up when Chrome cold launch.

Shouldn't it be merge-requested?

only need to merge cl at comment 26

Your change meets the bar and is auto-approved for M67. Please go ahead and merge the CL to branch 3396 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1b258301cd00b738c61efd47e178834a50d190c5

commit 1b258301cd00b738c61efd47e178834a50d190c5
Author: Biao She <bshe@chromium.org>
Date: Mon Apr 23 14:48:47 2018

Fix potential race when decide if data reduction promo should show up in VR

TBR=bshe@chromium.org

(cherry picked from commit 17d6d246f25c79d658fc8ea4cb2575ae18d1882f)

Bug:  743250 
Change-Id: I34f0f60d52f3f5a72bc7dbb1c0647d737137180f
Reviewed-on: https://chromium-review.googlesource.com/1019541
Reviewed-by: Amirhossein Simjour <asimjour@chromium.org>
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Commit-Queue: Biao She <bshe@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#552054}
Reviewed-on: https://chromium-review.googlesource.com/1023713
Reviewed-by: Biao She <bshe@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#207}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/1b258301cd00b738c61efd47e178834a50d190c5/chrome/android/java/src/org/chromium/chrome/browser/ChromeTabbedActivity.java

Fix verified on build 67.0.3396.29 beta.