New issue
Advanced search Search tips

Issue 667900 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug
Proj-XR
Proj-XR-VR

Blocked on:
issue 666811



Sign in to add a comment

Re-enable rappor for vrshell

Project Member Reported by billorr@chromium.org, Nov 22 2016

Issue description

There was a dependency cycle and resulting test code build break in 666811.  This was fixed by removing the cycle, but also removing RAPPOR measurements in vrshell.

This bug is the work to properly removing the dependency cycle and re-enable RAPPOR from vrshell.
 
Blockedon: 666811
Components: UI>Browser>VR
Labels: -Pri-3 Proj-VR M-57 OS-All Pri-1
Status: Started (was: Assigned)
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 30 2016

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

commit fc622a50ae2f9501d07b271fe22181ed23bde1f5
Author: billorr <billorr@chromium.org>
Date: Wed Nov 30 01:55:15 2016

Re-enable rappor for vrshell

There was a dependency cycle and resulting test code build break when
consuming g_browser_process from vr_shell.  This fix provides a way to
report rappor metrics in components that don't depend on chrome/browser.

BUG= 667900 

Review-Url: https://codereview.chromium.org/2526643002
Cr-Commit-Position: refs/heads/master@{#435119}

[modify] https://crrev.com/fc622a50ae2f9501d07b271fe22181ed23bde1f5/chrome/browser/android/vr_shell/BUILD.gn
[modify] https://crrev.com/fc622a50ae2f9501d07b271fe22181ed23bde1f5/chrome/browser/android/vr_shell/vr_usage_monitor.cc
[modify] https://crrev.com/fc622a50ae2f9501d07b271fe22181ed23bde1f5/chrome/browser/browser_process_impl.cc
[modify] https://crrev.com/fc622a50ae2f9501d07b271fe22181ed23bde1f5/components/rappor/rappor_utils.cc
[modify] https://crrev.com/fc622a50ae2f9501d07b271fe22181ed23bde1f5/components/rappor/rappor_utils.h

Labels: -M-57 M-56
See 667964, as this is a dependent change to enable tests for VRShell.

Comment 5 by amp@chromium.org, Dec 1 2016

This is still M57, isn't it? The original disabling didn't go into M56 iirc.
Status: Fixed (was: Started)
Labels: -M-56 M-57
Not needed in M56. Fixed in M57.

Sign in to add a comment