[Offline pages] Fixing crashes caused by access to offline pages marked for deletion

components/offline_pages/offline_page_model.cc

Index: components/offline_pages/offline_page_model.cc
diff --git a/components/offline_pages/offline_page_model.cc b/components/offline_pages/offline_page_model.cc
index 242ade295c21c5ffb70575f1147708045042b194..1304b70b6ae7efb1c81a90cdae87c1c12c30eba9 100644
--- a/components/offline_pages/offline_page_model.cc
+++ b/components/offline_pages/offline_page_model.cc
@@ -253,7 +253,13 @@ void OfflinePageModel::ClearAll(const base::Closure& callback) {
 
 bool OfflinePageModel::HasOfflinePages() const {
   DCHECK(is_loaded_);
-  return !offline_pages_.empty();
+  // Check that at least one page is not marked for deletion. Because we have
+  // pages marked for deletion, we cannot simply invert result of |empty()|.
+  for (const auto& iter : offline_pages_) {

[Michael Courage, 2015/11/25 19:00:58]

optional nit: seems like there's a pretty even split between "iter" and
"id_page_pair" in this file, but the other nearby instances of "for (iterator :
offline_pages_)" go with the latter, which seems a little bit clearer to me.

[fgorski, 2015/11/25 19:27:26]

Done.

+    if (!iter.second.IsMarkedForDeletion())
+      return true;
+  }
+  return false;
 }
 
 const std::vector<OfflinePageItem> OfflinePageModel::GetAllPages() const {
@@ -283,7 +289,9 @@ const std::vector<OfflinePageItem> OfflinePageModel::GetPagesToCleanUp() const {
 const OfflinePageItem* OfflinePageModel::GetPageByBookmarkId(
     int64 bookmark_id) const {
   const auto iter = offline_pages_.find(bookmark_id);
-  return iter != offline_pages_.end() ? &(iter->second) : nullptr;
+  return iter != offline_pages_.end() && !iter->second.IsMarkedForDeletion()
+             ? &(iter->second)
+             : nullptr;
 }
 
 const OfflinePageItem* OfflinePageModel::GetPageByOfflineURL(
@@ -291,8 +299,10 @@ const OfflinePageItem* OfflinePageModel::GetPageByOfflineURL(
   for (auto iter = offline_pages_.begin();
        iter != offline_pages_.end();
        ++iter) {
-    if (iter->second.GetOfflineURL() == offline_url)
+    if (iter->second.GetOfflineURL() == offline_url &&
+        !iter->second.IsMarkedForDeletion()) {
       return &(iter->second);
+    }
   }
   return nullptr;
 }
@@ -301,8 +311,9 @@ const OfflinePageItem* OfflinePageModel::GetPageByOnlineURL(
     const GURL& online_url) const {
   for (auto iter = offline_pages_.begin(); iter != offline_pages_.end();
        ++iter) {
-    if (iter->second.url == online_url)
+    if (iter->second.url == online_url && !iter->second.IsMarkedForDeletion()) {
       return &(iter->second);
+    }
   }
   return nullptr;
 }