Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2559873002:
Enable VR shell on Android builds (Closed)
DescriptionEnable VR shell on Android builds
BUG=671349
Committed: https://crrev.com/597a5430c27891e7d1402ea97c7622a666ce5f6c
Cr-Commit-Position: refs/heads/master@{#437622}
Patch Set 1 #Patch Set 2 : Rebase against head and disable broken vr tests. #Patch Set 3 : minor adustment #Patch Set 4 : rebase #
Messages
Total messages: 29 (18 generated)
amp@chromium.org changed reviewers: + bshe@chromium.org
On 2016/12/07 21:38:28, amp wrote: lgtm
The CQ bit was checked by amp@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
amp@chromium.org changed reviewers: + bsheedy@chromium.org, tedchoc@chromium.org
PTAL +tedchoc for owners +bsheedy just to verify the way I'm disabling the tests.
The CQ bit was checked by amp@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
On 2016/12/08 21:42:55, amp wrote: > PTAL > > +tedchoc for owners > > +bsheedy just to verify the way I'm disabling the tests. LGTM
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by amp@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
On 2016/12/08 21:50:34, bsheedy wrote: > On 2016/12/08 21:42:55, amp wrote: > > PTAL > > > > +tedchoc for owners > > > > +bsheedy just to verify the way I'm disabling the tests. > > LGTM Does this have any user facing changes? The bug doesn't have any launch-y things on it, so I want to make sure this shouldn't be going through that process.
On 2016/12/09 18:23:07, Ted C (OOO 12.8.16) wrote: > On 2016/12/08 21:50:34, bsheedy wrote: > > On 2016/12/08 21:42:55, amp wrote: > > > PTAL > > > > > > +tedchoc for owners > > > > > > +bsheedy just to verify the way I'm disabling the tests. > > > > LGTM > > Does this have any user facing changes? The bug doesn't have > any launch-y things on it, so I want to make sure this shouldn't > be going through that process. Everything is still behind a run time flag. Most of the build flags are a no op as well since the WebVR build flag for origin trials which went out in 56 implied vr shell in most cases. The only visible change from this will be that the Chrome VR flag shows up in about://flags (where before it was only settable through command line). Do we need launch-y type things for about://flags additions?
On 2016/12/09 18:37:00, amp wrote: > On 2016/12/09 18:23:07, Ted C (OOO 12.8.16) wrote: > > On 2016/12/08 21:50:34, bsheedy wrote: > > > On 2016/12/08 21:42:55, amp wrote: > > > > PTAL > > > > > > > > +tedchoc for owners > > > > > > > > +bsheedy just to verify the way I'm disabling the tests. > > > > > > LGTM > > > > Does this have any user facing changes? The bug doesn't have > > any launch-y things on it, so I want to make sure this shouldn't > > be going through that process. > > Everything is still behind a run time flag. Most of the build flags are a no op > as well since the WebVR build flag for origin trials which went out in 56 > implied vr shell in most cases. > > The only visible change from this will be that the Chrome VR flag shows up in > about://flags (where before it was only settable through command line). > > Do we need launch-y type things for about://flags additions? To clarify on the about://flags part, the flag is already there but only shows up when this build flag is enabled: https://cs.chromium.org/chromium/src/chrome/browser/about_flags.cc?l=1619
On 2016/12/09 18:37:00, amp wrote: > On 2016/12/09 18:23:07, Ted C (OOO 12.8.16) wrote: > > On 2016/12/08 21:50:34, bsheedy wrote: > > > On 2016/12/08 21:42:55, amp wrote: > > > > PTAL > > > > > > > > +tedchoc for owners > > > > > > > > +bsheedy just to verify the way I'm disabling the tests. > > > > > > LGTM > > > > Does this have any user facing changes? The bug doesn't have > > any launch-y things on it, so I want to make sure this shouldn't > > be going through that process. > > Everything is still behind a run time flag. Most of the build flags are a no op > as well since the WebVR build flag for origin trials which went out in 56 > implied vr shell in most cases. > > The only visible change from this will be that the Chrome VR flag shows up in > about://flags (where before it was only settable through command line). > > Do we need launch-y type things for about://flags additions? Nah, you shouldn't need that for about:flags stuff. Just things that will get shown to users w/o their interaction. lgtm
The CQ bit was unchecked by amp@chromium.org
The CQ bit was checked by amp@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from bshe@chromium.org, bsheedy@chromium.org Link to the patchset: https://codereview.chromium.org/2559873002/#ps60001 (title: "rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 60001, "attempt_start_ts": 1481309442876590,
"parent_rev": "81d7791581b9121a9da76e9d63c2b7925e7e875a", "commit_rev":
"5de6ef6778c0b342fe603b5d3735bc110167264c"}
Message was sent while issue was closed.
Description was changed from ========== Enable VR shell on Android builds BUG=671349 ========== to ========== Enable VR shell on Android builds BUG=671349 Review-Url: https://codereview.chromium.org/2559873002 ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== Enable VR shell on Android builds BUG=671349 Review-Url: https://codereview.chromium.org/2559873002 ========== to ========== Enable VR shell on Android builds BUG=671349 Committed: https://crrev.com/597a5430c27891e7d1402ea97c7622a666ce5f6c Cr-Commit-Position: refs/heads/master@{#437622} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/597a5430c27891e7d1402ea97c7622a666ce5f6c Cr-Commit-Position: refs/heads/master@{#437622} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
