Tracks GVR version crossed with headset type using UMA.

chrome/browser/android/vr_shell/vr_usage_monitor.h

Index: chrome/browser/android/vr_shell/vr_usage_monitor.h
diff --git a/chrome/browser/android/vr_shell/vr_usage_monitor.h b/chrome/browser/android/vr_shell/vr_usage_monitor.h
index 594f34841f07235fb3327585e4050f6857269520..5c9205b1e12a5ea5c2a662951a3cf26ca0353787 100644
--- a/chrome/browser/android/vr_shell/vr_usage_monitor.h
+++ b/chrome/browser/android/vr_shell/vr_usage_monitor.h
@@ -12,6 +12,8 @@
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_observer.h"
+#include "third_party/gvr-android-sdk/src/libraries/headers/vr/gvr/capi/include/gvr.h"

[billorr, 2017/05/05 17:48:41]

We should try to keep gvr-related code limited to as few places as possible. 
VRMetricsHelper could be used for Windows.

[tiborg, 2017/05/05 19:03:34]

Good point. What about putting an #ifdef ANDROID around this and the function
which use GVR? Alternatively, we could add a VrMetricsHelperAndroid. But that's
maybe overkill for only three methods. What do you think?

[billorr, 2017/05/05 20:10:21]

#ifdef could work, or the histograms could be moved out to vrshell/gl. I'm fine
with either.

The original purpose of this helper class was to be a webcontentsobserver, and
aggregate other data needed to have some of the metrics.  If the data is
available elsewhere we could log metrics there instead of doing extra plumbing
to get here.

[tiborg, 2017/05/10 20:39:52]

I put an #ifdef guard around it.

Also, moved the static helper logging functions into VrMetricsUtil (a new
class).

+#include "third_party/gvr-android-sdk/src/libraries/headers/vr/gvr/capi/include/gvr_types.h"
 
 namespace vr_shell {
 
@@ -53,6 +55,44 @@ class SessionTimer {
   DISALLOW_COPY_AND_ASSIGN(SessionTimer);
 };
 
+enum class ViewerType {
+  UNKNOWN_TYPE = 0,
+  CARDBOARD = 1,
+  DAYDREAM = 2,
+  VIEWER_TYPE_MAX,
+};
+
+enum class GVRVersionCrossedVRViewerType {
+
+  GVR_OLDER_AND_UNKNOWN_TYPE = 0,

[ddorwin, 2017/05/05 20:29:59]

Does unknown == not selected? Do we need the GVR update to know for sure?

[tiborg, 2017/05/10 20:39:52]

It's gone. See comment below.

+  GVR_1_10_AND_UNKNOWN_TYPE = 1,
+  GVR_1_20_AND_UNKNOWN_TYPE = 2,
+  GVR_1_30_AND_UNKNOWN_TYPE = 3,
+  GVR_1_40_AND_UNKNOWN_TYPE = 4,
+  GVR_NEWER_AND_UNKNOWN_TYPE = 5,
+  GVR_OTHER_AND_UNKNOWN_TYPE = 6,
+
+  GVR_OLDER_AND_CARDBOARD = 7,
+  GVR_1_10_AND_CARDBOARD = 8,
+  GVR_1_20_AND_CARDBOARD = 9,
+  GVR_1_30_AND_CARDBOARD = 10,
+  GVR_1_40_AND_CARDBOARD = 11,
+  GVR_NEWER_AND_CARDBOARD = 12,

[billorr, 2017/05/05 17:48:41]

what is the plan when newer versions come out?  do we have to rush out new
events (and wait 12 weeks for the our changes to reach stable?)

should we preemtively add some new buckets?

Can we make gvr_version a component in the system info metadata?

[tiborg, 2017/05/05 19:03:35]

Putting the GVR version in the metadata is probably the best solution. But I
would put that in a follow up CL since it is likely not finished for M60. I
don't know how we could preemtively add buckets since I don't even know how to
recognize future versions. The next version could be 1.50, 1.75, 2.3 or
whatever.

[billorr, 2017/05/05 20:10:21]

indeed.

One minor thing we could do to make it easier when new versions do come out is
change our numbering here:
GVR_OLDER_AND_UNKNOWN_TYPE = 0,
GVR_OLDER_AND_CARDBOARD = 1,
GVR_OLDER_AND_DAYDREAM = 2,

GVR_1_10_AND_UNKNOWN_TYPE = 3,
GVR_1_10_AND_CARDBOARD = 4,
GVR_1_10_AND_DAYDREAM = 5,

GVR_1_20_AND_UNKNOWN_TYPE = 6,
GVR_1_20_AND_CARDBOARD = 7,
GVR_1_20_AND_DAYDREAM = 8,

Then when we do have a new version, it just extends our enum (without having
weird values out of place).

[ddorwin, 2017/05/05 20:29:59]

We should definitely be able to report versions beyond what already exist
because GVR will undoubtedly be updated before the end of this version of
Chrome's lifetime. Also, we don't want to keep updating this code.

More generally, this doesn't scale and requires maintenance.

Can we instead have a separate metric for CardboardViewerGvrVersion and
DaydreamViewerGvrVersion, for example? That will also be much easier to reason
about on the dashboards.

The values could be a string or a number (e.g. for x.y.z, multiple x 8 100 and
add y).


P.S.
Regarding the M60 comment, UMAs are "forever." You can't change the enums and we
have to maintain the definitions indefinitely.

[tiborg, 2017/05/10 20:39:52]

Ok, these are good points. As discussed offline, changed the logging so that the
GVR version is encoded in one value and logged for each headset type separately
using UMA_HISTOGRAM_SPARSE_SLOWLY.

+  GVR_OTHER_AND_CARDBOARD = 13,
+
+  GVR_OLDER_AND_DAYDREAM = 14,
+  GVR_1_10_AND_DAYDREAM = 15,
+  GVR_1_20_AND_DAYDREAM = 16,
+  GVR_1_30_AND_DAYDREAM = 17,
+  GVR_1_40_AND_DAYDREAM = 18,
+  GVR_NEWER_AND_DAYDREAM = 19,
+  GVR_OTHER_AND_DAYDREAM = 20,
+
+  GVR_NONE = 21,
+
+  GVR_VERSION_CROSSED_VR_VIEWER_TYPE_MAX,
+};
+
 // This class is not threadsafe and must only be used from the main thread.
 class VrMetricsHelper : public content::WebContentsObserver {
  public:
@@ -61,6 +101,10 @@ class VrMetricsHelper : public content::WebContentsObserver {
   void SetWebVREnabled(bool is_webvr_presenting);
   void SetVRActive(bool is_vr_enabled);
 
+  static void LogGvrVersionAndVrViewerType(gvr_context* context,
+                                           gvr_version version);
+  static void LogVrViewerType(gvr_context* context);
+
  private:
   // WebContentObserver
   void MediaStartedPlaying(const MediaPlayerInfo& media_info,
@@ -74,6 +118,8 @@ class VrMetricsHelper : public content::WebContentsObserver {
   void SetVrMode(VRMode mode);
   void UpdateMode();
 
+  static ViewerType GetVrViewerType(gvr_context* context);
+
   std::unique_ptr<SessionTimer> mode_video_timer_;
   std::unique_ptr<SessionTimer> session_video_timer_;
   std::unique_ptr<SessionTimer> mode_timer_;
@@ -90,6 +136,8 @@ class VrMetricsHelper : public content::WebContentsObserver {
   int num_session_navigation_ = 0;
   int num_session_video_playback_ = 0;
 
+  static bool logged_gvr_version_;
+
   GURL origin_;
 };