New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 757742 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
(currently inactive on Chromium)
Closed: Aug 2017
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Addition of SeparateTaskCustomTabVrActivity.java appears to break the build

Project Member Reported by mattcary@chromium.org, Aug 22 2017

Issue description

Reverting the cl.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 22 2017

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

commit c1395a1816131fa8639bf7104f2253b8acdd3762
Author: Matthew Cary <mattcary@chromium.org>
Date: Tue Aug 22 09:08:28 2017

Revert "[vr] Use CCT for auto-present intents instead of CTA"

This reverts commit eaf9d7edad93017ca1013155f2a29e5763bbbde2.

Reason for revert: this appears to break the build. SeparateTaskCustomTabVrActivity.java is added to chrome_java_sources, but requires the VR library. Probably that means it needs to be added to chrome_vr_java_sources instead, but I'm not totally sure.

Original change's description:
> [vr] Use CCT for auto-present intents instead of CTA
> 
> At a high-level, this CL makes the following modifications:
> 1) Auto-present intents open CCT instead of CTA. More specifically they open a
>    VR-specific CustomTab in a seperate task.
> 2) Controller app button is a no-op from auto-presented WebVR presentation.
> 3) The VR-specific CustomTab hides system UI from the VR splash screen.
> 
> There is no way for the user to go back to this CCT once they're done with. This
> means that we call finishAndClose once the user exits VR mode. More explicitly:
> - Clicking the home/back button from the system UI menu, and the 'x' to close VR
>   mode takes you back to 2D homescreen (this is what other VR apps do).
> - Clicking the recent apps button and coming back to CCT will restart the CCT in
>   2D mode and NOT auto-present.
> - Clicking the controller home button takes you back to Daydream
> 
> This CL also moves intent logic from VrShellDelegate to VrIntentUtils.
> 
> Bug: 755199,743214,755197
> Change-Id: I3e46a5ac2ea192fba6476b857be1a57b489f3241
> Reviewed-on: https://chromium-review.googlesource.com/616088
> Commit-Queue: Yash Malik <ymalik@chromium.org>
> Reviewed-by: Ted Choc (OOO 8.21-25) <tedchoc@chromium.org>
> Reviewed-by: Yusuf Ozuysal <yusufo@chromium.org>
> Reviewed-by: Biao She <bshe@chromium.org>
> Reviewed-by: Yash Malik <ymalik@chromium.org>
> Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#496241}

TBR=bshe@chromium.org,mthiesse@chromium.org,tedchoc@chromium.org,yusufo@chromium.org,ymalik@chromium.org,bsheedy@chromium.org

Change-Id: I431430dae65adcfde3f1b6a472039c0345e7d3e6
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 755199, 743214, 755197,  757742 
Reviewed-on: https://chromium-review.googlesource.com/625716
Commit-Queue: Matthew Cary <mattcary@chromium.org>
Reviewed-by: Matthew Cary <mattcary@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496257}
[modify] https://crrev.com/c1395a1816131fa8639bf7104f2253b8acdd3762/chrome/android/java/AndroidManifest.xml
[modify] https://crrev.com/c1395a1816131fa8639bf7104f2253b8acdd3762/chrome/android/java/res/values-v17/styles.xml
[modify] https://crrev.com/c1395a1816131fa8639bf7104f2253b8acdd3762/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabIntentDataProvider.java
[modify] https://crrev.com/c1395a1816131fa8639bf7104f2253b8acdd3762/chrome/android/java/src/org/chromium/chrome/browser/document/ChromeLauncherActivity.java
[modify] https://crrev.com/c1395a1816131fa8639bf7104f2253b8acdd3762/chrome/android/java/src/org/chromium/chrome/browser/firstrun/FirstRunFlowSequencer.java
[delete] https://crrev.com/352b02d06f4b5317884ffea14df45594bcf89573/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/SeparateTaskCustomTabVrActivity.java
[modify] https://crrev.com/c1395a1816131fa8639bf7104f2253b8acdd3762/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrFirstRunActivity.java
[add] https://crrev.com/c1395a1816131fa8639bf7104f2253b8acdd3762/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrIntentHandler.java
[delete] https://crrev.com/352b02d06f4b5317884ffea14df45594bcf89573/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrIntentUtils.java
[modify] https://crrev.com/c1395a1816131fa8639bf7104f2253b8acdd3762/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java
[modify] https://crrev.com/c1395a1816131fa8639bf7104f2253b8acdd3762/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellImpl.java
[modify] https://crrev.com/c1395a1816131fa8639bf7104f2253b8acdd3762/chrome/android/java_sources.gni
[modify] https://crrev.com/c1395a1816131fa8639bf7104f2253b8acdd3762/chrome/android/javatests/src/org/chromium/chrome/browser/vr_shell/WebVrTransitionTest.java
[modify] https://crrev.com/c1395a1816131fa8639bf7104f2253b8acdd3762/chrome/android/javatests/src/org/chromium/chrome/browser/vr_shell/mock/MockVrIntentHandler.java
[modify] https://crrev.com/c1395a1816131fa8639bf7104f2253b8acdd3762/chrome/android/javatests/src/org/chromium/chrome/browser/vr_shell/util/VrTransitionUtils.java
[modify] https://crrev.com/c1395a1816131fa8639bf7104f2253b8acdd3762/chrome/browser/vr/ui_scene_manager.cc
[modify] https://crrev.com/c1395a1816131fa8639bf7104f2253b8acdd3762/chrome/browser/vr/ui_scene_manager_unittest.cc

