Reland "[Android] Create host-specific history query"

This is a reland of I7a588400bec9c4b3aeddffd203e6b3349191c6f6 Fixed the test by adding the correct time to the |TestEntry|s. > [Android] Create host-specific history query > > Add backend function to query history urls and only returning matching > hosts. > > Screenshot: https://crbug.com/1173154#c17 > Bug: 1173154 > Change-Id: I7a588400bec9c4b3aeddffd203e6b3349191c6f6 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3001890 > Commit-Queue: Ehimare Okoyomon <eokoyomon@chromium.org> > Reviewed-by: Scott Violet <sky@chromium.org> > Reviewed-by: Christian Dullweber <dullweber@chromium.org> > Cr-Commit-Position: refs/heads/master@{#901635} Bug: 1173154, 1229461 Change-Id: I6dcda64b290ca91c684ab60e8684b69fe3e412de Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3033421 Commit-Queue: Ehimare Okoyomon <eokoyomon@chromium.org> Reviewed-by: Scott Violet <sky@chromium.org> Cr-Commit-Position: refs/heads/master@{#902328}
marcosholgado · Jul 16, 2021 · 4bd2758 · 4bd2758
1 parent 5a8e432
commit 4bd2758
Show file tree

Hide file tree

Showing 16 changed files with 235 additions and 66 deletions.
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/history/BrowsingHistoryBridge.java b/chrome/android/java/src/org/chromium/chrome/browser/history/BrowsingHistoryBridge.java
@@ -41,7 +41,13 @@ public void destroy() {
     @Override
     public void queryHistory(String query) {
         BrowsingHistoryBridgeJni.get().queryHistory(mNativeHistoryBridge,
-                BrowsingHistoryBridge.this, new ArrayList<HistoryItem>(), query);
+                BrowsingHistoryBridge.this, new ArrayList<HistoryItem>(), query, false);
+    }
+
+    @Override
+    public void queryHistoryForHost(String hostName) {
+        BrowsingHistoryBridgeJni.get().queryHistory(mNativeHistoryBridge,
+                BrowsingHistoryBridge.this, new ArrayList<HistoryItem>(), hostName, true);
     }
 
     @Override
