Get GWS ID from field trial

chrome/browser/android/data_usage/external_data_use_observer_bridge.cc

Index: chrome/browser/android/data_usage/external_data_use_observer_bridge.cc
diff --git a/chrome/browser/android/data_usage/external_data_use_observer_bridge.cc b/chrome/browser/android/data_usage/external_data_use_observer_bridge.cc
index 03b0e7220f4bcc993be5ba5e5e58dda7bbb2e09a..b6e710cf0a65ca84ee2bdb3e798a61fdc2b44e95 100644
--- a/chrome/browser/android/data_usage/external_data_use_observer_bridge.cc
+++ b/chrome/browser/android/data_usage/external_data_use_observer_bridge.cc
@@ -12,6 +12,7 @@
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/single_thread_task_runner.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/time/time.h"
 #include "chrome/browser/android/data_usage/data_use_tab_model.h"
 #include "chrome/browser/android/data_usage/external_data_use_observer.h"
@@ -38,6 +39,21 @@ const std::string GetControlAppPackageName() {
       "control_app_package_name");
 }
 
+// Returns the google variation ID from the field trial.
+variations::VariationID GetGoogleVariationID() {
+  variations::VariationID variation_id;
+  std::string variation_value = variations::GetVariationParamValue(
+      chrome::android::ExternalDataUseObserver::
+          kExternalDataUseObserverFieldTrial,
+      "variation_id");
+  if (!variation_value.empty() &&
+      static_cast<variations::VariationID>(

[sclittle, 2016/08/30 20:06:45]

nit: this static_cast doesn't really do anything, since base::StringToInt
returns a bool. Could you remove it?

[Raj, 2016/08/30 20:21:09]

Done.

+          base::StringToInt(variation_value, &variation_id))) {
+    return variation_id;
+  }
+  return variations::EMPTY_ID;
+}
+
 }  // namespace
 
 namespace chrome {
@@ -196,17 +212,17 @@ void ExternalDataUseObserverBridge::ShouldRegisterAsDataUseObserver(
                  external_data_use_observer_, should_register));
 
   // Set or clear the variation id for the enabled group.
-  JNIEnv* env = base::android::AttachCurrentThread();
-  variations::AssociateGoogleVariationID(
-      variations::GOOGLE_WEB_PROPERTIES, kSyntheticFieldTrial,
-      kSyntheticFieldTrialEnabledGroup,
-      should_register ? Java_ExternalDataUseObserver_getGoogleVariationID(
-                            env, j_external_data_use_observer_.obj())
-                      : variations::EMPTY_ID);
-  ChromeMetricsServiceAccessor::RegisterSyntheticFieldTrial(
-      kSyntheticFieldTrial, should_register
-                                ? kSyntheticFieldTrialEnabledGroup
-                                : kSyntheticFieldTrialDisabledGroup);
+  variations::VariationID variation_id = GetGoogleVariationID();
+  if (variation_id != variations::EMPTY_ID) {
+    variations::AssociateGoogleVariationID(
+        variations::GOOGLE_WEB_PROPERTIES, kSyntheticFieldTrial,
+        kSyntheticFieldTrialEnabledGroup,
+        should_register ? variation_id : variations::EMPTY_ID);

[sclittle, 2016/08/30 20:06:45]

So if should_register is false, then this sends EMPTY_ID, but if variation_id is
EMPTY_ID, then send nothing? Could add a comment explaining this, e.g. why does
(variation_id == EMPTY_ID) lead to different behavior than (should_register ==
false)?

[Raj, 2016/08/30 20:21:09]

Added comment. Setting EMPTY_ID, is actually clearing it and sending nothing.
Changing to Disabled group should be enough, and setting EMPTY_ID to Enabled
group is not needed. But there was some confusion what happens when disabling it
in the middle of a UMA log session. UMA records in that log session will be
treated as in a mixed mode w.r.t the synthetic trial. This only applies to UMA
records, and we do not plan to distinguish UMA based on this synthetic trial.

+    ChromeMetricsServiceAccessor::RegisterSyntheticFieldTrial(
+        kSyntheticFieldTrial, should_register
+                                  ? kSyntheticFieldTrialEnabledGroup
+                                  : kSyntheticFieldTrialDisabledGroup);
+  }
 }
 
 bool RegisterExternalDataUseObserver(JNIEnv* env) {