Sorry, 625716 is the reverting CL. The original CL is 616088.
Labels: sheriff-android
Labels: -Pri-3 Pri-2
Summary: Addition of SeparateTaskCustomTabVrActivity.java appears to break the build (was: crrev.com/625716 appears to break the build)
Project Member

Comment 6 by bugdroid1@chromium.org, Aug 22 2017

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

commit ba5468be92dde91424e518b062c2bbf96a01402b
Author: Yash Malik <ymalik@google.com>
Date: Tue Aug 22 14:16:53 2017

Revert "Revert "[vr] Use CCT for auto-present intents instead of CTA""

This reverts commit c1395a1816131fa8639bf7104f2253b8acdd3762.

Reason for revert: Re-landing with the fix to build on non-VR enabled platforms. We were using an api to enable VR mode in SeparateTaskCustomTabVrActivity.java that was only enabled in VR mode. The CL removes that API call and enables VR mode in the AndroidManifest entry for the activity. 

Original change's description:
> Revert "[vr] Use CCT for auto-present intents instead of CTA"
> 
> This reverts commit eaf9d7edad93017ca1013155f2a29e5763bbbde2.
> 
> Reason for revert: this appears to break the build. SeparateTaskCustomTabVrActivity.java is added to chrome_java_sources, but requires the VR library. Probably that means it needs to be added to chrome_vr_java_sources instead, but I'm not totally sure.
> 
> Original change's description:
> > [vr] Use CCT for auto-present intents instead of CTA
> > 
> > At a high-level, this CL makes the following modifications:
> > 1) Auto-present intents open CCT instead of CTA. More specifically they open a
> >    VR-specific CustomTab in a seperate task.
> > 2) Controller app button is a no-op from auto-presented WebVR presentation.
> > 3) The VR-specific CustomTab hides system UI from the VR splash screen.
> > 
> > There is no way for the user to go back to this CCT once they're done with. This
> > means that we call finishAndClose once the user exits VR mode. More explicitly:
> > - Clicking the home/back button from the system UI menu, and the 'x' to close VR
> >   mode takes you back to 2D homescreen (this is what other VR apps do).
> > - Clicking the recent apps button and coming back to CCT will restart the CCT in
> >   2D mode and NOT auto-present.
> > - Clicking the controller home button takes you back to Daydream
> > 
> > This CL also moves intent logic from VrShellDelegate to VrIntentUtils.
> > 
> > Bug: 755199,743214,755197
> > Change-Id: I3e46a5ac2ea192fba6476b857be1a57b489f3241
> > Reviewed-on: https://chromium-review.googlesource.com/616088
> > Commit-Queue: Yash Malik <ymalik@chromium.org>
> > Reviewed-by: Ted Choc (OOO 8.21-25) <tedchoc@chromium.org>
> > Reviewed-by: Yusuf Ozuysal <yusufo@chromium.org>
> > Reviewed-by: Biao She <bshe@chromium.org>
> > Reviewed-by: Yash Malik <ymalik@chromium.org>
> > Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#496241}
> 
> TBR=bshe@chromium.org,mthiesse@chromium.org,tedchoc@chromium.org,yusufo@chromium.org,ymalik@chromium.org,bsheedy@chromium.org
> 
> Change-Id: I431430dae65adcfde3f1b6a472039c0345e7d3e6
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 755199, 743214, 755197,  757742 
> Reviewed-on: https://chromium-review.googlesource.com/625716
> Commit-Queue: Matthew Cary <mattcary@chromium.org>
> Reviewed-by: Matthew Cary <mattcary@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#496257}

