Get GWS ID from field trial

chrome/browser/android/data_usage/external_data_use_observer_bridge.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/android/data_usage/external_data_use_observer_bridge.h"
 
 #include <memory>
 #include <vector>
 
 #include "base/android/context_utils.h"
 #include "base/android/jni_string.h"
 #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"
 #include "chrome/browser/metrics/chrome_metrics_service_accessor.h"
 #include "components/variations/variations_associated_data.h"
 #include "content/public/browser/browser_thread.h"
 #include "jni/ExternalDataUseObserver_jni.h"
 
 using base::android::ConvertUTF8ToJavaString;
 
 namespace {
 
 // Name of the external data use observer synthetic field trial, enabled and
 // disabled groups.
 const char kSyntheticFieldTrial[] = "SyntheticExternalDataUseObserver";
 const char kSyntheticFieldTrialEnabledGroup[] = "Enabled";
 const char kSyntheticFieldTrialDisabledGroup[] = "Disabled";
 
 // Returns the package name of the control app from the field trial.
 const std::string GetControlAppPackageName() {
   return variations::GetVariationParamValue(
       chrome::android::ExternalDataUseObserver::
           kExternalDataUseObserverFieldTrial,
       "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 {
 
 namespace android {
 
 ExternalDataUseObserverBridge::ExternalDataUseObserverBridge()
     : construct_time_(base::TimeTicks::Now()),
       is_first_matching_rule_fetch_(true) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
@@ skipping 138 matching lines @@
     bool should_register) const {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(!j_external_data_use_observer_.is_null());
 
   io_task_runner_->PostTask(
       FROM_HERE,
       base::Bind(&ExternalDataUseObserver::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) {
   return RegisterNativesImpl(env);
 }
 
 }  // namespace android
 
 }  // namespace chrome