Read later: IsReadListBookmark should check pointer instead of URL.

IsReadListBookmark only checks the URL, which creates a problem that
other bookmark source may have the same URL.

Instead, this should check the in memory tree.

Also print an error log for a legacy NOTREACHED() in bookmark_bridge.cc,
since reading list eventually may be able to delete but not editable.

Bug: 1139133
Change-Id: I58d2cbb6e2cd43c94670e2b707f5b4dd6fdd3128
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2476750Reviewed-by: Theresa  <twellington@chromium.org>
Reviewed-by: Brandon Wylie <wylieb@chromium.org>
Commit-Queue: Xing Liu <xingliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#817984}

chrome/browser/android/bookmarks/bookmark_bridge.cc

@@ -741,6 +741,7 @@ void BookmarkBridge::DeleteBookmark(
   // why this is called with an uneditable node.
   // See https://crbug.com/981172.
   if (!IsEditable(node)) {
+    LOG(ERROR) << "Deleting non editable bookmark, type:" << type;
     NOTREACHED();
     return;
   }

chrome/browser/reading_list/android/reading_list_manager_impl.cc

@@ -114,7 +114,13 @@ bool ReadingListManagerImpl::IsReadingListBookmark(
   if (root_.get() == node)
     return true;
 
-  return Get(node->url());
+  // Not recursive since there is only one level of children.
+  for (const auto& child : root_->children()) {
+    if (node == child.get())
+      return true;
+  }
+
+  return false;
 }
 
 void ReadingListManagerImpl::Delete(const GURL& url) {

chrome/browser/reading_list/android/reading_list_manager_impl_unittest.cc

@@ -8,6 +8,7 @@
 #include <string>
 #include <utility>
 
+#include "base/guid.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/simple_test_clock.h"
 #include "chrome/browser/reading_list/android/reading_list_manager.h"
@@ -141,6 +142,11 @@ TEST_F(ReadingListManagerImplTest, GetNodeByIDIsReadingListBookmark) {
   node = manager()->GetNodeByID(12345);
   EXPECT_FALSE(node);
   EXPECT_FALSE(manager()->IsReadingListBookmark(node));
+
+  // Node with the same URL but not in the tree.
+  auto node_same_url =
+      std::make_unique<BookmarkNode>(0, base::GenerateGUID(), url);
+  EXPECT_FALSE(manager()->IsReadingListBookmark(node_same_url.get()));
 }
 
 // Verifies Add() the same URL twice will not invalidate returned pointers, and