TBR=bshe@chromium.org,mthiesse@chromium.org,tedchoc@chromium.org,yusufo@chromium.org,ymalik@chromium.org,mattcary@chromium.org,bsheedy@chromium.org

Change-Id: Ib049381962dca1818a2229f95fffd04cf3ebdfe4
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 755199, 743214, 755197,  757742 
Reviewed-on: https://chromium-review.googlesource.com/626256
Reviewed-by: Yash Malik <ymalik@chromium.org>
Commit-Queue: Yash Malik <ymalik@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496301}
[modify] https://crrev.com/ba5468be92dde91424e518b062c2bbf96a01402b/chrome/android/java/AndroidManifest.xml
[modify] https://crrev.com/ba5468be92dde91424e518b062c2bbf96a01402b/chrome/android/java/res/values-v17/styles.xml
[modify] https://crrev.com/ba5468be92dde91424e518b062c2bbf96a01402b/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabIntentDataProvider.java
[modify] https://crrev.com/ba5468be92dde91424e518b062c2bbf96a01402b/chrome/android/java/src/org/chromium/chrome/browser/document/ChromeLauncherActivity.java
[modify] https://crrev.com/ba5468be92dde91424e518b062c2bbf96a01402b/chrome/android/java/src/org/chromium/chrome/browser/firstrun/FirstRunFlowSequencer.java
[add] https://crrev.com/ba5468be92dde91424e518b062c2bbf96a01402b/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/SeparateTaskCustomTabVrActivity.java
[modify] https://crrev.com/ba5468be92dde91424e518b062c2bbf96a01402b/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrFirstRunActivity.java
[delete] https://crrev.com/497159595a57a1f210a30b9b170945b76232ac93/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrIntentHandler.java
[add] https://crrev.com/ba5468be92dde91424e518b062c2bbf96a01402b/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrIntentUtils.java
[modify] https://crrev.com/ba5468be92dde91424e518b062c2bbf96a01402b/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java
[modify] https://crrev.com/ba5468be92dde91424e518b062c2bbf96a01402b/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellImpl.java
[modify] https://crrev.com/ba5468be92dde91424e518b062c2bbf96a01402b/chrome/android/java_sources.gni
[modify] https://crrev.com/ba5468be92dde91424e518b062c2bbf96a01402b/chrome/android/javatests/src/org/chromium/chrome/browser/vr_shell/WebVrTransitionTest.java
[modify] https://crrev.com/ba5468be92dde91424e518b062c2bbf96a01402b/chrome/android/javatests/src/org/chromium/chrome/browser/vr_shell/mock/MockVrIntentHandler.java
[modify] https://crrev.com/ba5468be92dde91424e518b062c2bbf96a01402b/chrome/android/javatests/src/org/chromium/chrome/browser/vr_shell/util/VrTransitionUtils.java
[modify] https://crrev.com/ba5468be92dde91424e518b062c2bbf96a01402b/chrome/browser/vr/ui_scene_manager.cc
[modify] https://crrev.com/ba5468be92dde91424e518b062c2bbf96a01402b/chrome/browser/vr/ui_scene_manager_unittest.cc

Comment 7 by pasko@google.com, Aug 25 2017

Status: Fixed (was: Untriaged)
nothing is broken after reland ..

Sign in to add a comment