Commit 1258642b authored by Shakti Sahu's avatar Shakti Sahu Committed by Commit Bot

Read Later : Added search capability

This CL adds search capability to read later bookmarks. This enables
read later items to be searchable from anywhere inside bookmarks.

Bug: 1139072
Change-Id: I13329f066d300be1e1a70ddd51f42908b8454749
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2536813
Commit-Queue: Shakti Sahu <shaktisahu@chromium.org>
Reviewed-by: default avatarBrandon Wylie <wylieb@chromium.org>
Reviewed-by: default avatarTheresa  <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#827935}
parent ba7a4ed8
...@@ -687,6 +687,7 @@ void BookmarkBridge::SearchBookmarks(JNIEnv* env, ...@@ -687,6 +687,7 @@ void BookmarkBridge::SearchBookmarks(JNIEnv* env,
GetBookmarksMatchingProperties(bookmark_model_, query, max_results, &results); GetBookmarksMatchingProperties(bookmark_model_, query, max_results, &results);
reading_list_manager_->GetMatchingNodes(query, max_results, &results);
if (partner_bookmarks_shim_->HasPartnerBookmarks() && if (partner_bookmarks_shim_->HasPartnerBookmarks() &&
IsReachable(partner_bookmarks_shim_->GetPartnerBookmarksRoot())) { IsReachable(partner_bookmarks_shim_->GetPartnerBookmarksRoot())) {
partner_bookmarks_shim_->GetPartnerBookmarksMatchingProperties( partner_bookmarks_shim_->GetPartnerBookmarksMatchingProperties(
...@@ -694,15 +695,10 @@ void BookmarkBridge::SearchBookmarks(JNIEnv* env, ...@@ -694,15 +695,10 @@ void BookmarkBridge::SearchBookmarks(JNIEnv* env,
} }
DCHECK((int)results.size() <= max_results); DCHECK((int)results.size() <= max_results);
for (const bookmarks::BookmarkNode* match : results) { for (const bookmarks::BookmarkNode* match : results) {
// If this bookmark is a partner bookmark if (!IsReachable(match))
if (partner_bookmarks_shim_->IsPartnerBookmark(match) && continue;
IsReachable(match)) { Java_BookmarkBridge_addToBookmarkIdList(env, j_list, match->id(),
Java_BookmarkBridge_addToBookmarkIdList( GetBookmarkType(match));
env, j_list, match->id(), BookmarkType::BOOKMARK_TYPE_PARTNER);
} else {
Java_BookmarkBridge_addToBookmarkIdList(
env, j_list, match->id(), BookmarkType::BOOKMARK_TYPE_NORMAL);
}
} }
} }
......
...@@ -4,6 +4,8 @@ ...@@ -4,6 +4,8 @@
#include "chrome/browser/reading_list/android/empty_reading_list_manager.h" #include "chrome/browser/reading_list/android/empty_reading_list_manager.h"
#include "components/bookmarks/browser/bookmark_utils.h"
EmptyReadingListManager::EmptyReadingListManager() = default; EmptyReadingListManager::EmptyReadingListManager() = default;
EmptyReadingListManager::~EmptyReadingListManager() = default; EmptyReadingListManager::~EmptyReadingListManager() = default;
...@@ -28,6 +30,11 @@ const bookmarks::BookmarkNode* EmptyReadingListManager::GetNodeByID( ...@@ -28,6 +30,11 @@ const bookmarks::BookmarkNode* EmptyReadingListManager::GetNodeByID(
return nullptr; return nullptr;
} }
void EmptyReadingListManager::GetMatchingNodes(
const bookmarks::QueryFields& query,
size_t max_count,
std::vector<const bookmarks::BookmarkNode*>* nodes) {}
bool EmptyReadingListManager::IsReadingListBookmark( bool EmptyReadingListManager::IsReadingListBookmark(
const bookmarks::BookmarkNode* node) const { const bookmarks::BookmarkNode* node) const {
return false; return false;
......
...@@ -22,6 +22,10 @@ class EmptyReadingListManager : public ReadingListManager { ...@@ -22,6 +22,10 @@ class EmptyReadingListManager : public ReadingListManager {
const std::string& title) override; const std::string& title) override;
const bookmarks::BookmarkNode* Get(const GURL& url) const override; const bookmarks::BookmarkNode* Get(const GURL& url) const override;
const bookmarks::BookmarkNode* GetNodeByID(int64_t id) const override; const bookmarks::BookmarkNode* GetNodeByID(int64_t id) const override;
void GetMatchingNodes(
const bookmarks::QueryFields& query,
size_t max_count,
std::vector<const bookmarks::BookmarkNode*>* nodes) override;
bool IsReadingListBookmark( bool IsReadingListBookmark(
const bookmarks::BookmarkNode* node) const override; const bookmarks::BookmarkNode* node) const override;
void Delete(const GURL& url) override; void Delete(const GURL& url) override;
......
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
namespace bookmarks { namespace bookmarks {
class BookmarkNode; class BookmarkNode;
struct QueryFields;
} // namespace bookmarks } // namespace bookmarks
class GURL; class GURL;
...@@ -59,6 +60,14 @@ class ReadingListManager : public KeyedService { ...@@ -59,6 +60,14 @@ class ReadingListManager : public KeyedService {
// root folder node. Returns nullptr if no match. // root folder node. Returns nullptr if no match.
virtual const bookmarks::BookmarkNode* GetNodeByID(int64_t id) const = 0; virtual const bookmarks::BookmarkNode* GetNodeByID(int64_t id) const = 0;
// Gets the bookmark nodes that match a search |query|. The resulting nodes
// are appended to the |results| up to a upper limit of |max_count|. No-op if
// |results| already have |max_count| nodes.
virtual void GetMatchingNodes(
const bookmarks::QueryFields& query,
size_t max_count,
std::vector<const bookmarks::BookmarkNode*>* results) = 0;
// Returns whether the bookmark node is maintained in reading list manager. // Returns whether the bookmark node is maintained in reading list manager.
// Will return true if |node| is the root for reading list nodes. // Will return true if |node| is the root for reading list nodes.
virtual bool IsReadingListBookmark( virtual bool IsReadingListBookmark(
......
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "chrome/grit/generated_resources.h" #include "chrome/grit/generated_resources.h"
#include "components/bookmarks/browser/bookmark_node.h" #include "components/bookmarks/browser/bookmark_node.h"
#include "components/bookmarks/browser/bookmark_utils.h"
#include "components/reading_list/core/reading_list_model.h" #include "components/reading_list/core/reading_list_model.h"
#include "ui/base/l10n/l10n_util.h" #include "ui/base/l10n/l10n_util.h"
#include "url/gurl.h" #include "url/gurl.h"
...@@ -158,6 +159,27 @@ const BookmarkNode* ReadingListManagerImpl::GetNodeByID(int64_t id) const { ...@@ -158,6 +159,27 @@ const BookmarkNode* ReadingListManagerImpl::GetNodeByID(int64_t id) const {
return nullptr; return nullptr;
} }
void ReadingListManagerImpl::GetMatchingNodes(
const bookmarks::QueryFields& query,
size_t max_count,
std::vector<const BookmarkNode*>* results) {
if (results->size() >= max_count)
return;
auto query_words = bookmarks::ParseBookmarkQuery(query);
if (query_words.empty())
return;
for (const auto& node : root_->children()) {
if (bookmarks::DoesBookmarkContainWords(node->GetTitle(), node->url(),
query_words)) {
results->push_back(node.get());
if (results->size() == max_count)
break;
}
}
}
bool ReadingListManagerImpl::IsReadingListBookmark( bool ReadingListManagerImpl::IsReadingListBookmark(
const BookmarkNode* node) const { const BookmarkNode* node) const {
if (!node) if (!node)
......
...@@ -47,6 +47,10 @@ class ReadingListManagerImpl : public ReadingListManager, ...@@ -47,6 +47,10 @@ class ReadingListManagerImpl : public ReadingListManager,
const std::string& title) override; const std::string& title) override;
const bookmarks::BookmarkNode* Get(const GURL& url) const override; const bookmarks::BookmarkNode* Get(const GURL& url) const override;
const bookmarks::BookmarkNode* GetNodeByID(int64_t id) const override; const bookmarks::BookmarkNode* GetNodeByID(int64_t id) const override;
void GetMatchingNodes(
const bookmarks::QueryFields& query,
size_t max_count,
std::vector<const bookmarks::BookmarkNode*>* results) override;
bool IsReadingListBookmark( bool IsReadingListBookmark(
const bookmarks::BookmarkNode* node) const override; const bookmarks::BookmarkNode* node) const override;
void Delete(const GURL& url) override; void Delete(const GURL& url) override;
......
...@@ -13,6 +13,7 @@ ...@@ -13,6 +13,7 @@
#include "base/test/simple_test_clock.h" #include "base/test/simple_test_clock.h"
#include "chrome/browser/reading_list/android/reading_list_manager.h" #include "chrome/browser/reading_list/android/reading_list_manager.h"
#include "components/bookmarks/browser/bookmark_node.h" #include "components/bookmarks/browser/bookmark_node.h"
#include "components/bookmarks/browser/bookmark_utils.h"
#include "components/reading_list/core/reading_list_model_impl.h" #include "components/reading_list/core/reading_list_model_impl.h"
#include "testing/gmock/include/gmock/gmock.h" #include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
...@@ -23,10 +24,11 @@ using ReadingListEntries = ReadingListModelImpl::ReadingListEntries; ...@@ -23,10 +24,11 @@ using ReadingListEntries = ReadingListModelImpl::ReadingListEntries;
namespace { namespace {
constexpr char kURL[] = "https://www.example.com"; constexpr char kURL[] = "https://www.example.com";
constexpr char kURL1[] = "https://www.anotherexample.com";
constexpr char kTitle[] = constexpr char kTitle[] =
"In earlier tellings, the dog had a better reputation than the cat, " "In earlier tellings, the dog had a better reputation than the cat, "
"however the president vetoed it."; "however the president vetoed it.";
constexpr char kTitle1[] = "boring title."; constexpr char kTitle1[] = "boring title about dogs.";
constexpr char kReadStatusKey[] = "read_status"; constexpr char kReadStatusKey[] = "read_status";
constexpr char kReadStatusRead[] = "true"; constexpr char kReadStatusRead[] = "true";
constexpr char kReadStatusUnread[] = "false"; constexpr char kReadStatusUnread[] = "false";
...@@ -165,6 +167,56 @@ TEST_F(ReadingListManagerImplTest, GetNodeByIDIsReadingListBookmark) { ...@@ -165,6 +167,56 @@ TEST_F(ReadingListManagerImplTest, GetNodeByIDIsReadingListBookmark) {
EXPECT_FALSE(manager()->IsReadingListBookmark(node_same_url.get())); EXPECT_FALSE(manager()->IsReadingListBookmark(node_same_url.get()));
} }
// Verifies GetMatchingNodes() API in reading list manager.
TEST_F(ReadingListManagerImplTest, GetMatchingNodes) {
manager()->Add(GURL(kURL), kTitle);
manager()->Add(GURL(kURL1), kTitle1);
EXPECT_EQ(2u, manager()->size());
// Search with a multi-word query text.
std::vector<const BookmarkNode*> results;
bookmarks::QueryFields query;
query.word_phrase_query.reset(
new base::string16(base::ASCIIToUTF16("dog cat")));
manager()->GetMatchingNodes(query, 5, &results);
EXPECT_EQ(1u, results.size());
// Search with a single word query text.
results.clear();
query.word_phrase_query.reset(new base::string16(base::ASCIIToUTF16("dog")));
manager()->GetMatchingNodes(query, 5, &results);
EXPECT_EQ(2u, results.size());
// Search with empty string. Shouldn't match anything.
results.clear();
query.word_phrase_query.reset(new base::string16());
manager()->GetMatchingNodes(query, 5, &results);
EXPECT_EQ(0u, results.size());
}
TEST_F(ReadingListManagerImplTest, GetMatchingNodesWithMaxCount) {
manager()->Add(GURL(kURL), kTitle);
manager()->Add(GURL(kURL1), kTitle1);
EXPECT_EQ(2u, manager()->size());
// Search with a query text.
std::vector<const BookmarkNode*> results;
bookmarks::QueryFields query;
query.word_phrase_query.reset(new base::string16(base::ASCIIToUTF16("dog")));
manager()->GetMatchingNodes(query, 5, &results);
EXPECT_EQ(2u, results.size());
// Search with having pre-existing elements in |results|.
manager()->GetMatchingNodes(query, 5, &results);
EXPECT_EQ(4u, results.size());
// Max count should never be exceeded.
manager()->GetMatchingNodes(query, 5, &results);
EXPECT_EQ(5u, results.size());
manager()->GetMatchingNodes(query, 5, &results);
EXPECT_EQ(5u, results.size());
}
// If Add() the same URL twice, the first bookmark node pointer will be // If Add() the same URL twice, the first bookmark node pointer will be
// invalidated. // invalidated.
TEST_F(ReadingListManagerImplTest, AddTwice) { TEST_F(ReadingListManagerImplTest, AddTwice) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment