Restrict sending GWS ID only to signed in users

When valid matching rules are available and user is signed-in to Chrome,
a synthetic field trial is created with GWS ID retrieved from field trial.

BUG=648651

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

chrome/browser/android/data_usage/data_use_ui_tab_model_factory.cc

@@ -12,7 +12,9 @@
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/io_thread.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/signin/signin_manager_factory.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
+#include "components/signin/core/browser/signin_manager.h"
 #include "content/public/browser/browser_thread.h"
 
 namespace chrome {
@@ -35,6 +37,19 @@ DataUseTabModel* GetDataUseTabModelOnIOThread(IOThread* io_thread) {
   return io_thread->globals()->external_data_use_observer->GetDataUseTabModel();
 }
 
+void SetRegisterGoogleVariationIDOnIOThread(IOThread* io_thread,
+                                            bool register_google_variation_id) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+
+  // Avoid null pointer referencing during browser shutdown.
+  if (io_thread && !io_thread->globals() &&
+      io_thread->globals()->external_data_use_observer) {
+    io_thread->globals()
+        ->external_data_use_observer->SetRegisterGoogleVariationID(
+            register_google_variation_id);
+  }
+}
+
 }  // namespace
 
 // static
@@ -64,10 +79,12 @@ DataUseUITabModelFactory::~DataUseUITabModelFactory() {}
 KeyedService* DataUseUITabModelFactory::BuildServiceInstanceFor(
     content::BrowserContext* context) const {
   DataUseUITabModel* data_use_ui_tab_model = new DataUseUITabModel();
+  Profile* profile = Profile::FromBrowserContext(context)->GetOriginalProfile();
+  const SigninManager* signin_manager =
+      SigninManagerFactory::GetForProfileIfExists(profile);
 
   // DataUseUITabModel should not be created for incognito profile.
-  DCHECK_EQ(Profile::FromBrowserContext(context)->GetOriginalProfile(),
-            Profile::FromBrowserContext(context));
+  DCHECK_EQ(profile, Profile::FromBrowserContext(context));
 
   // Pass the DataUseTabModel pointer to DataUseUITabModel.
   content::BrowserThread::PostTaskAndReplyWithResult(
@@ -76,6 +93,12 @@ KeyedService* DataUseUITabModelFactory::BuildServiceInstanceFor(
       base::Bind(&chrome::android::DataUseUITabModel::SetDataUseTabModel,
                  data_use_ui_tab_model->GetWeakPtr()));
 
+  content::BrowserThread::PostTask(
+      content::BrowserThread::IO, FROM_HERE,
+      base::Bind(&SetRegisterGoogleVariationIDOnIOThread,
+                 g_browser_process->io_thread(),
+                 signin_manager && signin_manager->IsAuthenticated()));
+
   return data_use_ui_tab_model;
 }
 

chrome/browser/android/data_usage/data_use_ui_tab_model_unittest.cc

@@ -21,6 +21,9 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/sessions/session_tab_helper.h"
 #include "chrome/common/url_constants.h"
+#include "chrome/test/base/testing_browser_process.h"
+#include "chrome/test/base/testing_profile.h"
+#include "chrome/test/base/testing_profile_manager.h"
 #include "components/data_usage/core/data_use_aggregator.h"
 #include "components/data_usage/core/data_use_amortizer.h"
 #include "components/data_usage/core/data_use_annotator.h"
@@ -115,6 +118,11 @@ class DataUseUITabModelTest : public testing::Test {
 
  protected:
   void SetUp() override {
+    profile_manager_.reset(
+        new TestingProfileManager(TestingBrowserProcess::GetGlobal()));
+    EXPECT_TRUE(profile_manager_->SetUp());
+    profile_ = profile_manager_->CreateTestingProfile("p1");
+
     io_task_runner_ = content::BrowserThread::GetTaskRunnerForThread(
         content::BrowserThread::IO);
     ui_task_runner_ = content::BrowserThread::GetTaskRunnerForThread(
@@ -141,6 +149,12 @@ class DataUseUITabModelTest : public testing::Test {
   DataUseUITabModel data_use_ui_tab_model_;
   scoped_refptr<base::SingleThreadTaskRunner> io_task_runner_;
   scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner_;
+
+  std::unique_ptr<TestingProfileManager> profile_manager_;
+
+  // Test profile used by the tests is owned by |profile_manager_|.
+  TestingProfile* profile_;
+
   std::unique_ptr<data_usage::DataUseAggregator> data_use_aggregator_;
   std::unique_ptr<ExternalDataUseObserver> external_data_use_observer_;
   std::unique_ptr<TestDataUseTabModel> data_use_tab_model_;

chrome/browser/android/data_usage/external_data_use_observer.cc

@@ -217,6 +217,19 @@ DataUseTabModel* ExternalDataUseObserver::GetDataUseTabModel() const {
   return data_use_tab_model_;
 }
 
+void ExternalDataUseObserver::SetRegisterGoogleVariationID(
+    bool register_google_variation_id) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  // It is okay to use base::Unretained here since
+  // |external_data_use_observer_bridge_| is owned by |this|, and is destroyed
+  // on UI thread when |this| is destroyed.
+  ui_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&ExternalDataUseObserverBridge::SetRegisterGoogleVariationID,
+                 base::Unretained(external_data_use_observer_bridge_),
+                 register_google_variation_id));
+}
+
 }  // namespace android
 
 }  // namespace chrome

