[Offline pages] Preventing GetPagesMatchingQuery from running before model is loaded

components/offline_pages/offline_page_model_impl.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/offline_pages/offline_page_model_impl.h"
 
 #include <algorithm>
 #include <limits>
 
 #include "base/bind.h"
@@ skipping 423 matching lines @@
 void OfflinePageModelImpl::DeletePagesByClientIds(
     const std::vector<ClientId>& client_ids,
     const DeletePageCallback& callback) {
   OfflinePageModelQueryBuilder builder;
   builder
       .SetClientIds(OfflinePageModelQuery::Requirement::INCLUDE_MATCHING,
                     client_ids)
       .AllowExpiredPages(true);
   auto delete_pages = base::Bind(&OfflinePageModelImpl::DeletePages,
                                  weak_ptr_factory_.GetWeakPtr(), callback);
-  RunWhenLoaded(base::Bind(&OfflinePageModelImpl::GetPagesMatchingQuery,
-                           weak_ptr_factory_.GetWeakPtr(),
-                           base::Passed(builder.Build(GetPolicyController())),
-                           delete_pages));
+  RunWhenLoaded(base::Bind(
+      &OfflinePageModelImpl::GetPagesMatchingQueryWhenLoadDone,
+      weak_ptr_factory_.GetWeakPtr(),
+      base::Passed(builder.Build(GetPolicyController())), delete_pages));
 }
 
 void OfflinePageModelImpl::DeletePages(
     const DeletePageCallback& callback,
     const MultipleOfflinePageItemResult& pages) {
   DCHECK(is_loaded_);
 
   std::vector<int64_t> offline_ids;
   for (auto& page : pages)
     offline_ids.emplace_back(page.offline_id);
 
   DoDeletePagesByOfflineId(offline_ids, callback);
 }
 
+void OfflinePageModelImpl::GetPagesMatchingQuery(
+    std::unique_ptr<OfflinePageModelQuery> query,
+    const MultipleOfflinePageItemCallback& callback) {
+  RunWhenLoaded(base::Bind(
+      &OfflinePageModelImpl::GetPagesMatchingQueryWhenLoadDone,
+      weak_ptr_factory_.GetWeakPtr(), base::Passed(&query), callback));
+}
+
+void OfflinePageModelImpl::GetPagesMatchingQueryWhenLoadDone(
+    std::unique_ptr<OfflinePageModelQuery> query,
+    const MultipleOfflinePageItemCallback& callback) {
+  DCHECK(query);
+  DCHECK(is_loaded_);
+
+  MultipleOfflinePageItemResult offline_pages_result;
+
+  for (const auto& id_page_pair : offline_pages_) {
+    if (query->Matches(id_page_pair.second))
+      offline_pages_result.emplace_back(id_page_pair.second);
+  }
+
+  callback.Run(offline_pages_result);
+}
+
 void OfflinePageModelImpl::GetPagesByClientIds(
     const std::vector<ClientId>& client_ids,
     const MultipleOfflinePageItemCallback& callback) {
   OfflinePageModelQueryBuilder builder;
   builder.SetClientIds(OfflinePageModelQuery::Requirement::INCLUDE_MATCHING,
                        client_ids);
-  RunWhenLoaded(base::Bind(&OfflinePageModelImpl::GetPagesMatchingQuery,
-                           weak_ptr_factory_.GetWeakPtr(),
-                           base::Passed(builder.Build(GetPolicyController())),
-                           callback));
+  RunWhenLoaded(

[dewittj, 2016/11/28 17:52:09]

This could simply be rewritten as GetPagesMatchingQuery(...)

[fgorski, 2016/11/28 18:30:58]

I know, but it sticks to the pattern applied in this file.

+      base::Bind(&OfflinePageModelImpl::GetPagesMatchingQueryWhenLoadDone,
+                 weak_ptr_factory_.GetWeakPtr(),
+                 base::Passed(builder.Build(GetPolicyController())), callback));
 }
 
 void OfflinePageModelImpl::DeleteCachedPagesByURLPredicate(
     const UrlPredicate& predicate,
     const DeletePageCallback& callback) {
   RunWhenLoaded(
       base::Bind(&OfflinePageModelImpl::DoDeleteCachedPagesByURLPredicate,
                  weak_ptr_factory_.GetWeakPtr(), predicate, callback));
 }
 
@@ skipping 23 matching lines @@
           OfflinePageModelQueryBuilder::Requirement::EXCLUDE_MATCHING);
   auto pages_to_urls = base::Bind(
       [](const CheckPagesExistOfflineCallback& callback,
          const MultipleOfflinePageItemResult& pages) {
         CheckPagesExistOfflineResult result;
         for (auto& page : pages)
           result.insert(page.url);
         callback.Run(result);
       },
       callback);
