Retrieve the web&app activity enabled state from WebHistoryService.

components/history/core/browser/web_history_service.cc

Index: components/history/core/browser/web_history_service.cc
diff --git a/components/history/core/browser/web_history_service.cc b/components/history/core/browser/web_history_service.cc
index b46f6fbac5817cc61d481a17efbcb3fe7c9b5267..07a730c2fe8fcf780a518b41003eab588de4a367 100644
--- a/components/history/core/browser/web_history_service.cc
+++ b/components/history/core/browser/web_history_service.cc
@@ -44,6 +44,9 @@ const char kHistoryAudioHistoryUrl[] =
 const char kHistoryAudioHistoryChangeUrl[] =
     "https://history.google.com/history/api/change";
 
+const char kQueryWebAndAppActivityUrl[] =
+    "https://history.google.com/history/api/lookup?client=web_app";
+
 const char kPostDataMimeType[] = "text/plain";
 
 // The maximum number of retries for the URLFetcher requests.
@@ -437,10 +440,18 @@ size_t WebHistoryService::GetNumberOfPendingAudioHistoryRequests() {
   return pending_audio_history_requests_.size();
 }
 
-bool WebHistoryService::HasOtherFormsOfBrowsingHistory() const {
-  // TODO(msramek): Query history.google.com for existence of other forms of
-  // browsing history. In the meantime, assume that there isn't.
-  return false;
+void WebHistoryService::QueryWebAndAppActivity(
+    const QueryWebAndAppActivityCallback& callback) {
+  // Wrap the original callback into a generic completion callback.
+  CompletionCallback completion_callback =
+      base::Bind(&WebHistoryService::QueryWebAndAppActivityCompletionCallback,
+                 weak_ptr_factory_.GetWeakPtr(),
+                 callback);
+
+  GURL url(kQueryWebAndAppActivityUrl);
+  scoped_ptr<Request> request(CreateRequest(url, completion_callback));
+  request->Start();

[sdefresne, 2016/03/24 09:50:07]

You have a race condition here if the request finish (i.e. call
completion_callback) before you add it to pending_web_and_app_activity_requests_
I think. I see this is the pattern used in the rest of the file though, so maybe
you can leave as is.

I would do it like this:

// Request is deleted in the completion callback.
Request* request = CreateRequest(url, completion_callback);
pending_web_and_app_activity_requests_.insert(request);
request->Start();

[msramek, 2016/03/24 12:53:22]

Done.

Yes, I used the same approach as in other Query* methods. In reality it isn't a
race condition, since request->Start() will always end up posting a task on the
same thread, so the callback will not run before this method finishes. And the
unnecessary scoped_ptr looks like a copy-paste from QueryHistory which actually
needs it.

I'm happy to change the order to make sure that we don't make assumptions on the
internals of Request, but I'd prefer not to change the other four occurrences in
this CL.

+  pending_web_and_app_activity_requests_.insert(request.release());
 }
 
 // static
@@ -491,4 +502,25 @@ void WebHistoryService::AudioHistoryCompletionCallback(
   callback.Run(success && response_value, enabled_value);
 }
 
+void WebHistoryService::QueryWebAndAppActivityCompletionCallback(
+    const WebHistoryService::QueryWebAndAppActivityCallback& callback,
+    WebHistoryService::Request* request,
+    bool success) {
+  scoped_ptr<base::DictionaryValue> response_value;

[sdefresne, 2016/03/24 09:50:07]

The pattern for the other completion handler is to do the following:

pending_web_and_app_activity_requests_.erase(request);
scoped_ptr<Request> request_ptr(request);

This is safer as it will correctly cleanup the request even if the function is
changed to have an early return.

[msramek, 2016/03/24 12:53:22]

Done. I don't expect early returns in these straightforward methods, but I agree
that a scoped_ptr would be safer. Let me change it in
ExpireHistoryCompletionCallback as well (since I used that as the template).

+  bool web_and_app_activity_enabled = false;
+
+  if (success) {
+    response_value = ReadResponse(request);
+    if (response_value) {
+      response_value->GetBoolean(
+          "history_recording_enabled", &web_and_app_activity_enabled);
+    }
+  }
+
+  callback.Run(success && web_and_app_activity_enabled);
+
+  pending_web_and_app_activity_requests_.erase(request);
+  delete request;
+}
+
 }  // namespace history