Commit f20b780c authored by Mathias Carlen's avatar Mathias Carlen Committed by Commit Bot

[AA Direct Actions] Static action for FindInPage.

This patch adds a static direct action for "find in page" to the
set of already existing static actions (e.g. new tab, go back, etc). In
order to be consistent the FindInPageManager is used to show the UI and
the TextField for the search term is then populated with the search
query passed in by an assist app using Android's direct action API.

There is no voice support planned for moving to the next/previous match
or close the "find in page" UI again.

Bug: b/145106813
Change-Id: Ieaee42f039cc77b7f03bdbba0b110aaa0887e039
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1944396Reviewed-by: default avatarStephane Zermatten <szermatt@chromium.org>
Reviewed-by: default avatarTed Choc <tedchoc@chromium.org>
Commit-Queue: Mathias Carlen <mcarlen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#723313}
parent e8767efe
......@@ -471,6 +471,7 @@ chrome_java_sources = [
"java/src/org/chromium/chrome/browser/directactions/DirectActionInitializer.java",
"java/src/org/chromium/chrome/browser/directactions/DirectActionReporter.java",
"java/src/org/chromium/chrome/browser/directactions/DirectActionUsageHistogram.java",
"java/src/org/chromium/chrome/browser/directactions/FindInPageDirectActionHandler.java",
"java/src/org/chromium/chrome/browser/directactions/GoBackDirectActionHandler.java",
"java/src/org/chromium/chrome/browser/directactions/CloseTabDirectActionHandler.java",
"java/src/org/chromium/chrome/browser/directactions/SimpleDirectActionHandler.java",
......
......@@ -55,6 +55,7 @@ chrome_junit_test_java_sources = [
"junit/src/org/chromium/chrome/browser/customtabs/content/CustomTabActivityUrlLoadingTest.java",
"junit/src/org/chromium/chrome/browser/customtabs/shadows/ShadowExternalNavigationDelegateImpl.java",
"junit/src/org/chromium/chrome/browser/directactions/GoBackDirectActionHandlerTest.java",
"junit/src/org/chromium/chrome/browser/directactions/FindInPageDirectActionHandlerTest.java",
"junit/src/org/chromium/chrome/browser/display_cutout/DisplayCutoutControllerTest.java",
"junit/src/org/chromium/chrome/browser/download/DownloadResumptionSchedulerTest.java",
"junit/src/org/chromium/chrome/browser/download/DownloadSharedPreferenceEntryTest.java",
......
......@@ -360,7 +360,12 @@ public class AutofillAssistantDirectActionHandlerTest {
public void report() {}
}
/** A simple action definition for testing. */
/**
* A simple action definition for testing.
*
* TODO(crbug.com/806868): Share these fakes. There is another one in
* chrome/android/junit/...directactions/
*/
private static class FakeDirectActionDefinition implements Definition {
final String mId;
List<FakeParameter> mParameters = new ArrayList<>();
......
......@@ -1385,12 +1385,15 @@ public abstract class ChromeActivity<C extends ChromeActivityComponent>
*
* <p>Subclasses should override this method and either extend the set of direct actions or
* suppress registration of default direct actions.
*
* TODO(crrev.com/959841): Move the DirectActionInitializer in the UiRootCoordinator.
*/
protected void registerDirectActions() {
mDirectActionInitializer.registerCommonChromeActions(this, getActivityType(), this,
this::onBackPressed, mTabModelSelector,
AutofillAssistantFacade.areDirectActionsAvailable(getActivityType()) ?
getBottomSheetController() : null,
this::onBackPressed, mTabModelSelector, mRootUiCoordinator.getFindToolbarManager(),
AutofillAssistantFacade.areDirectActionsAvailable(getActivityType())
? getBottomSheetController()
: null,
mScrimView);
}
......
......@@ -46,6 +46,9 @@ public class ChromeDirectActionIds {
// Close all tabs
public static final String CLOSE_ALL_TABS = "close_all_tabs";
// Find in page.
public static final String FIND_IN_PAGE = "find_in_page";
// If you add a new action to this list, consider extending ChromeDirectActionUsageHistogram to
// track usage of the new action.
......
......@@ -14,6 +14,7 @@ import org.chromium.chrome.R;
import org.chromium.chrome.browser.AppHooks;
import org.chromium.chrome.browser.MenuOrKeyboardActionController;
import org.chromium.chrome.browser.autofill_assistant.AutofillAssistantFacade;
import org.chromium.chrome.browser.findinpage.FindToolbarManager;
import org.chromium.chrome.browser.flags.ActivityType;
import org.chromium.chrome.browser.tabmodel.TabModelSelector;
import org.chromium.chrome.browser.widget.ScrimView;
......@@ -66,14 +67,17 @@ public class DirectActionInitializer {
* @param goBackAction Implementation of the "go_back" action, usually {@link
* android.app.Activity#onBackPressed}.
* @param tabModelSelector The activity's {@link TabModelSelector}
* @param findToolbarManager Manager to use for the "find_in_page" action, if it exists
* @param bottomSheetController Controller for the activity's bottom sheet, if it exists
* @param scrim The activity's scrim view, if it exists
*/
public void registerCommonChromeActions(Context context, @ActivityType int activityType,
MenuOrKeyboardActionController actionController, Runnable goBackAction,
TabModelSelector tabModelSelector,
TabModelSelector tabModelSelector, @Nullable FindToolbarManager findToolbarManager,
@Nullable BottomSheetController bottomSheetController, ScrimView scrim) {
mCoordinator.register(new GoBackDirectActionHandler(goBackAction));
mCoordinator.register(
new FindInPageDirectActionHandler(tabModelSelector, findToolbarManager));
registerMenuHandlerIfNecessary(actionController, tabModelSelector)
.whitelistActions(R.id.forward_menu_id, R.id.reload_menu_id);
......
......@@ -39,6 +39,7 @@ class DirectActionUsageHistogram {
map.put(ChromeDirectActionIds.NEW_TAB, DirectActionId.NEW_TAB);
map.put(ChromeDirectActionIds.CLOSE_TAB, DirectActionId.CLOSE_TAB);
map.put(ChromeDirectActionIds.CLOSE_ALL_TABS, DirectActionId.CLOSE_ALL_TABS);
map.put(ChromeDirectActionIds.FIND_IN_PAGE, DirectActionId.FIND_IN_PAGE);
ACTION_ID_MAP = Collections.unmodifiableMap(map);
}
......@@ -50,7 +51,8 @@ class DirectActionUsageHistogram {
@IntDef({DirectActionId.GO_BACK, DirectActionId.RELOAD, DirectActionId.GO_FORWARD,
DirectActionId.BOOKMARK_THIS_PAGE, DirectActionId.DOWNLOADS, DirectActionId.PREFERENCES,
DirectActionId.OPEN_HISTORY, DirectActionId.HELP, DirectActionId.NEW_TAB,
DirectActionId.CLOSE_TAB, DirectActionId.CLOSE_ALL_TABS, DirectActionId.NUM_ENTRIES})
DirectActionId.CLOSE_TAB, DirectActionId.CLOSE_ALL_TABS, DirectActionId.FIND_IN_PAGE,
DirectActionId.NUM_ENTRIES})
@interface DirectActionId {
/** No action was executed. */
int UNKNOWN = 0;
......@@ -70,8 +72,9 @@ class DirectActionUsageHistogram {
int NEW_TAB = 10;
int CLOSE_TAB = 11;
int CLOSE_ALL_TABS = 12;
int FIND_IN_PAGE = 13;
int NUM_ENTRIES = 13;
int NUM_ENTRIES = 14;
}
/**
......
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
package org.chromium.chrome.browser.directactions;
import android.os.Bundle;
import android.text.TextUtils;
import org.chromium.base.Callback;
import org.chromium.chrome.browser.directactions.DirectActionReporter.Type;
import org.chromium.chrome.browser.findinpage.FindToolbarManager;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tabmodel.TabModelSelector;
/**
* Maps the direct action {@code find_in_page} to Chrome's find in page feature.
*/
public class FindInPageDirectActionHandler implements DirectActionHandler {
private static final String ACTION_ID = ChromeDirectActionIds.FIND_IN_PAGE;
private static final String ARGUMENT_NAME = "SEARCH_QUERY";
private final TabModelSelector mTabModelSelector;
private final FindToolbarManager mFindToolbarManager;
public FindInPageDirectActionHandler(
TabModelSelector tabModelSelector, FindToolbarManager findToolbarManager) {
mTabModelSelector = tabModelSelector;
mFindToolbarManager = findToolbarManager;
}
@Override
public final void reportAvailableDirectActions(DirectActionReporter reporter) {
if (isAvailable()) {
reporter.addDirectAction(ACTION_ID).withParameter(
ARGUMENT_NAME, Type.STRING, /* required = */ true);
}
}
@Override
public final boolean performDirectAction(
String actionId, Bundle arguments, Callback<Bundle> callback) {
if (!ACTION_ID.equals(actionId)) return false;
if (!isAvailable()) return false;
mFindToolbarManager.showToolbar();
String findText = arguments.getString(ARGUMENT_NAME, "");
if (!TextUtils.isEmpty(findText)) {
mFindToolbarManager.setFindQuery(findText);
}
callback.onResult(Bundle.EMPTY);
return true;
}
/** Returns {@code true} if the action is currently available. */
private final boolean isAvailable() {
Tab tab = mTabModelSelector.getCurrentTab();
return !tab.isNativePage() && tab.getWebContents() != null;
}
}
......@@ -726,6 +726,11 @@ public class FindToolbar extends LinearLayout {
mSettingFindTextProgrammatically = false;
}
/** Sets the find query text string. */
void setFindQuery(String findText) {
mFindQuery.setText(findText);
}
/** Clears the result displays (except in-page match highlighting). */
protected void clearResults() {
setStatus("", false);
......
......@@ -66,6 +66,8 @@ public class FindToolbarManager {
/**
* Shows the toolbar if it's not already visible otherwise activates.
*
* TODO(crrev.com/959841): Return a boolean for whether the toolbar was actually shown.
*/
public void showToolbar() {
if (mFindToolbar == null) {
......@@ -93,6 +95,14 @@ public class FindToolbarManager {
mFindToolbar.activate();
}
/**
* Sets the find query text string.
*/
public void setFindQuery(String findText) {
assert mFindToolbar != null;
mFindToolbar.setFindQuery(findText);
}
/**
* Add an observer for find in page changes.
*/
......
......@@ -109,6 +109,14 @@ public class RootUiCoordinator
return mToolbarManager;
}
/**
* @return The find toolbar manager or {@code null} if UI inflation is not yet complete.
*/
@Nullable
public FindToolbarManager getFindToolbarManager() {
return mFindToolbarManager;
}
@Override
public void destroy() {
mMenuOrKeyboardActionController.unregisterMenuOrKeyboardActionHandler(this);
......
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
package org.chromium.chrome.browser.directactions;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.anyString;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import android.os.Bundle;
import android.support.test.filters.SmallTest;
import android.support.v7.widget.AppCompatEditText;
import org.hamcrest.Matchers;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.junit.MockitoJUnit;
import org.mockito.junit.MockitoRule;
import org.robolectric.annotation.Config;
import org.chromium.base.test.BaseRobolectricTestRunner;
import org.chromium.chrome.browser.directactions.DirectActionReporter.Definition;
import org.chromium.chrome.browser.directactions.DirectActionReporter.Type;
import org.chromium.chrome.browser.findinpage.FindToolbarManager;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tabmodel.TabModelSelector;
import org.chromium.content_public.browser.WebContents;
import java.util.ArrayList;
import java.util.List;
/** Tests {@link FindInPageDirectActionHandler}. */
@RunWith(BaseRobolectricTestRunner.class)
@Config(manifest = Config.NONE)
public class FindInPageDirectActionHandlerTest {
@Rule
public MockitoRule mMockitoRule = MockitoJUnit.rule();
@Mock
private DirectActionReporter mMockedReporter;
@Mock
private AppCompatEditText mMockedEditText;
@Mock
private WebContents mWebContents;
@Mock
private FindToolbarManager mMockedFindToolbarManager;
private DirectActionHandler mHandler;
@Before
public void setUp() {
Tab mockTab = mock(Tab.class);
when(mockTab.isNativePage()).thenReturn(false);
when(mockTab.getWebContents()).thenReturn(mWebContents);
TabModelSelector mockTabModelSelector = mock(TabModelSelector.class);
when(mockTabModelSelector.getCurrentTab()).thenReturn(mockTab);
when(mMockedReporter.addDirectAction(org.mockito.Matchers.anyString()))
.thenReturn(new FakeDirectActionDefinition("unused"));
mHandler =
new FindInPageDirectActionHandler(mockTabModelSelector, mMockedFindToolbarManager);
mHandler.reportAvailableDirectActions(mMockedReporter);
verify(mMockedReporter).addDirectAction("find_in_page");
}
@Test
@SmallTest
public void testWrongActionIdNotHandled() {
assertFalse(mHandler.performDirectAction("wrong_name", Bundle.EMPTY, null));
verify(mMockedFindToolbarManager, never()).showToolbar();
}
@Test
@SmallTest
public void testEmptyStringIgnored() {
List<Bundle> responses = new ArrayList<>();
assertTrue(mHandler.performDirectAction(
"find_in_page", Bundle.EMPTY, (response) -> responses.add(response)));
assertThat(responses, Matchers.hasSize(1));
// We still want to bring up the find toolbar.
verify(mMockedFindToolbarManager).showToolbar();
// Setting the search string should not happen.
verify(mMockedFindToolbarManager, never()).setFindQuery(anyString());
}
@Test
@SmallTest
public void testFindInPageFull() {
List<Bundle> responses = new ArrayList<>();
Bundle args = new Bundle();
args.putString("SEARCH_QUERY", "search string");
assertTrue(mHandler.performDirectAction(
"find_in_page", args, (response) -> responses.add(response)));
assertThat(responses, Matchers.hasSize(1));
verify(mMockedFindToolbarManager).showToolbar();
verify(mMockedFindToolbarManager).setFindQuery("search string");
}
/**
* A simple action definition for testing.
*
* TODO(crbug.com/806868): Share these fakes. There is another one in
* features/autofill_assistant.
*/
private static class FakeDirectActionDefinition implements Definition {
final String mId;
List<FakeParameter> mParameters = new ArrayList<>();
List<FakeParameter> mResults = new ArrayList<>();
FakeDirectActionDefinition(String id) {
mId = id;
}
@Override
public Definition withParameter(String name, @Type int type, boolean required) {
mParameters.add(new FakeParameter(name, type, required));
return this;
}
@Override
public Definition withResult(String name, @Type int type) {
mResults.add(new FakeParameter(name, type, true));
return this;
}
}
/** A simple parameter definition for testing. */
private static class FakeParameter {
final String mName;
@Type
final int mType;
final boolean mRequired;
FakeParameter(String name, @Type int type, boolean required) {
mName = name;
mType = type;
mRequired = required;
}
}
}
......@@ -14406,6 +14406,7 @@ to ensure that the crash string is shown properly on the user-facing crash UI.
<int value="10" label="new_tab"/>
<int value="11" label="close_tab"/>
<int value="12" label="close_all_tabs"/>
<int value="13" label="find_in_page"/>
</enum>
<enum name="DirectCompositionVideoPresentationMode">
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