[Offline pages] Fixing a crash in URL change, when offline copy not present

Currently code dchecks on the existence of the offline page for a given
bookmark, which might not be the case.
Removing DCHECK and only continuing execution if the offline page exists.

BUG=560518
R=jianli@chromium.org

Review URL: https://codereview.chromium.org/1473043002

Cr-Commit-Position: refs/heads/master@{#361363}

components/offline_pages/offline_page_model.cc

@@ -492,8 +492,8 @@ void OfflinePageModel::BookmarkNodeChanged(
     const bookmarks::BookmarkNode* node) {
   // BookmarkNodeChanged could be triggered if title or URL gets changed. If
   // the latter, we need to invalidate the offline copy.
-  DCHECK(offline_pages_.count(node->id()) > 0);
-  if (offline_pages_[node->id()].url != node->url())
+  auto iter = offline_pages_.find(node->id());
+  if (iter != offline_pages_.end() && iter->second.url != node->url())
     DeletePageByBookmarkId(node->id(), DeletePageCallback());
 }
 

components/offline_pages/offline_page_model.h

@@ -208,6 +208,7 @@ class OfflinePageModel : public KeyedService,
 
  private:
   FRIEND_TEST_ALL_PREFIXES(OfflinePageModelTest, MarkPageForDeletion);
+  FRIEND_TEST_ALL_PREFIXES(OfflinePageModelTest, BookmarkNodeChangesUrl);
 
   typedef ScopedVector<OfflinePageArchiver> PendingArchivers;
 

components/offline_pages/offline_page_model_unittest.cc

@@ -15,6 +15,7 @@
 #include "base/strings/string16.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
+#include "components/bookmarks/browser/bookmark_node.h"
 #include "components/offline_pages/offline_page_item.h"
 #include "components/offline_pages/offline_page_metadata_store.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -985,4 +986,32 @@ TEST_F(OfflinePageModelTest, ClearAll) {
   EXPECT_EQ(1UL, GetStore()->offline_pages().size());
 }
 
+TEST_F(OfflinePageModelTest, BookmarkNodeChangesUrl) {
+  scoped_ptr<OfflinePageTestArchiver> archiver(
+      BuildArchiver(kTestUrl,
+                    OfflinePageArchiver::ArchiverResult::SUCCESSFULLY_CREATED)
+          .Pass());
+  model()->SavePage(
+      kTestUrl, kTestPageBookmarkId1, archiver.Pass(),
+      base::Bind(&OfflinePageModelTest::OnSavePageDone, AsWeakPtr()));
+  PumpLoop();
+
+  EXPECT_EQ(1UL, model()->GetAllPages().size());
+
+  bookmarks::BookmarkNode bookmark_node(kTestPageBookmarkId1, kTestUrl2);
+  model()->BookmarkNodeChanged(nullptr, &bookmark_node);
+  PumpLoop();
+
+  // Offline copy should be removed. Chrome should not crash.
+  // http://crbug.com/558929
+  EXPECT_EQ(0UL, model()->GetAllPages().size());
+
+  // Chrome should not crash when a bookmark with no offline copy is changed.
+  // http://crbug.com/560518
+  bookmark_node.set_url(kTestUrl);
+  model()->BookmarkNodeChanged(nullptr, &bookmark_node);
+  PumpLoop();
+  EXPECT_EQ(0UL, model()->GetAllPages().size());
+}
+
 }  // namespace offline_pages