Make BrowsingDataHandler observe WebHistoryService deletions

chrome/browser/ui/webui/browsing_history_handler.cc

Index: chrome/browser/ui/webui/browsing_history_handler.cc
diff --git a/chrome/browser/ui/webui/browsing_history_handler.cc b/chrome/browser/ui/webui/browsing_history_handler.cc
index 3619109dde138b85c0a0633a907e07ce6668ec6a..447282b8f14fe5a6455ce68491c140c44b88931b 100644
--- a/chrome/browser/ui/webui/browsing_history_handler.cc
+++ b/chrome/browser/ui/webui/browsing_history_handler.cc
@@ -50,6 +50,7 @@
 #include "components/query_parser/snippet.h"
 #include "components/sync/device_info/device_info.h"
 #include "components/sync/device_info/device_info_tracker.h"
+#include "components/sync/driver/sync_service_observer.h"
 #include "components/sync/protocol/history_delete_directive_specifics.pb.h"
 #include "components/sync/protocol/sync_enums.pb.h"
 #include "components/url_formatter/url_formatter.h"
@@ -316,10 +317,11 @@ bool BrowsingHistoryHandler::HistoryEntry::SortByTimeDescending(
 BrowsingHistoryHandler::BrowsingHistoryHandler()
     : has_pending_delete_request_(false),
       history_service_observer_(this),
+      web_history_service_observer_(this),
+      sync_service_observer_(this),
       has_synced_results_(false),
       has_other_forms_of_browsing_history_(false),
-      weak_factory_(this) {
-}
+      weak_factory_(this) {}
 
 BrowsingHistoryHandler::~BrowsingHistoryHandler() {
   query_task_tracker_.TryCancelAll();
@@ -343,10 +345,25 @@ void BrowsingHistoryHandler::RegisterMessages() {
 #endif
 
   // Get notifications when history is cleared.
-  history::HistoryService* hs = HistoryServiceFactory::GetForProfile(
+  history::HistoryService* local_history = HistoryServiceFactory::GetForProfile(
       profile, ServiceAccessType::EXPLICIT_ACCESS);
-  if (hs)
-    history_service_observer_.Add(hs);
+  if (local_history)
+    history_service_observer_.Add(local_history);
+
+  // Get notifications when web history is deleted.
+  history::WebHistoryService* web_history =
+      WebHistoryServiceFactory::GetForProfile(profile);
+  if (web_history) {
+    web_history_service_observer_.Add(web_history);
+  } else {
+    // If |web_history| is not available, it means that the history sync is
+    // disabled. Observe |sync_service| so that we can attach the listener
+    // in case it gets enabled later.
+    ProfileSyncService* sync_service =
+        ProfileSyncServiceFactory::GetForProfile(profile);
+    if (sync_service)
+      sync_service_observer_.Add(sync_service);
+  }
 
   web_ui()->RegisterMessageCallback("queryHistory",
       base::Bind(&BrowsingHistoryHandler::HandleQueryHistory,
@@ -375,6 +392,18 @@ bool BrowsingHistoryHandler::ExtractIntegerValueAtIndex(
   return false;
 }
 
+void BrowsingHistoryHandler::OnStateChanged() {
+  // If the history sync was enabled, start observing WebHistoryService.
+  // This method should not be called after we already added the observer.
+  history::WebHistoryService* web_history =
+      WebHistoryServiceFactory::GetForProfile(Profile::FromWebUI(web_ui()));
+  if (web_history) {
+    DCHECK(!web_history_service_observer_.IsObserving(web_history));
+    web_history_service_observer_.Add(web_history);
+    sync_service_observer_.RemoveAll();
+  }
+}
+
 void BrowsingHistoryHandler::WebHistoryTimeout() {
   // TODO(dubroy): Communicate the failure to the front end.
   if (!query_task_tracker_.HasTrackedTasks())
@@ -960,3 +989,7 @@ void BrowsingHistoryHandler::OnURLsDeleted(
   if (all_history || DeletionsDiffer(deleted_rows, urls_to_be_deleted_))
     web_ui()->CallJavascriptFunctionUnsafe("historyDeleted");
 }
+
+void BrowsingHistoryHandler::OnWebHistoryDeleted() {
+  web_ui()->CallJavascriptFunctionUnsafe("historyDeleted");

[Dan Beam, 2016/08/25 19:20:10]

note: if the page is in the middle of a reload when this is called, the message
may be dropped (and might eventually crash chrome if we re-instate a CHECK()
that once was here)

we've slowly been changing things to OnJavascript{Allowed,Disallowed} throughout
webui.

just FYI, that's why it's named "Unsafe"

[msramek, 2016/08/25 20:21:34]

Thanks for checking.

On the JS side, "historyDeleted" should do exactly that - reload the page. If
the request is dropped during a reload, that's not a problem - it's already
doing what it's supposed to.

If it doesn't cause a crash for other reasons, I would still prefer to go
forward with this CL, as I'm trying to fix a long-standing user complaint. I'm
happy to work with you on a followup where we apply
OnJavascript{Allowed,Disallowed}.

+}