-  RunWhenLoaded(base::Bind(&OfflinePageModelImpl::GetPagesMatchingQuery,
-                           weak_ptr_factory_.GetWeakPtr(),
-                           base::Passed(builder.Build(GetPolicyController())),
-                           pages_to_urls));
-}
-
-void OfflinePageModelImpl::GetPagesMatchingQuery(
-    std::unique_ptr<OfflinePageModelQuery> query,
-    const MultipleOfflinePageItemCallback& callback) {
-  DCHECK(query);
-
-  MultipleOfflinePageItemResult offline_pages_result;
-
-  for (const auto& id_page_pair : offline_pages_) {
-    if (query->Matches(id_page_pair.second))
-      offline_pages_result.emplace_back(id_page_pair.second);
-  }
-
-  callback.Run(offline_pages_result);
+  RunWhenLoaded(base::Bind(
+      &OfflinePageModelImpl::GetPagesMatchingQueryWhenLoadDone,
+      weak_ptr_factory_.GetWeakPtr(),
+      base::Passed(builder.Build(GetPolicyController())), pages_to_urls));
 }
 
 void OfflinePageModelImpl::GetAllPages(
     const MultipleOfflinePageItemCallback& callback) {
   OfflinePageModelQueryBuilder builder;
-  RunWhenLoaded(base::Bind(&OfflinePageModelImpl::GetPagesMatchingQuery,
-                           weak_ptr_factory_.GetWeakPtr(),
-                           base::Passed(builder.Build(GetPolicyController())),
-                           callback));
+  RunWhenLoaded(
+      base::Bind(&OfflinePageModelImpl::GetPagesMatchingQueryWhenLoadDone,
+                 weak_ptr_factory_.GetWeakPtr(),
+                 base::Passed(builder.Build(GetPolicyController())), callback));
 }
 
 void OfflinePageModelImpl::GetAllPagesWithExpired(
     const MultipleOfflinePageItemCallback& callback) {
   OfflinePageModelQueryBuilder builder;
   builder.AllowExpiredPages(true);
 
-  RunWhenLoaded(base::Bind(&OfflinePageModelImpl::GetPagesMatchingQuery,
-                           weak_ptr_factory_.GetWeakPtr(),
-                           base::Passed(builder.Build(GetPolicyController())),
-                           callback));
+  RunWhenLoaded(
+      base::Bind(&OfflinePageModelImpl::GetPagesMatchingQueryWhenLoadDone,
+                 weak_ptr_factory_.GetWeakPtr(),
+                 base::Passed(builder.Build(GetPolicyController())), callback));
 }
 
 void OfflinePageModelImpl::GetOfflineIdsForClientId(
     const ClientId& client_id,
     const MultipleOfflineIdCallback& callback) {
   RunWhenLoaded(
       base::Bind(&OfflinePageModelImpl::GetOfflineIdsForClientIdWhenLoadDone,
                  weak_ptr_factory_.GetWeakPtr(), client_id, callback));
 }
 
 void OfflinePageModelImpl::GetOfflineIdsForClientIdWhenLoadDone(
     const ClientId& client_id,
     const MultipleOfflineIdCallback& callback) const {
+  DCHECK(is_loaded_);
   callback.Run(MaybeGetOfflineIdsForClientId(client_id));
 }
 
 const std::vector<int64_t> OfflinePageModelImpl::MaybeGetOfflineIdsForClientId(
     const ClientId& client_id) const {
   DCHECK(is_loaded_);
   std::vector<int64_t> results;
 
   // We want only all pages, including those marked for deletion.
   // TODO(fgorski): actually use an index rather than linear scan.
@@ skipping 21 matching lines @@
          const MultipleOfflinePageItemResult& result) {
         DCHECK_LE(result.size(), 1U);
         if (result.empty()) {
           callback.Run(nullptr);
         } else {
           callback.Run(&result[0]);
         }
       },
       callback);
 
-  RunWhenLoaded(base::Bind(&OfflinePageModelImpl::GetPagesMatchingQuery,
-                           weak_ptr_factory_.GetWeakPtr(),
-                           base::Passed(builder.Build(GetPolicyController())),
-                           multiple_callback));
+  RunWhenLoaded(base::Bind(
+      &OfflinePageModelImpl::GetPagesMatchingQueryWhenLoadDone,
+      weak_ptr_factory_.GetWeakPtr(),
+      base::Passed(builder.Build(GetPolicyController())), multiple_callback));
 }
 
 void OfflinePageModelImpl::GetPagesByURL(
     const GURL& url,
     URLSearchMode url_search_mode,
     const MultipleOfflinePageItemCallback& callback) {
   RunWhenLoaded(
       base::Bind(&OfflinePageModelImpl::GetPagesByURLWhenLoadDone,
                  weak_ptr_factory_.GetWeakPtr(), url,
                  url_search_mode, callback));
 }
 
 void OfflinePageModelImpl::GetPagesByURLWhenLoadDone(
     const GURL& url,
     URLSearchMode url_search_mode,
     const MultipleOfflinePageItemCallback& callback) const {
+  DCHECK(is_loaded_);
   std::vector<OfflinePageItem> result;
 
   GURL::Replacements remove_params;
   remove_params.ClearRef();
 
   GURL url_without_fragment =
       url.ReplaceComponents(remove_params);
 
   for (const auto& id_page_pair : offline_pages_) {
     if (id_page_pair.second.IsExpired())
@@ skipping 506 matching lines @@
   }
 
   base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, task);
 }
 
 base::Time OfflinePageModelImpl::GetCurrentTime() const {
   return testing_clock_ ? testing_clock_->Now() : base::Time::Now();
 }
 
 }  // namespace offline_pages