Skip to content

Commit

Permalink
[AuxiliarySearch] Do not donate native page
Browse files Browse the repository at this point in the history 
AuxiliarySearch should not donate native pages for bookmarks and tabs.

b: 289988275
Change-Id: I623cd505e750441f298161189c08cc58b1a1326f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4780469
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Reviewed-by: Tomasz Wiszkowski <ender@google.com>
Commit-Queue: Gang Wu <gangwu@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1187541}
  • Loading branch information
Gang Wu authored and Chromium LUCI CQ committed Aug 23, 2023
1 parent e47e904 commit 489257c
Show file tree
Hide file tree
Showing 7 changed files with 213 additions and 43 deletions.
28 changes: 28 additions & 0 deletions .../android/java/src/org/chromium/chrome/browser/auxiliary_search/AuxiliarySearchBridge.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,33 @@ public AuxiliarySearchBridge(@NonNull Profile profile) {
}

/**
* This method is for filtering the tabs, which will only return the tabs which are http or
* https. This method is called when the AndroidAppIntegrationSafeSearch is not enabled.
*
* @param tabs A list of {@link Tab}s want to be check if there should be searched by Auxiliary
* Search.
* @return tabs which can be searched by Auxiliary Searchh.
*/
public @NonNull List<Tab> getSearchableTabs(@NonNull List<Tab> tabs) {
ArrayList<Tab> tabList = new ArrayList<>();
if (mNativeBridge != 0) {
Object[] tab_objects = AuxiliarySearchBridgeJni.get().getSearchableTabs(
mNativeBridge, tabs.toArray(new Tab[0]));

for (Object o : tab_objects) {
if (o instanceof Tab) {
tabList.add((Tab) o);
}
}
}

return tabList;
}