chrome/browser/android/data_usage/external_data_use_observer.h

@@ -78,6 +78,8 @@ class ExternalDataUseObserver : public data_usage::DataUseAggregator::Observer {
     return external_data_use_reporter_;
   }
 
+  void SetRegisterGoogleVariationID(bool register_google_variation_id);
+
  private:
   friend class DataUseTabModelTest;
   friend class DataUseUITabModelTest;

chrome/browser/android/data_usage/external_data_use_observer_bridge.cc

@@ -61,7 +61,8 @@ namespace android {
 
 ExternalDataUseObserverBridge::ExternalDataUseObserverBridge()
     : construct_time_(base::TimeTicks::Now()),
-      is_first_matching_rule_fetch_(true) {
+      is_first_matching_rule_fetch_(true),
+      register_google_variation_id_(false) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
 
   // Detach from IO thread since rest of ExternalDataUseObserverBridge lives on
@@ -210,7 +211,11 @@ void ExternalDataUseObserverBridge::ShouldRegisterAsDataUseObserver(
       base::Bind(&ExternalDataUseObserver::ShouldRegisterAsDataUseObserver,
                  external_data_use_observer_, should_register));
 
+  if (!register_google_variation_id_)
+    return;
+
   variations::VariationID variation_id = GetGoogleVariationID();
+
   if (variation_id != variations::EMPTY_ID) {
     // Set variation id for the enabled group if |should_register| is true.
     // Otherwise clear the variation id for the enabled group by setting to
@@ -226,6 +231,12 @@ void ExternalDataUseObserverBridge::ShouldRegisterAsDataUseObserver(
   }
 }
 
+void ExternalDataUseObserverBridge::SetRegisterGoogleVariationID(
+    bool register_google_variation_id) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  register_google_variation_id_ = register_google_variation_id;
+}
+
 bool RegisterExternalDataUseObserver(JNIEnv* env) {
   return RegisterNativesImpl(env);
 }

chrome/browser/android/data_usage/external_data_use_observer_bridge.h

@@ -98,6 +98,8 @@ class ExternalDataUseObserverBridge {
   // should register as a data use observer.
   virtual void ShouldRegisterAsDataUseObserver(bool should_register) const;
 
+  void SetRegisterGoogleVariationID(bool register_google_variation_id);
+
  private:
   // Java listener that provides regular expressions to |this|. Data use
   // reports are submitted to |j_external_data_use_observer_|.
@@ -117,6 +119,9 @@ class ExternalDataUseObserverBridge {
   // True if matching rules are fetched for the first time.
   bool is_first_matching_rule_fetch_;
 
+  // True if Google variation ID should be registered.
+  bool register_google_variation_id_;
+
   // |io_task_runner_| accesses ExternalDataUseObserver members on IO thread.
   scoped_refptr<base::SingleThreadTaskRunner> io_task_runner_;
 

chrome/browser/android/data_usage/external_data_use_observer_unittest.cc

@@ -18,6 +18,9 @@
 #include "base/test/histogram_tester.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "chrome/browser/android/data_usage/data_use_tab_model.h"
+#include "chrome/test/base/testing_browser_process.h"
+#include "chrome/test/base/testing_profile.h"
+#include "chrome/test/base/testing_profile_manager.h"
 #include "components/data_usage/core/data_use.h"
 #include "components/data_usage/core/data_use_aggregator.h"
 #include "components/sessions/core/session_id.h"
@@ -45,6 +48,11 @@ class ExternalDataUseObserverTest : public testing::Test {
   void SetUp() override {
     thread_bundle_.reset(new content::TestBrowserThreadBundle(
         content::TestBrowserThreadBundle::IO_MAINLOOP));
+    profile_manager_.reset(
+        new TestingProfileManager(TestingBrowserProcess::GetGlobal()));
+    EXPECT_TRUE(profile_manager_->SetUp());
+    profile_ = profile_manager_->CreateTestingProfile("p1");
+
     io_task_runner_ = content::BrowserThread::GetTaskRunnerForThread(
         content::BrowserThread::IO);
     ui_task_runner_ = content::BrowserThread::GetTaskRunnerForThread(
@@ -121,6 +129,11 @@ class ExternalDataUseObserverTest : public testing::Test {
   std::unique_ptr<data_usage::DataUseAggregator> data_use_aggregator_;
   std::unique_ptr<ExternalDataUseObserver> external_data_use_observer_;
 
+  std::unique_ptr<TestingProfileManager> profile_manager_;
+
+  // Test profile used by the tests is owned by |profile_manager_|.
+  TestingProfile* profile_;
+
   scoped_refptr<base::SingleThreadTaskRunner> io_task_runner_;
   scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner_;
 };

chrome/browser/android/data_usage/external_data_use_reporter_unittest.cc

@@ -20,6 +20,9 @@
 #include "base/threading/thread_task_runner_handle.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/test/base/testing_browser_process.h"
+#include "chrome/test/base/testing_profile.h"
+#include "chrome/test/base/testing_profile_manager.h"
 #include "components/data_usage/core/data_use.h"
 #include "components/data_usage/core/data_use_aggregator.h"
 #include "components/sessions/core/session_id.h"
@@ -58,6 +61,11 @@ class ExternalDataUseReporterTest : public testing::Test {
   void SetUp() override {
     thread_bundle_.reset(new content::TestBrowserThreadBundle(
         content::TestBrowserThreadBundle::IO_MAINLOOP));
+    profile_manager_.reset(
+        new TestingProfileManager(TestingBrowserProcess::GetGlobal()));
+    EXPECT_TRUE(profile_manager_->SetUp());
+    profile_ = profile_manager_->CreateTestingProfile("p1");
+
     io_task_runner_ = content::BrowserThread::GetTaskRunnerForThread(
         content::BrowserThread::IO);
     ui_task_runner_ = content::BrowserThread::GetTaskRunnerForThread(
@@ -160,6 +168,11 @@ class ExternalDataUseReporterTest : public testing::Test {
   std::unique_ptr<data_usage::DataUseAggregator> data_use_aggregator_;
   std::unique_ptr<ExternalDataUseObserver> external_data_use_observer_;
 
+  std::unique_ptr<TestingProfileManager> profile_manager_;
+
+  // Test profile used by the tests is owned by |profile_manager_|.
+  TestingProfile* profile_;
+
   scoped_refptr<base::SingleThreadTaskRunner> io_task_runner_;
   scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner_;
 };