android: update DataObserver to remove observer

DataObserver adds itself as an observer to HistoryService, but never
removes itself. This also converts to using ScopedObservation.

BUG=1158388
TEST=none

Change-Id: Ia438428ce0ecd7b8207be5bdc38bd448817300a9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2612125Reviewed-by: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#841061}

chrome/browser/android/history_report/data_observer.cc

@@ -25,28 +25,23 @@ DataObserver::DataObserver(
     UsageReportsBufferService* usage_reports_buffer_service,
     BookmarkModel* bookmark_model,
     history::HistoryService* history_service)
-    : bookmark_model_(bookmark_model),
-      data_changed_callback_(data_changed_callback),
+    : data_changed_callback_(data_changed_callback),
       data_cleared_callback_(data_cleared_callback),
       stop_reporting_callback_(stop_reporting_callback),
       delta_file_service_(delta_file_service),
       usage_reports_buffer_service_(usage_reports_buffer_service) {
-  bookmark_model_->AddObserver(this);
-  history_service->AddObserver(this);
+  scoped_bookmark_model_observer_.Observe(bookmark_model);
+  scoped_history_service_observer_.Observe(history_service);
 }
 
-DataObserver::~DataObserver() {
-  if (bookmark_model_)
-    bookmark_model_->RemoveObserver(this);
-}
+DataObserver::~DataObserver() = default;
 
 void DataObserver::BookmarkModelLoaded(BookmarkModel* model,
                                        bool ids_reassigned) {}
 
 void DataObserver::BookmarkModelBeingDeleted(BookmarkModel* model) {
-  DCHECK_EQ(model, bookmark_model_);
-  bookmark_model_->RemoveObserver(this);
-  bookmark_model_ = nullptr;
+  DCHECK(scoped_bookmark_model_observer_.IsObservingSource(model));
+  scoped_bookmark_model_observer_.Reset();
 }
 
 void DataObserver::BookmarkNodeMoved(BookmarkModel* model,
@@ -146,4 +141,9 @@ void DataObserver::OnURLsDeleted(history::HistoryService* history_service,
   }
 }
 
+void DataObserver::HistoryServiceBeingDeleted(
+    history::HistoryService* history_service) {
+  scoped_history_service_observer_.Reset();
+}
+
 }  // namespace history_report

chrome/browser/android/history_report/data_observer.h

@@ -9,8 +9,11 @@
 
 #include "base/callback.h"
 #include "base/macros.h"
+#include "base/scoped_observation.h"
 #include "base/time/time.h"
+#include "components/bookmarks/browser/bookmark_model.h"
 #include "components/bookmarks/browser/bookmark_model_observer.h"
+#include "components/history/core/browser/history_service.h"
 #include "components/history/core/browser/history_service_observer.h"
 #include "components/history/core/browser/history_types.h"
 #include "components/history/core/browser/url_row.h"
@@ -76,11 +79,18 @@ class DataObserver : public bookmarks::BookmarkModelObserver,
                       const history::URLRows& changed_urls) override;
   void OnURLsDeleted(history::HistoryService* history_service,
                      const history::DeletionInfo& deletion_info) override;
+  void HistoryServiceBeingDeleted(
+      history::HistoryService* history_service) override;
 
  private:
   void DeleteBookmarks(const std::set<GURL>& removed_urls);
 
-  bookmarks::BookmarkModel* bookmark_model_;
+  base::ScopedObservation<bookmarks::BookmarkModel,
+                          bookmarks::BookmarkModelObserver>
+      scoped_bookmark_model_observer_{this};
+  base::ScopedObservation<history::HistoryService,
+                          history::HistoryServiceObserver>
+      scoped_history_service_observer_{this};
   base::RepeatingCallback<void(void)> data_changed_callback_;
   base::RepeatingCallback<void(void)> data_cleared_callback_;
   base::RepeatingCallback<void(void)> stop_reporting_callback_;

chrome/browser/android/history_report/data_observer_unittest.cc

@@ -79,6 +79,9 @@ class DataObserverTest : public testing::Test {
     delta_file_service_.reset();
     usage_report_service_.reset();
     bookmark_model_.reset();
+    // As this code does not call HistoryService::Init(), HistoryService
+    // doesn't call HistoryServiceBeingDeleted().
+    data_observer_->HistoryServiceBeingDeleted(history_service_.get());
     history_service_.reset();
     data_observer_.reset();
   }