/**
* This method will return non sensitive url tabs, and the scheme is http or https.
* This method is called when the AndroidAppIntegrationSafeSearch not enabled.
*
* @param tabs A list of {@link Tab}s to check if they are sensitive.
* @param callback {@link Callback} to pass back the list of non sensitive {@link Tab}s.
*/
Expand Down Expand Up @@ -91,5 +118,6 @@ public interface Natives {
byte[] getBookmarksSearchableData(long nativeAuxiliarySearchProvider);
void getNonSensitiveTabs(
long nativeAuxiliarySearchProvider, Tab[] tabs, Callback<Object[]> callback);
Object[] getSearchableTabs(long nativeAuxiliarySearchProvider, Tab[] tabs);
}
}
10 changes: 8 additions & 2 deletions ...ndroid/java/src/org/chromium/chrome/browser/auxiliary_search/AuxiliarySearchProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,16 +47,22 @@ public AuxiliarySearchBookmarkGroup getBookmarksSearchableDataProto() {
* @return AuxiliarySearchGroup for {@link Tab}s.
*/
public AuxiliarySearchTabGroup getTabsSearchableDataProto() {
var tabGroupBuilder = AuxiliarySearchTabGroup.newBuilder();
TabList tabList = mTabModelSelector.getModel(false).getComprehensiveModel();

// Find the the bottom of tabs in the tab switcher view if the number of the tabs more than
// 'kNumTabsToSend'. In the multiwindow mode, the order of the 'tabList' is one window's
// tabs, and then another's.
int firstTabIndex = Math.max(tabList.getCount() - kNumTabsToSend, 0);
int end = tabList.getCount() - 1;
List<Tab> listTab = new ArrayList<>();
for (int i = firstTabIndex; i <= end; i++) {
Tab tab = tabList.getTabAt(i);
listTab.add(tabList.getTabAt(i));
}

// Send tabs to native to filter the tabs.
List<Tab> filteredTabs = mAuxiliarySearchBridge.getSearchableTabs(listTab);
var tabGroupBuilder = AuxiliarySearchTabGroup.newBuilder();
for (Tab tab : filteredTabs) {
AuxiliarySearchEntry entry = tabToAuxiliarySearchEntry(tab);
if (entry != null) {
tabGroupBuilder.addTab(entry);
Expand Down
5 changes: 5 additions & 0 deletions ...id/java/src/org/chromium/chrome/browser/auxiliary_search/AuxiliarySearchProviderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.mockito.Mock;
import org.mockito.junit.MockitoJUnit;
import org.mockito.junit.MockitoRule;
import org.mockito.stubbing.Answer;
import org.robolectric.annotation.Config;

import org.chromium.base.Callback;
Expand Down Expand Up @@ -58,6 +59,7 @@ public class AuxiliarySearchProviderTest {
private static final String TAB_URL = "https://tab.google.com/";
private static final String BOOKMARK_TITLE = "bookmark";
private static final String BOOKMARK_URL = "https://bookmark.google.com";
private static final String NEW_TAB_PAGE_URL = "chrome-native://newtab";
private static final long FAKE_NATIVE_PROVIDER = 1;

public @Rule JniMocker mJniMocker = new JniMocker();
Expand All @@ -74,6 +76,9 @@ public class AuxiliarySearchProviderTest {
public void setUp() {
mJniMocker.mock(AuxiliarySearchBridgeJni.TEST_HOOKS, mMockAuxiliarySearchBridgeJni);
doReturn(FAKE_NATIVE_PROVIDER).when(mMockAuxiliarySearchBridgeJni).getForProfile(mProfile);
doAnswer((Answer<Object[]>) invocation -> ((Object[]) invocation.getArguments()[1]))
.when(mMockAuxiliarySearchBridgeJni)
.getSearchableTabs(eq(FAKE_NATIVE_PROVIDER), any(Tab[].class));
mAuxiliarySearchProvider = new AuxiliarySearchProvider(mProfile, mTabModelSelector);
}

Expand Down
68 changes: 54 additions & 14 deletions chrome/browser/android/auxiliary_search/auxiliary_search_provider.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "components/bookmarks/browser/bookmark_model.h"
#include "components/bookmarks/browser/bookmark_node.h"
#include "components/bookmarks/browser/bookmark_utils.h"
#include "url/url_constants.h"

using base::android::ToJavaByteArray;
using bookmarks::BookmarkModel;
Expand Down Expand Up @@ -76,7 +77,11 @@ void callJavaCallbackWithTabList(
j_callback_obj, base::android::ToJavaArrayOfObjects(env, j_tabs_list));
}

void FilterNonSensitiveTabs(
bool IsSchemeAllowed(const GURL& url) {
return url.SchemeIs(url::kHttpScheme) || url.SchemeIs(url::kHttpsScheme);
}

void FilterNonSensitiveSearchableTabs(
std::unique_ptr<std::vector<TabAndroid*>> all_tabs,
int current_tab_index,
std::unique_ptr<std::vector<TabAndroid*>> non_sensitive_tabs,
Expand All @@ -98,9 +103,10 @@ void FilterNonSensitiveTabs(

TabAndroid* next_tab = all_tabs->at(next_tab_index);
SensitivityPersistedTabDataAndroid::From(
next_tab, base::BindOnce(&FilterNonSensitiveTabs, std::move(all_tabs),
next_tab_index, std::move(non_sensitive_tabs),
std::move(callback)));
next_tab,
base::BindOnce(&FilterNonSensitiveSearchableTabs, std::move(all_tabs),
next_tab_index, std::move(non_sensitive_tabs),
std::move(callback)));
}

} // namespace
Expand All @@ -123,6 +129,22 @@ AuxiliarySearchProvider::GetBookmarksSearchableData(JNIEnv* env) const {
return ToJavaByteArray(env, serialized_group);
}

base::android::ScopedJavaLocalRef<jobjectArray>
AuxiliarySearchProvider::GetSearchableTabs(
JNIEnv* env,
const base::android::JavaParamRef<jobjectArray>& j_tabs_android) const {
std::vector<TabAndroid*> all_tabs = TabAndroid::GetAllNativeTabs(
env, base::android::ScopedJavaLocalRef(j_tabs_android));
std::vector<TabAndroid*> filtered_tabs = FilterTabsByScheme(all_tabs);

std::vector<base::android::ScopedJavaLocalRef<jobject>> j_filtered_tabs;
j_filtered_tabs.reserve(filtered_tabs.size());
for (TabAndroid* tab : filtered_tabs) {
j_filtered_tabs.push_back(tab->GetJavaObject());
}
return base::android::ToJavaArrayOfObjects(env, j_filtered_tabs);
}

void AuxiliarySearchProvider::GetNonSensitiveTabs(
JNIEnv* env,
const base::android::JavaParamRef<jobjectArray>& j_tabs_android,
Expand All @@ -131,10 +153,9 @@ void AuxiliarySearchProvider::GetNonSensitiveTabs(
env, base::android::ScopedJavaLocalRef(j_tabs_android));

GetNonSensitiveTabsInternal(
std::make_unique<std::vector<TabAndroid*>>(all_tabs),
base::BindOnce(
&callJavaCallbackWithTabList, env,
base::android::ScopedJavaGlobalRef<jobject>(j_callback_obj)));
all_tabs, base::BindOnce(&callJavaCallbackWithTabList, env,
base::android::ScopedJavaGlobalRef<jobject>(
j_callback_obj)));
}

auxiliary_search::AuxiliarySearchBookmarkGroup
Expand All @@ -143,9 +164,14 @@ AuxiliarySearchProvider::GetBookmarks(bookmarks::BookmarkModel* model) const {
std::vector<const BookmarkNode*> nodes;
bookmarks::GetMostRecentlyUsedEntries(model, kMaxBookmarksCount, &nodes);
for (const BookmarkNode* node : nodes) {
GURL url = node->url();
if (!IsSchemeAllowed(url)) {
continue;
}

auxiliary_search::AuxiliarySearchEntry* bookmark = group.add_bookmark();
bookmark->set_title(base::UTF16ToUTF8(node->GetTitle()));
bookmark->set_url(node->url().spec());
bookmark->set_url(url.spec());
if (!node->date_added().is_null()) {
bookmark->set_creation_timestamp(node->date_added().ToJavaTime());
}
Expand All @@ -157,21 +183,35 @@ AuxiliarySearchProvider::GetBookmarks(bookmarks::BookmarkModel* model) const {
return group;
}

// static
std::vector<TabAndroid*> AuxiliarySearchProvider::FilterTabsByScheme(
const std::vector<TabAndroid*>& tabs) {
std::vector<TabAndroid*> filtered_tabs;
for (TabAndroid* tab : tabs) {
if (IsSchemeAllowed(tab->GetURL())) {
filtered_tabs.push_back(tab);
}
}
return filtered_tabs;
}

void AuxiliarySearchProvider::GetNonSensitiveTabsInternal(
std::unique_ptr<std::vector<TabAndroid*>> all_tabs,
const std::vector<TabAndroid*>& all_tabs,
NonSensitiveTabsCallback callback) const {
std::unique_ptr<std::vector<TabAndroid*>> non_sensitive_tabs =
std::make_unique<std::vector<TabAndroid*>>();
if (all_tabs->size() == 0) {
std::vector<TabAndroid*> filtered_tabs = FilterTabsByScheme(all_tabs);
if (filtered_tabs.size() == 0) {
std::move(callback).Run(std::move(non_sensitive_tabs));
return;
}

TabAndroid* next_tab = all_tabs->at(all_tabs->size() - 1);
TabAndroid* next_tab = filtered_tabs.at(filtered_tabs.size() - 1);
SensitivityPersistedTabDataAndroid::From(
next_tab,
base::BindOnce(&FilterNonSensitiveTabs, std::move(all_tabs),
all_tabs->size() - 1, std::move(non_sensitive_tabs),
base::BindOnce(&FilterNonSensitiveSearchableTabs,
std::make_unique<std::vector<TabAndroid*>>(filtered_tabs),
filtered_tabs.size() - 1, std::move(non_sensitive_tabs),
std::move(callback)));
}

Expand Down
16 changes: 13 additions & 3 deletions chrome/browser/android/auxiliary_search/auxiliary_search_provider.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ class AuxiliarySearchProvider : public KeyedService {
base::android::ScopedJavaLocalRef<jbyteArray> GetBookmarksSearchableData(
JNIEnv* env) const;

base::android::ScopedJavaLocalRef<jobjectArray> GetSearchableTabs(
JNIEnv* env,
const base::android::JavaParamRef<jobjectArray>& j_tabs_android) const;

void GetNonSensitiveTabs(
JNIEnv* env,
const base::android::JavaParamRef<jobjectArray>& j_tabs_android,
Expand All @@ -43,22 +47,28 @@ class AuxiliarySearchProvider : public KeyedService {

private:
FRIEND_TEST_ALL_PREFIXES(AuxiliarySearchProviderTest, QueryBookmarks);
FRIEND_TEST_ALL_PREFIXES(AuxiliarySearchProviderTest,
QueryBookmarks_nativePageShouldBeFiltered);
FRIEND_TEST_ALL_PREFIXES(AuxiliarySearchProviderBrowserTest,
QuerySensitiveTab);
FRIEND_TEST_ALL_PREFIXES(AuxiliarySearchProviderBrowserTest,
QueryNonSensitiveTab);
FRIEND_TEST_ALL_PREFIXES(AuxiliarySearchProviderBrowserTest,
QueryEmptyTabList);
FRIEND_TEST_ALL_PREFIXES(AuxiliarySearchProviderBrowserTest, NativeTabTest);
FRIEND_TEST_ALL_PREFIXES(AuxiliarySearchProviderBrowserTest, FilterTabsTest);

using NonSensitiveTabsCallback =
base::OnceCallback<void(std::unique_ptr<std::vector<TabAndroid*>>)>;

auxiliary_search::AuxiliarySearchBookmarkGroup GetBookmarks(
bookmarks::BookmarkModel* model) const;

void GetNonSensitiveTabsInternal(
std::unique_ptr<std::vector<TabAndroid*>> all_tabs,
NonSensitiveTabsCallback callback) const;
static std::vector<TabAndroid*> FilterTabsByScheme(
const std::vector<TabAndroid*>& tabs);

void GetNonSensitiveTabsInternal(const std::vector<TabAndroid*>& all_tabs,
NonSensitiveTabsCallback callback) const;

raw_ptr<Profile> profile_;

Expand Down

0 comments on commit 489257c

Please sign in to comment.