[SyncSetup]: Allow querying sWAA independently from sync.

As part of friendly settings project, we communicate the sWAA in sync
setup page. This CL enables to query the sWAA bit independently from
sync or history sync status. This is needed for the 2 following
scenarios during which the |WebHistoryService| is null:
1- The advanced sync setup flow (click on settings from the turn syn on
confirmation dialog).
2- Switching sync history from 'Off' to 'On'. This change does not take
effect until we close the |syncSetup| settings page.

Bug: 1035421
Change-Id: Id1651bd0d9508643e6218c6cef0c6f310109bef8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2012950Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Commit-Queue: Monica Basta <msalama@chromium.org>
Cr-Commit-Position: refs/heads/master@{#736295}

chrome/browser/ui/webui/settings/people_handler.cc

@@ -38,7 +38,6 @@
 #include "chrome/grit/generated_resources.h"
 #include "components/autofill/core/common/autofill_prefs.h"
 #include "components/browsing_data/core/history_notice_utils.h"
-#include "components/history/core/browser/web_history_service.h"
 #include "components/prefs/pref_service.h"
 #include "components/signin/core/browser/signin_error_controller.h"
 #include "components/signin/public/base/signin_metrics.h"
@@ -52,7 +51,9 @@
 #include "components/sync/driver/sync_service_utils.h"
 #include "components/sync/driver/sync_user_settings.h"
 #include "components/unified_consent/unified_consent_metrics.h"
+#include "content/public/browser/browser_context.h"
 #include "content/public/browser/render_view_host.h"
+#include "content/public/browser/storage_partition.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_delegate.h"
 #include "google_apis/gaia/gaia_auth_util.h"
@@ -491,18 +492,43 @@ void PeopleHandler::HandleGetIsHistoryRecordingEnabledAndCanBeUsed(
 
   DCHECK(base::FeatureList::IsEnabled(features::kSyncSetupFriendlySettings));
   syncer::SyncService* sync_service = GetSyncService();
-  history::WebHistoryService* history_service =
-      WebHistoryServiceFactory::GetForProfile(profile_);
+  if (!sync_service) {
+    OnQueryHistoryRecordingCompletion(webui_callback_id, nullptr,
+                                      base::nullopt);
+    return;
+  }
 
-  browsing_data::IsHistoryRecordingEnabledAndCanBeUsed(
-      sync_service, history_service,
-      base::Bind(&PeopleHandler::OnQueryHistoryRecordingCompletion,
+  if (sync_service->GetUserSettings()->IsUsingSecondaryPassphrase()) {
+    OnQueryHistoryRecordingCompletion(webui_callback_id, nullptr, false);
+    return;
+  }
+
+  std::unique_ptr<history::WebHistoryService::Request> request =
+      browsing_data::CreateQueryWebAndAppActivityRequest(
+          IdentityManagerFactory::GetForProfile(profile_),
+          content::BrowserContext::GetDefaultStoragePartition(profile_)
+              ->GetURLLoaderFactoryForBrowserProcess(),
+          base::BindOnce(&PeopleHandler::OnQueryHistoryRecordingCompletion,
                          weak_factory_.GetWeakPtr(), webui_callback_id));
+  DCHECK(request);
+  auto* request_ptr = request.get();
+  web_and_app_activity_requests_.insert(std::move(request));
+  request_ptr->Start();
 }
 
 void PeopleHandler::OnQueryHistoryRecordingCompletion(
     const std::string& webui_callback_id,
+    history::WebHistoryService::Request* request,
     const base::Optional<bool>& history_recording_enabled) {
+  if (request) {
+    auto it =
+        std::find_if(web_and_app_activity_requests_.begin(),
+                     web_and_app_activity_requests_.end(),
+                     [request](const auto& r) { return r.get() == request; });
+    DCHECK(web_and_app_activity_requests_.end() != it);
+    web_and_app_activity_requests_.erase(it);
+  }
+
   if (!IsJavascriptAllowed())
     return;
 

chrome/browser/ui/webui/settings/people_handler.h

@@ -16,6 +16,7 @@
 #include "build/buildflag.h"
 #include "chrome/browser/ui/webui/settings/settings_page_ui_handler.h"
 #include "chrome/browser/ui/webui/signin/login_ui_service.h"
+#include "components/history/core/browser/web_history_service.h"
 #include "components/prefs/pref_change_registrar.h"
 #include "components/signin/public/base/signin_buildflags.h"
 #include "components/signin/public/identity_manager/identity_manager.h"
@@ -186,6 +187,7 @@ class PeopleHandler : public SettingsPageUIHandler,
 
   void OnQueryHistoryRecordingCompletion(
       const std::string& webui_callback_id,
+      history::WebHistoryService::Request* request,
       const base::Optional<bool>& history_recording_enabled);
 
   void HandleGetStoredAccounts(const base::ListValue* args);
@@ -231,6 +233,11 @@ class PeopleHandler : public SettingsPageUIHandler,
   // Used to listen for pref changes to allow or disallow signin.
   PrefChangeRegistrar profile_pref_registrar_;
 
+  // Pending web and app activity requests to query whether history recording
+  // is enabled or not.
+  std::set<std::unique_ptr<history::WebHistoryService::Request>>
+      web_and_app_activity_requests_;
+
   // Manages observer lifetimes.
   ScopedObserver<signin::IdentityManager, signin::IdentityManager::Observer>
       identity_manager_observer_{this};

components/browsing_data/core/BUILD.gn

@@ -39,10 +39,12 @@ static_library("core") {
     "//components/password_manager/core/browser",
     "//components/pref_registry",
     "//components/prefs",
+    "//components/signin/public/identity_manager",
     "//components/strings",
     "//components/sync",
     "//components/version_info",
     "//components/webdata/common",
+    "//services/network/public/cpp",
     "//ui/base",
   ]
 }

components/browsing_data/core/DEPS

@@ -7,6 +7,7 @@ include_rules = [
   "+components/password_manager/core/browser",
   "+components/pref_registry",
   "+components/prefs",
+  "+components/signin/public/identity_manager",
   "+components/strings/grit/components_strings.h",
   "+components/sync/base",
   "+components/sync/driver",
@@ -14,5 +15,6 @@ include_rules = [
   "+components/version_info",
   "+components/webdata/common",
   "+net",
+  "+services/network/public",
   "+ui/base",
 ]

components/browsing_data/core/history_notice_utils.cc

@@ -10,12 +10,36 @@
 #include "base/single_thread_task_runner.h"
 #include "base/strings/stringprintf.h"
 #include "base/threading/thread_task_runner_handle.h"
-#include "components/history/core/browser/web_history_service.h"
 #include "components/sync/driver/sync_service.h"
 #include "components/sync/driver/sync_user_settings.h"
 #include "components/version_info/version_info.h"
 
 namespace {
+const char kQueryWebAndAppActivityPartialNetworkAnnotation[] = R"(
+        semantics {
+          description:
+            "Queries history.google.com to find out if user has the 'Include "
+            "Chrome browsing history and activity from websites and apps that "
+            "use Google services' option enabled in the Activity controls of "
+            "their Google account. This is done for users who sync their "
+            "browsing history without a custom passphrase in order to show "
+            "information about history.google.com on the history page, "
+            "the settings sync setup page and in the Clear Browsing Data "
+            "dialog."
+          trigger:
+            "This request is sent when user opens the history page or the "
+            "settings sync setup page or the Clear Browsing Data dialog and "
+            "history sync without a custom passphrase is (re)enabled."
+          data:
+            "An OAuth2 token authenticating the user."
+        }
+        policy {
+          chrome_policy {
+            SyncDisabled {
+              SyncDisabled: true
+            }
+          }
+        })";
 
 // Merges several asynchronous boolean callbacks into one that returns a boolean
 // product of their responses. Deletes itself when done.
@@ -67,6 +91,24 @@ void ShouldShowNoticeAboutOtherFormsOfBrowsingHistory(
           std::move(callback)));
 }
 
+std::unique_ptr<history::WebHistoryService::Request>
+CreateQueryWebAndAppActivityRequest(
+    signin::IdentityManager* identity_manager,
+    scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
+    base::OnceCallback<void(history::WebHistoryService::Request*,
+                            const base::Optional<bool>&)> callback) {
+  DCHECK(identity_manager);
+  DCHECK(url_loader_factory);
+  net::PartialNetworkTrafficAnnotationTag partial_traffic_annotation =
+      net::DefinePartialNetworkTrafficAnnotation(
+          "history_recording_enabled", "web_history_service",
+          kQueryWebAndAppActivityPartialNetworkAnnotation);
+
+  return history::WebHistoryService::CreateQueryWebAndAppActivityRequest(
+      identity_manager, url_loader_factory, std::move(callback),
+      partial_traffic_annotation);
+}
+
 void IsHistoryRecordingEnabledAndCanBeUsed(
     const syncer::SyncService* sync_service,
     history::WebHistoryService* history_service,
@@ -87,31 +129,10 @@ void IsHistoryRecordingEnabledAndCanBeUsed(
   }
 
   net::PartialNetworkTrafficAnnotationTag partial_traffic_annotation =
-      net::DefinePartialNetworkTrafficAnnotation("history_recording_enabled",
-                                                 "web_history_service", R"(
-      semantics {
-        description:
-          "Queries history.google.com to find out if user has the 'Include "
-          "Chrome browsing history and activity from websites and apps that "
-          "use Google services' option enabled in the Activity controls of "
-          "their Google account. This is done for users who sync their "
-          "browsing history without a custom passphrase in order to show "
-          "information about history.google.com on the history page, "
-          "the settings sync setup page and in the Clear Browsing Data dialog."
-        trigger:
-          "This request is sent when user opens the history page or the "
-          "settings sync setup page or the Clear Browsing Data dialog and "
-          "history sync without a custom passphrase is (re)enabled."
-        data:
-          "An OAuth2 token authenticating the user."
-      }
-      policy {
-        chrome_policy {
-          SyncDisabled {
-            SyncDisabled: true
-          }
-        }
-      })");
+      net::DefinePartialNetworkTrafficAnnotation(
+          "history_recording_enabled", "web_history_service",
+          kQueryWebAndAppActivityPartialNetworkAnnotation);
+
   history_service->QueryWebAndAppActivity(std::move(callback),
                                           partial_traffic_annotation);
 }

components/browsing_data/core/history_notice_utils.h

@@ -9,10 +9,9 @@
 
 #include "base/callback_forward.h"
 #include "base/optional.h"
-
-namespace history {
-class WebHistoryService;
-}
+#include "components/history/core/browser/web_history_service.h"
+#include "components/signin/public/identity_manager/identity_manager.h"
+#include "services/network/public/cpp/shared_url_loader_factory.h"
 
 namespace syncer {
 class SyncService;
@@ -24,16 +23,33 @@ enum class Channel;
 
 namespace browsing_data {
 
-// Whether history recording is enabled and can be used to provide personalized
-// experience. The prior is true if all the following is true:
-// - User is syncing and sync history is on.
-// - Data is not encrypted with custom passphrase.
-// - User has enabled 'Include Chrome browsing history and activity from
-//   websites and apps that use Google services' in the |Web and App Activity|
-//   of their Google Account.
-// The response is returned in |callback|. The response is base::nullopt if the
-// sWAA bit could not be retrieved (i.e. the history service has not been
-// created yet).
+// Returns a request that can be used to query |Web and App Activity|. It can be
+// made independently from the history sync state and its lifetime needs to be
+// managed by the caller.
+//
+// Once the request is completed, |callback| is called with the following
+// arguments:
+//   * a pointer to the request associated with the response.
+//   * a base::Optional<bool> that indicated whether the user has enabled
+//     'Include Chrome browsing history and activity from websites and apps that
+//     use Google services' in the |Web and App Activity| for their Google
+//     Account. This argument is base::nullopt if the request to fetch the
+//     |Web and App Activity| information failed.
+std::unique_ptr<history::WebHistoryService::Request>
+CreateQueryWebAndAppActivityRequest(
+    signin::IdentityManager* identity_manager,
+    scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
+    base::OnceCallback<void(history::WebHistoryService::Request*,
+                            const base::Optional<bool>&)> callback);
+
+// The response is returned in the |callback|. It can be:
+// * nullopt: If we fail to query the |Web And App Activity| or history sync is
+//   not fully active yet.
+// * True: If the user has enabled 'Include Chrome browsing history and activity
+//   from websites and apps that use Google services' in the
+//   |Web and App Activity| of their Google Account, data is not encrypted with
+//   custom passphrase and history sync is active.
+// * False: Otherwise.
 void IsHistoryRecordingEnabledAndCanBeUsed(
     const syncer::SyncService* sync_service,
     history::WebHistoryService* history_service,

components/history/core/browser/web_history_service.cc

@@ -526,6 +526,30 @@ void WebHistoryService::QueryWebAndAppActivity(
   request->Start();
 }
 
+// static
+std::unique_ptr<history::WebHistoryService::Request>
+WebHistoryService::CreateQueryWebAndAppActivityRequest(
+    signin::IdentityManager* identity_manager,
+    scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
+    QueryWebAndAppActivityWithRequestCallback callback,
+    const net::PartialNetworkTrafficAnnotationTag& partial_traffic_annotation) {
+  CompletionCallback completion_callback = base::BindOnce(
+      [](QueryWebAndAppActivityWithRequestCallback callback,
+         WebHistoryService::Request* request, bool success) {
+        std::move(callback).Run(
+            request,
+            WebHistoryService::ReportQueryWebAndAppActivity(request, success));
+      },
+      std::move(callback));
+
+  GURL url(kQueryWebAndAppActivityUrl);
+  std::unique_ptr<history::WebHistoryService::Request> request =
+      base::WrapUnique(new RequestImpl(identity_manager, url_loader_factory,
+                                       url, std::move(completion_callback),
+                                       partial_traffic_annotation));
+  return request;
+}
+
 void WebHistoryService::QueryOtherFormsOfBrowsingHistory(
     version_info::Channel channel,
     QueryOtherFormsOfBrowsingHistoryCallback callback,
@@ -628,18 +652,7 @@ void WebHistoryService::QueryWebAndAppActivityCompletionCallback(
       std::move(pending_web_and_app_activity_requests_[request]);
   pending_web_and_app_activity_requests_.erase(request);
 
-  std::unique_ptr<base::DictionaryValue> response_value;
-  if (success) {
-    response_value = ReadResponse(request);
-    bool web_and_app_activity_enabled = false;
-    if (response_value &&
-        response_value->GetBoolean("history_recording_enabled",
-                                   &web_and_app_activity_enabled)) {
-      std::move(callback).Run(web_and_app_activity_enabled);
-      return;
-    }
-  }
-  std::move(callback).Run(base::nullopt);
+  std::move(callback).Run(ReportQueryWebAndAppActivity(request, success));
 }
 
 void WebHistoryService::QueryOtherFormsOfBrowsingHistoryCompletionCallback(
@@ -660,4 +673,21 @@ void WebHistoryService::QueryOtherFormsOfBrowsingHistoryCompletionCallback(
   std::move(callback).Run(has_other_forms_of_browsing_history);
 }
 
+// static
+base::Optional<bool> WebHistoryService::ReportQueryWebAndAppActivity(
+    WebHistoryService::Request* request,
+    bool success) {
+  std::unique_ptr<base::DictionaryValue> response_value;
+  if (success) {
+    response_value = ReadResponse(request);
+    bool web_and_app_activity_enabled = false;
+    if (response_value &&
+        response_value->GetBoolean("history_recording_enabled",
+                                   &web_and_app_activity_enabled)) {
+      return web_and_app_activity_enabled;
+    }
+  }
+  return base::nullopt;
+}
+
 }  // namespace history

components/history/core/browser/web_history_service.h

@@ -92,6 +92,10 @@ class WebHistoryService : public KeyedService {
   using QueryWebAndAppActivityCallback = base::OnceCallback<void(
       const base::Optional<bool>& history_recording_enabled)>;
 
+  using QueryWebAndAppActivityWithRequestCallback = base::OnceCallback<void(
+      WebHistoryService::Request* request,
+      const base::Optional<bool>& history_recording_enabled)>;
+
   using QueryOtherFormsOfBrowsingHistoryCallback =
       base::OnceCallback<void(bool success)>;
 
@@ -153,6 +157,18 @@ class WebHistoryService : public KeyedService {
       const net::PartialNetworkTrafficAnnotationTag&
           partial_traffic_annotation);
 
+  // Returns a request to query whether web and app activity is enabled on the
+  // server. The request can be made independently from sync state. The caller
+  // must make sure that the |identity_manager| outlives the returned request
+  // object.
+  static std::unique_ptr<history::WebHistoryService::Request>
+  CreateQueryWebAndAppActivityRequest(
+      signin::IdentityManager* identity_manager,
+      scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
+      QueryWebAndAppActivityWithRequestCallback callback,
+      const net::PartialNetworkTrafficAnnotationTag&
+          partial_traffic_annotation);
+
   // Used for tests.
   size_t GetNumberOfPendingAudioHistoryRequests();
 
@@ -219,6 +235,11 @@ class WebHistoryService : public KeyedService {
  private:
   friend class WebHistoryServiceTest;
 
+  // Extracts from the request's response if history recording is enabled.
+  static base::Optional<bool> ReportQueryWebAndAppActivity(
+      WebHistoryService::Request* request,
+      bool success);
+
   // Stores pointer to IdentityManager instance. It must outlive the
   // WebHistoryService and can be null during tests.
   signin::IdentityManager* identity_manager_;