Fix a crash in DownloadUIAddapter that happens when Java bridge is created/removed quickly while

components/offline_pages/downloads/download_ui_adapter.cc

Index: components/offline_pages/downloads/download_ui_adapter.cc
diff --git a/components/offline_pages/downloads/download_ui_adapter.cc b/components/offline_pages/downloads/download_ui_adapter.cc
index 906ae7b2379ff7199cceb979c96d79d1ee20446d..5cdb122f82c0ba2da760512215d12837d5d24231 100644
--- a/components/offline_pages/downloads/download_ui_adapter.cc
+++ b/components/offline_pages/downloads/download_ui_adapter.cc
@@ -29,6 +29,7 @@ DownloadUIAdapter::ItemInfo::~ItemInfo() {}
 DownloadUIAdapter::DownloadUIAdapter(OfflinePageModel* model)
     : model_(model),
       is_loaded_(false),
+      observers_count_(0),
       weak_ptr_factory_(this) {
 }
 
@@ -48,9 +49,10 @@ DownloadUIAdapter* DownloadUIAdapter::FromOfflinePageModel(
 
 void DownloadUIAdapter::AddObserver(Observer* observer) {
   DCHECK(observer);
-  if (!observers_.might_have_observers())
+  if (observers_count_ == 0)
     LoadCache();
   observers_.AddObserver(observer);
+  ++observers_count_;
   // If the items are already loaded, post the notification right away.
   // Don't just invoke it from here to avoid reentrancy in the client.
   if (is_loaded_) {
@@ -63,8 +65,9 @@ void DownloadUIAdapter::AddObserver(Observer* observer) {
 
 void DownloadUIAdapter::RemoveObserver(Observer* observer) {
   observers_.RemoveObserver(observer);
+  --observers_count_;
   // Once the last observer is gone, clear cached data.
-  if (!observers_.might_have_observers())
+  if (observers_count_ == 0)
     ClearCache();
 }
 
@@ -132,8 +135,9 @@ GURL DownloadUIAdapter::GetOfflineUrlByGuid(
   return GURL();
 }
 
+// Note that several LoadCache calls may be issued before the async GetAllPages
+// comes back.
 void DownloadUIAdapter::LoadCache() {
-  DCHECK(!is_loaded_);
   // TODO(dimich): Add fetching from RequestQueue as well.
   model_->AddObserver(this);

[dewittj, 2016/08/22 21:54:07]

I think you'll want to AddObserver after the GetAllPages returns.  If you
imagine a prior operation starting to add a page right before this happens, you
might get an observer method firing before OnOfflinePagesLoaded, which would
probably cause a DCHECK in OnOfflinePagesLoaded

   model_->GetAllPages(

[dewittj, 2016/08/22 21:54:07]

do you want a loading_ indicator so you don't GetAllPages multiple times?

@@ -149,6 +153,11 @@ void DownloadUIAdapter::ClearCache() {
 
 void DownloadUIAdapter::OnOfflinePagesLoaded(
     const MultipleOfflinePageItemResult& pages) {
+  // If multiple observers register quickly, the cache might be already loaded
+  // by the previous LoadCache call. At the same time, if all observers already
+  // left, there is no reason to populate the cache.
+  if (is_loaded_ || observers_count_ == 0)
+    return;
   for (const auto& page : pages) {
     if (IsVisibleInUI(page.client_id)) {
       std::string guid = page.client_id.id;