@@ -113,7 +119,7 @@ interface Natives {
         long init(BrowsingHistoryBridge caller, Profile profile);
         void destroy(long nativeBrowsingHistoryBridge, BrowsingHistoryBridge caller);
         void queryHistory(long nativeBrowsingHistoryBridge, BrowsingHistoryBridge caller,
-                List<HistoryItem> historyItems, String query);
+                List<HistoryItem> historyItems, String query, boolean hostOnly);
         void queryHistoryContinuation(long nativeBrowsingHistoryBridge,
                 BrowsingHistoryBridge caller, List<HistoryItem> historyItems);
         void markItemForRemoval(long nativeBrowsingHistoryBridge, BrowsingHistoryBridge caller,

diff --git a/chrome/android/java/src/org/chromium/chrome/browser/history/HistoryAdapter.java b/chrome/android/java/src/org/chromium/chrome/browser/history/HistoryAdapter.java
@@ -66,6 +66,7 @@ public class HistoryAdapter extends DateDividedAdapter implements BrowsingHistor
     private boolean mPrivacyDisclaimersVisible;
     private boolean mClearBrowsingDataButtonVisible;
     private String mQueryText = EMPTY_QUERY;
+    private String mHostName;
 
     private boolean mDisableScrollToLoadForTest;
 
@@ -98,7 +99,11 @@ public void initialize() {
         mAreHeadersInitialized = false;
         mIsLoadingItems = true;
         mClearOnNextQueryComplete = true;
-        mHistoryProvider.queryHistory(mQueryText);
+        if (mHostName != null) {
+            mHistoryProvider.queryHistoryForHost(mHostName);
+        } else {
+            mHistoryProvider.queryHistory(mQueryText);
+        }
     }
 
     @Override
@@ -411,12 +416,9 @@ private void updateClearBrowsingDataButtonVisibility() {
         if (mAreHeadersInitialized) setHeaders();
     }
 
-    /**
-     * Sets the query text.
-     * @param query The text to be searched for.
-     */
-    void setQueryText(String query) {
-        mQueryText = query;
+    /** @param hostName The hostName to retrieve history entries for. */
+    public void setHostName(String hostName) {
+        mHostName = hostName;
     }
 
     @VisibleForTesting

diff --git a/chrome/android/java/src/org/chromium/chrome/browser/history/HistoryContentManager.java b/chrome/android/java/src/org/chromium/chrome/browser/history/HistoryContentManager.java
@@ -225,12 +225,8 @@ public void onScrolled(RecyclerView recyclerView, int dx, int dy) {
 
     /** Initialize the HistoryContentManager to start loading items. */
     public void initialize() {
-        if (mHostName != null) {
-            // Filtering the adapter to only the results from this particular host.
-            // TODO(crbug.com/1173154): Add robust filtering for just items with a matching host in
-            // backend. QueryText could return entries that contain the string anywhere.
-            mHistoryAdapter.setQueryText(mHostName);
-        }
+        // Filtering the adapter to only the results from this particular host.
+        mHistoryAdapter.setHostName(mHostName);
         mHistoryAdapter.initialize();
     }
 

diff --git a/chrome/android/java/src/org/chromium/chrome/browser/history/HistoryProvider.java b/chrome/android/java/src/org/chromium/chrome/browser/history/HistoryProvider.java
@@ -52,6 +52,13 @@ void onQueryHistoryComplete(List<HistoryItem> items,
      */
     void queryHistory(String query);
 
+    /**
+     * Query browsing history for a particular host. Only one query may be in-flight at any time.
+     * See BrowsingHistoryService::QueryHistory.
+     * @param hostName The host name.
+     */
+    void queryHistoryForHost(String hostName);
+
     /*
      * Fetches more results using the previous query's text, only valid to call
      * after queryHistory is called.

diff --git a/chrome/browser/android/history/browsing_history_bridge.cc b/chrome/browser/android/history/browsing_history_bridge.cc
@@ -52,13 +52,15 @@ void BrowsingHistoryBridge::QueryHistory(
     JNIEnv* env,
     const JavaParamRef<jobject>& obj,
     const JavaParamRef<jobject>& j_result_obj,
-    jstring j_query) {
+    jstring j_query,
+    jboolean j_host_only) {
   j_query_result_obj_.Reset(env, j_result_obj);
   query_history_continuation_.Reset();
 
   history::QueryOptions options;
   options.max_count = kMaxQueryCount;
   options.duplicate_policy = history::QueryOptions::REMOVE_DUPLICATES_PER_DAY;
+  options.host_only = j_host_only;
 
   browsing_history_service_->QueryHistory(
       base::android::ConvertJavaStringToUTF16(env, j_query), options);

diff --git a/chrome/browser/android/history/browsing_history_bridge.h b/chrome/browser/android/history/browsing_history_bridge.h
@@ -28,7 +28,8 @@ class BrowsingHistoryBridge : public ProfileBasedBrowsingHistoryDriver {
   void QueryHistory(JNIEnv* env,
                     const JavaParamRef<jobject>& obj,
                     const JavaParamRef<jobject>& j_result_obj,
-                    jstring j_query);
+                    jstring j_query,
+                    jboolean j_host_only);
 
   void QueryHistoryContinuation(JNIEnv* env,
                                 const JavaParamRef<jobject>& obj,

diff --git a/...est/android/javatests/src/org/chromium/chrome/browser/history/StubbedHistoryProvider.java b/...est/android/javatests/src/org/chromium/chrome/browser/history/StubbedHistoryProvider.java
@@ -29,6 +29,7 @@ public class StubbedHistoryProvider implements HistoryProvider {
     private int mLastQueryEndPosition;
     private String mLastQuery;
     private int mPaging = 5;
+    private boolean mHostOnly;
 
     @Override
     public void setObserver(BrowsingHistoryObserver observer) {
@@ -37,6 +38,17 @@ public void setObserver(BrowsingHistoryObserver observer) {
 
     @Override
     public void queryHistory(String query) {
+        mHostOnly = false;
+        query(query);
+    }
+
+    @Override
+    public void queryHistoryForHost(String hostName) {
+        mHostOnly = true;
+        query(hostName);
+    }
+
+    private void query(String query) {
         mLastQueryEndPosition = 0;
         mLastQuery = query;
         queryHistoryContinuation();
@@ -50,10 +62,21 @@ public void queryHistoryContinuation() {
         if (!isSearch) {
             mSearchItems.clear();
         } else if (mLastQueryEndPosition == 0) {
+            mSearchItems.clear();
             // Start a new search; simulate basic search.
             mLastQuery = mLastQuery.toLowerCase(Locale.getDefault());
             for (HistoryItem item : mItems) {
-                if (item.getUrl().getSpec().toLowerCase(Locale.getDefault()).contains(mLastQuery)
+                if (mHostOnly) {
+                    if (item.getUrl()
+                                    .getHost()
+                                    .toLowerCase(Locale.getDefault())
+                                    .equals(mLastQuery)) {
+                        mSearchItems.add(item);
+                    }
+                } else if (item.getUrl()
+                                   .getSpec()
+                                   .toLowerCase(Locale.getDefault())
+                                   .contains(mLastQuery)
                         || item.getTitle().toLowerCase(Locale.getDefault()).contains(mLastQuery)) {
                     mSearchItems.add(item);
                 }

diff --git a/components/history/core/browser/browsing_history_service.cc b/components/history/core/browser/browsing_history_service.cc
@@ -648,6 +648,7 @@ void BrowsingHistoryService::WebHistoryQueryComplete(
     if (const base::Value* events = results_value->FindListKey("event")) {
       state->remote_results.reserve(state->remote_results.size() +
                                     events->GetList().size());
+      std::string host_name_utf8 = base::UTF16ToUTF8(state->search_text);
       for (const base::Value& event : events->GetList()) {
         if (!event.is_dict())
           continue;
@@ -664,8 +665,15 @@ void BrowsingHistoryService::WebHistoryQueryComplete(
         if (!ids || ids->GetList().empty())
           continue;
 
-        // Ignore any URLs that should not be shown in the history page.
         GURL gurl(*url);
+        if (state->original_options.host_only) {
+          // Do post filter to skip entries that do not have the correct
+          // hostname.
+          if (gurl.host() != host_name_utf8)
+            continue;
+        }
+
+        // Ignore any URLs that should not be shown in the history page.
         if (driver_->ShouldHideWebHistoryUrl(gurl))
           continue;
 

diff --git a/components/history/core/browser/browsing_history_service_unittest.cc b/components/history/core/browser/browsing_history_service_unittest.cc
@@ -41,6 +41,9 @@ const char kUrl4[] = "http://www.four.com";
 const char kUrl5[] = "http://www.five.com";
 const char kUrl6[] = "http://www.six.com";
 const char kUrl7[] = "http://www.seven.com";
+const char kUrl8[] = "http://eight.com";
+const char kUrl9[] = "http://nine.com/eight.com";
+const char kUrl10[] = "http://ten.com/eight";
 const char kIconUrl1[] = "http://www.one.com/favicon.ico";
 
 const HistoryEntry::EntryType kLocal = HistoryEntry::LOCAL_ENTRY;
@@ -253,8 +256,14 @@ class BrowsingHistoryServiceTest : public ::testing::Test {
 
   TestBrowsingHistoryDriver::QueryResult QueryHistory(
       const QueryOptions& options) {
+    return QueryHistory(std::u16string(), options);
+  }
+
+  TestBrowsingHistoryDriver::QueryResult QueryHistory(
+      const std::u16string& query_text,
+      const QueryOptions& options) {
     size_t previous_results_count = driver()->GetQueryResults().size();
-    service()->QueryHistory(std::u16string(), options);
+    service()->QueryHistory(query_text, options);
     BlockUntilHistoryProcessesPendingRequests();
     const std::vector<TestBrowsingHistoryDriver::QueryResult> all_results =
         driver()->GetQueryResults();
@@ -405,6 +414,23 @@ TEST_F(BrowsingHistoryServiceTest, QueryHistoryRemoteTimeRanges) {
       {{kUrl3, 3, kRemote}, {kUrl2, 2, kRemote}}, QueryHistory(options));
 }
 
+TEST_F(BrowsingHistoryServiceTest, QueryHistoryHostOnlyRemote) {
+  AddHistory({{kUrl8, 1, kRemote}, {kUrl9, 2, kRemote}, {kUrl10, 3, kRemote}});
+
+  QueryOptions options;
+  options.max_count = 0;
+  options.host_only = false;
+  VerifyQueryResult(
+      /*reached_beginning*/ true,
+      /*has_synced_results*/ true,
+      {{kUrl10, 3, kRemote}, {kUrl9, 2, kRemote}, {kUrl8, 1, kRemote}},
+      QueryHistory(u"eight.com", options));
+  options.host_only = true;
+  VerifyQueryResult(/*reached_beginning*/ true,
+                    /*has_synced_results*/ true, {{kUrl8, 1, kRemote}},
+                    QueryHistory(u"eight.com", options));
+}
+
 TEST_F(BrowsingHistoryServiceTest, QueryHistoryLocalPagingPartial) {
   AddHistory({{kUrl1, 1, kLocal}, {kUrl2, 2, kLocal}, {kUrl3, 3, kLocal}});
   VerifyQueryResult(/*reached_beginning*/ false,

diff --git a/components/history/core/browser/history_backend.cc b/components/history/core/browser/history_backend.cc
@@ -1704,9 +1704,12 @@ void HistoryBackend::QueryHistoryBasic(const QueryOptions& options,
 void HistoryBackend::QueryHistoryText(const std::u16string& text_query,
                                       const QueryOptions& options,
                                       QueryResults* result) {
-  URLRows text_matches;
-  db_->GetTextMatchesWithAlgorithm(text_query, options.matching_algorithm,
-                                   &text_matches);
+  URLRows text_matches =
+      options.host_only
+          ? GetMatchesForHost(text_query)
+          : db_->GetTextMatchesWithAlgorithm(
+                text_query, options.matching_algorithm.value_or(
+                                query_parser::MatchingAlgorithm::DEFAULT));
 
   std::vector<URLResult> matching_visits;
   VisitVector visits;  // Declare outside loop to prevent re-construction.
@@ -1742,6 +1745,23 @@ void HistoryBackend::QueryHistoryText(const std::u16string& text_query,
     result->set_reached_beginning(true);
 }
 
+URLRows HistoryBackend::GetMatchesForHost(const std::u16string& host_name) {
+  URLRows results;
+  URLDatabase::URLEnumerator iter;
+
+  if (db_ && db_->InitURLEnumeratorForEverything(&iter)) {
+    URLRow row;
+    std::string host_name_utf8 = base::UTF16ToUTF8(host_name);
+    while (iter.GetNextURL(&row)) {
+      if (row.url().is_valid() && row.url().host() == host_name_utf8) {
+        results.push_back(std::move(row));
+      }
+    }
+  }
+
+  return results;
+}
+
 RedirectList HistoryBackend::QueryRedirectsFrom(const GURL& from_url) {
   if (!db_)
     return {};

diff --git a/components/history/core/browser/history_backend.h b/components/history/core/browser/history_backend.h
@@ -647,6 +647,10 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>,
                         const QueryOptions& options,
                         QueryResults* result);
 
+  // Performs a brute force search over the database to find any host names that
+  // match the `host_name` string. Returns any matches.
+  URLRows GetMatchesForHost(const std::u16string& host_name);
+
   // Clusters ------------------------------------------------------------------
 
   // Convert `AnnotatedVisitRow`s to `AnnotatedVisit`s. Drops rows without