Skip to content

GitLab

T
tangled
Commit c40a0262 authored by Yusuf Ozuysal's avatar Yusuf Ozuysal Committed by Commit Bot
Browse files
Options

Revert "[StartSurface] Add long press behavior for the fake box" 

This reverts commit 14f7eff8.

Reason for revert: Crashes on Canary with voice input

Original change's description:
> [StartSurface] Add long press behavior for the fake box
> 
> This CL is based on
> https://chromium-review.googlesource.com/c/chromium/src/+/1824554/3
> 
> It is the fourth patch of splitting the bigger one
> https://chromium-review.googlesource.com/c/chromium/src/+/1797069
> 
> It also unifies the record of the 'Android.OmniboxFocusReason' metrics.
> 
> Screenshot:
> https://drive.google.com/file/d/1YlxcF40SYtGVj9K0YgviQtnt5tdGFm-m/view?usp=sharing
> 
> Bug: 982018, 1012275
> Change-Id: I16d045777164d49d72356af474e5c186c8318370
> Binary-Size: crbug.com/998206
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1824126
> Commit-Queue: Ganggui Tang <gogerald@chromium.org>
> Reviewed-by: Yusuf Ozuysal <yusufo@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#705264}

TBR=yusufo@chromium.org,gogerald@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 982018, 1012275, 1013943
Change-Id: I4a48866d15dc65e8f939fe1c86867754eea87604
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1860598Reviewed-by: default avatarYusuf Ozuysal <yusufo@chromium.org>
Commit-Queue: Yusuf Ozuysal <yusufo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#705727}
parent 1d5cb12a
Hide whitespace changes
Inline Side-by-side
Showing with 91 additions and 225 deletions
chrome/android/features/start_surface/internal/java/src/org/chromium/chrome/features/start_surface/StartSurfaceLocationBarDelegate.java
View file @ c40a0262
......@@ -22,10 +22,7 @@ class StartSurfaceLocationBarDelegate
// Implements TasksSurface.FakeSearchBoxDelegate.
@Override
public void requestUrlFocus(@Nullable String pastedText) {
mLocationBar.setUrlBarFocus(true, pastedText,
pastedText == null
? LocationBar.OmniboxFocusReason.TASKS_SURFACE_FAKE_BOX_TAP
: LocationBar.OmniboxFocusReason.TASKS_SURFACE_FAKE_BOX_LONG_PRESS);
mLocationBar.setUrlBarFocus(true);
}
// Implements StartSurfaceMediator.Delegate.
......
chrome/android/features/tab_ui/java/src/org/chromium/chrome/browser/tasks/TasksSurfaceMediator.java
View file @ c40a0262
......@@ -5,15 +5,12 @@
package org.chromium.chrome.browser.tasks;
import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.FAKE_SEARCH_BOX_CLICK_LISTENER;
import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.FAKE_SEARCH_BOX_TEXT_WATCHER;
import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.IS_FAKE_SEARCH_BOX_VISIBLE;
import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.IS_TAB_CAROUSEL;
import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.IS_VOICE_RECOGNITION_BUTTON_VISIBLE;
import android.content.Context;
import android.support.annotation.Nullable;
import android.text.Editable;
import android.text.TextWatcher;
import android.view.View;
import org.chromium.ui.modelutil.PropertyModel;
......@@ -39,28 +36,13 @@ class TasksSurfaceMediator {
mFakeSearchBoxDelegate.requestUrlFocus(null);
}
});
mModel.set(FAKE_SEARCH_BOX_TEXT_WATCHER, new TextWatcher() {
@Override
public void beforeTextChanged(CharSequence s, int start, int count, int after) {}
@Override
public void onTextChanged(CharSequence s, int start, int before, int count) {}
@Override
public void afterTextChanged(Editable s) {
if (s.length() == 0) return;
mFakeSearchBoxDelegate.requestUrlFocus(s.toString());
// This won't cause infinite loop since we checked s.length() == 0 above.
s.clear();
}
});
// Set the initial state.
mModel.set(IS_FAKE_SEARCH_BOX_VISIBLE, true);
mModel.set(IS_VOICE_RECOGNITION_BUTTON_VISIBLE, false);
// TODO(crbug.com/982018): Enable voice recognition button in the fake search box.
// TODO(crbug.com/982018): Enable long press and paste search in the fake search box.
// TODO(crbug.com/982018): Change the fake search box in dark mode.
}
}
chrome/android/features/tab_ui/java/src/org/chromium/chrome/browser/tasks/TasksSurfaceProperties.java
View file @ c40a0262
......@@ -6,7 +6,6 @@ package org.chromium.chrome.browser.tasks;
import static org.chromium.chrome.browser.tasks.MostVisitedListProperties.IS_VISIBLE;
import android.text.TextWatcher;
import android.view.View;
import org.chromium.ui.modelutil.PropertyKey;
......@@ -28,15 +27,11 @@ public class TasksSurfaceProperties {
public static final PropertyModel
.WritableObjectPropertyKey<View.OnClickListener> FAKE_SEARCH_BOX_CLICK_LISTENER =
new PropertyModel.WritableObjectPropertyKey<View.OnClickListener>();
public static final PropertyModel
.WritableObjectPropertyKey<TextWatcher> FAKE_SEARCH_BOX_TEXT_WATCHER =
new PropertyModel.WritableObjectPropertyKey<TextWatcher>();
public static final PropertyModel
.WritableObjectPropertyKey<View.OnClickListener> MORE_TABS_CLICK_LISTENER =
new PropertyModel.WritableObjectPropertyKey<View.OnClickListener>();
public static final PropertyModel.WritableBooleanPropertyKey MV_TILES_VISIBLE = IS_VISIBLE;
public static final PropertyKey[] ALL_KEYS =
new PropertyKey[] {IS_FAKE_SEARCH_BOX_VISIBLE, IS_INCOGNITO, IS_TAB_CAROUSEL,
IS_VOICE_RECOGNITION_BUTTON_VISIBLE, FAKE_SEARCH_BOX_CLICK_LISTENER,
FAKE_SEARCH_BOX_TEXT_WATCHER, MORE_TABS_CLICK_LISTENER, MV_TILES_VISIBLE};
public static final PropertyKey[] ALL_KEYS = new PropertyKey[] {IS_FAKE_SEARCH_BOX_VISIBLE,
IS_INCOGNITO, IS_TAB_CAROUSEL, IS_VOICE_RECOGNITION_BUTTON_VISIBLE,
FAKE_SEARCH_BOX_CLICK_LISTENER, MORE_TABS_CLICK_LISTENER, MV_TILES_VISIBLE};
}
chrome/android/features/tab_ui/java/src/org/chromium/chrome/browser/tasks/TasksView.java
View file @ c40a0262
......@@ -7,7 +7,6 @@ package org.chromium.chrome.browser.tasks;
import android.content.Context;
import android.content.res.Resources;
import android.support.v4.view.MarginLayoutParamsCompat;
import android.text.TextWatcher;
import android.util.AttributeSet;
import android.view.View;
import android.view.ViewGroup;
......@@ -76,14 +75,6 @@ class TasksView extends LinearLayout {
mSearchBoxText.setOnClickListener(listener);
}
/**
* Set the given watcher for the fake search box.
* @param textWatcher The given {@link TextWatcher}.
*/
void setFakeSearchBoxTextWatcher(TextWatcher textWatcher) {
mSearchBoxText.addTextChangedListener(textWatcher);
}
/**
* Set the visibility of the fake search box.
* @param isVisible Whether it's visible.
......
chrome/android/features/tab_ui/java/src/org/chromium/chrome/browser/tasks/TasksViewBinder.java
View file @ c40a0262
......@@ -5,7 +5,6 @@
package org.chromium.chrome.browser.tasks;
import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.FAKE_SEARCH_BOX_CLICK_LISTENER;
import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.FAKE_SEARCH_BOX_TEXT_WATCHER;
import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.IS_FAKE_SEARCH_BOX_VISIBLE;
import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.IS_INCOGNITO;
import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.IS_TAB_CAROUSEL;
......@@ -23,8 +22,6 @@ class TasksViewBinder {
public static void bind(PropertyModel model, TasksView view, PropertyKey propertyKey) {
if (propertyKey == FAKE_SEARCH_BOX_CLICK_LISTENER) {
view.setFakeSearchBoxClickListener(model.get(FAKE_SEARCH_BOX_CLICK_LISTENER));
} else if (propertyKey == FAKE_SEARCH_BOX_TEXT_WATCHER) {
view.setFakeSearchBoxTextWatcher(model.get(FAKE_SEARCH_BOX_TEXT_WATCHER));
} else if (propertyKey == IS_FAKE_SEARCH_BOX_VISIBLE) {
view.setFakeSearchBoxVisibility(model.get(IS_FAKE_SEARCH_BOX_VISIBLE));
} else if (propertyKey == IS_INCOGNITO) {
......
chrome/android/java/src/org/chromium/chrome/browser/ChromeTabbedActivity.java
View file @ c40a0262
......@@ -106,7 +106,6 @@ import org.chromium.chrome.browser.night_mode.WebContentsDarkModeController;
import org.chromium.chrome.browser.ntp.NewTabPage;
import org.chromium.chrome.browser.ntp.NewTabPageUma;
import org.chromium.chrome.browser.omaha.OmahaBase;
import org.chromium.chrome.browser.omnibox.LocationBar;
import org.chromium.chrome.browser.partnercustomizations.HomepageManager;
import org.chromium.chrome.browser.partnercustomizations.PartnerBrowserCustomizations;
import org.chromium.chrome.browser.preferences.ChromePreferenceManager;
......@@ -1533,9 +1532,7 @@ public class ChromeTabbedActivity extends ChromeActivity implements ScreenshotMo
assert false : "Unknown TabOpenType: " + tabOpenType;
break;
}
getToolbarManager().setUrlBarFocusOnceNativeInitialized(focus,
focus ? LocationBar.OmniboxFocusReason.LAUNCH_NEW_INCOGNITO_TAB
: LocationBar.OmniboxFocusReason.UNFOCUS);
getToolbarManager().setUrlBarFocusOnceNativeInitialized(focus);
}
@Override
......@@ -1831,8 +1828,7 @@ public class ChromeTabbedActivity extends ChromeActivity implements ScreenshotMo
boolean isUrlBarVisible = !mOverviewModeController.overviewVisible()
&& (!isTablet() || getCurrentTabModel().getCount() != 0);
if (isUrlBarVisible) {
getToolbarManager().setUrlBarFocus(
true, LocationBar.OmniboxFocusReason.MENU_OR_KEYBOARD_ACTION);
getToolbarManager().setUrlBarFocus(true);
}
} else if (id == R.id.downloads_menu_id) {
DownloadUtils.showDownloadManager(this, currentTab, DownloadOpenSource.MENU);
......
chrome/android/java/src/org/chromium/chrome/browser/modaldialog/TabModalPresenter.java
View file @ c40a0262
......@@ -22,7 +22,6 @@ import org.chromium.chrome.browser.ChromeActivity;
import org.chromium.chrome.browser.compositor.bottombar.OverlayPanel;
import org.chromium.chrome.browser.contextualsearch.ContextualSearchManager;
import org.chromium.chrome.browser.fullscreen.ChromeFullscreenManager;
import org.chromium.chrome.browser.omnibox.LocationBar;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tab.TabAttributeKeys;
import org.chromium.chrome.browser.tab.TabAttributes;
......@@ -325,8 +324,7 @@ public class TabModalPresenter
// Force toolbar to show and disable overflow menu.
onTabModalDialogStateChanged(true);
mChromeActivity.getToolbarManager().setUrlBarFocus(
false, LocationBar.OmniboxFocusReason.UNFOCUS);
mChromeActivity.getToolbarManager().setUrlBarFocus(false);
menuButton.setEnabled(false);
} else {
......
chrome/android/java/src/org/chromium/chrome/browser/omnibox/LocationBar.java
View file @ c40a0262
......@@ -9,9 +9,6 @@ import android.view.View;
import android.view.ViewGroup;
import android.view.Window;
import androidx.annotation.IntDef;
import androidx.annotation.Nullable;
import org.chromium.chrome.browser.ActivityTabProvider;
import org.chromium.chrome.browser.WindowDelegate;
import org.chromium.chrome.browser.ntp.NewTabPage;
......@@ -23,41 +20,10 @@ import org.chromium.chrome.browser.toolbar.top.Toolbar;
import org.chromium.chrome.browser.toolbar.top.ToolbarActionModeCallback;
import org.chromium.ui.base.WindowAndroid;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
/**
* Container that holds the {@link UrlBar} and SSL state related with the current {@link Tab}.
*/
public interface LocationBar extends UrlBarDelegate {
/** A means of tracking which mechanism is being used to focus the omnibox. */
@IntDef({OmniboxFocusReason.OMNIBOX_TAP, OmniboxFocusReason.OMNIBOX_LONG_PRESS,
OmniboxFocusReason.FAKE_BOX_TAP, OmniboxFocusReason.FAKE_BOX_LONG_PRESS,
OmniboxFocusReason.ACCELERATOR_TAP, OmniboxFocusReason.TAB_SWITCHER_OMNIBOX_TAP,
OmniboxFocusReason.TASKS_SURFACE_FAKE_BOX_TAP,
OmniboxFocusReason.TASKS_SURFACE_FAKE_BOX_LONG_PRESS,
OmniboxFocusReason.DEFAULT_WITH_HARDWARE_KEYBOARD, OmniboxFocusReason.SEARCH_QUERY,
OmniboxFocusReason.LAUNCH_NEW_INCOGNITO_TAB, OmniboxFocusReason.MENU_OR_KEYBOARD_ACTION,
OmniboxFocusReason.UNFOCUS})
@Retention(RetentionPolicy.SOURCE)
public @interface OmniboxFocusReason {
int OMNIBOX_TAP = 0;
int OMNIBOX_LONG_PRESS = 1;
int FAKE_BOX_TAP = 2;
int FAKE_BOX_LONG_PRESS = 3;
int ACCELERATOR_TAP = 4;
// TAB_SWITCHER_OMNIBOX_TAP has not been used anymore, keep it for record for now.
int TAB_SWITCHER_OMNIBOX_TAP = 5;
int TASKS_SURFACE_FAKE_BOX_TAP = 6;
int TASKS_SURFACE_FAKE_BOX_LONG_PRESS = 7;
int DEFAULT_WITH_HARDWARE_KEYBOARD = 8;
int SEARCH_QUERY = 9;
int LAUNCH_NEW_INCOGNITO_TAB = 10;
int MENU_OR_KEYBOARD_ACTION = 11;
int UNFOCUS = 12;
int NUM_ENTRIES = 13;
}
/**
* Cleanup resources when this goes out of scope.
*/
......@@ -156,11 +122,8 @@ public interface LocationBar extends UrlBarDelegate {
* Signal a {@link UrlBar} focus change request.
* @param shouldBeFocused Whether the focus should be requested or cleared. True requests focus
* and False clears focus.
* @param pastedText The given pasted text when focus, which could be null.
* @param reason The given reason.
*/
void setUrlBarFocus(
boolean shouldBeFocused, @Nullable String pastedText, @OmniboxFocusReason int reason);
void setUrlBarFocus(boolean shouldBeFocused);
/**
* Triggers the cursor to be visible in the UrlBar without triggering any of the focus animation
......
chrome/android/java/src/org/chromium/chrome/browser/omnibox/LocationBarLayout.java
View file @ c40a0262
......@@ -30,7 +30,6 @@ import org.chromium.base.ApiCompatibilityUtils;
import org.chromium.base.CommandLine;
import org.chromium.base.ObserverList;
import org.chromium.base.VisibleForTesting;
import org.chromium.base.metrics.CachedMetrics.EnumeratedHistogramSample;
import org.chromium.base.metrics.RecordUserAction;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.ActivityTabProvider;
......@@ -42,7 +41,6 @@ import org.chromium.chrome.browser.native_page.NativePageFactory;
import org.chromium.chrome.browser.ntp.NewTabPage;
import org.chromium.chrome.browser.ntp.NewTabPage.FakeboxDelegate;
import org.chromium.chrome.browser.ntp.NewTabPageUma;
import org.chromium.chrome.browser.omnibox.LocationBar.OmniboxFocusReason;
import org.chromium.chrome.browser.omnibox.UrlBar.ScrollType;
import org.chromium.chrome.browser.omnibox.UrlBarCoordinator.SelectionState;
import org.chromium.chrome.browser.omnibox.geo.GeolocationHeader;
......@@ -58,6 +56,7 @@ import org.chromium.chrome.browser.search_engines.TemplateUrlServiceFactory;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tasks.ReturnToChromeExperimentsUtil;
import org.chromium.chrome.browser.toolbar.ToolbarDataProvider;
import org.chromium.chrome.browser.toolbar.ToolbarManager;
import org.chromium.chrome.browser.toolbar.top.ToolbarActionModeCallback;
import org.chromium.chrome.browser.ui.widget.CompositeTouchDelegate;
import org.chromium.chrome.browser.util.AccessibilityUtil;
......@@ -78,10 +77,6 @@ import java.util.List;
public class LocationBarLayout extends FrameLayout
implements OnClickListener, LocationBar, AutocompleteDelegate, FakeboxDelegate,
LocationBarVoiceRecognitionHandler.Delegate {
private static final EnumeratedHistogramSample ENUMERATED_FOCUS_REASON =
new EnumeratedHistogramSample(
"Android.OmniboxFocusReason", OmniboxFocusReason.NUM_ENTRIES);
protected ImageButton mDeleteButton;
protected ImageButton mMicButton;
protected UrlBar mUrlBar;
......@@ -264,7 +259,7 @@ public class LocationBarLayout extends FrameLayout
&& newConfig.keyboard != Configuration.KEYBOARD_QWERTY) {
// If we lose the hardware keyboard and the focus animations were not run, then the
// user has not typed any text, so we will just clear the focus instead.
setUrlBarFocus(false, null, LocationBar.OmniboxFocusReason.UNFOCUS);
setUrlBarFocus(false);
}
}
......@@ -371,35 +366,13 @@ public class LocationBarLayout extends FrameLayout
}
@Override
public void setUrlBarFocus(
boolean shouldBeFocused, @Nullable String pastedText, @OmniboxFocusReason int reason) {
public void setUrlBarFocus(boolean shouldBeFocused) {
if (shouldBeFocused) {
if (!mUrlHasFocus) recordOmniboxFocusReason(reason);
if (reason == LocationBar.OmniboxFocusReason.FAKE_BOX_TAP
|| reason == LocationBar.OmniboxFocusReason.FAKE_BOX_LONG_PRESS
|| reason == LocationBar.OmniboxFocusReason.TASKS_SURFACE_FAKE_BOX_LONG_PRESS
|| reason == LocationBar.OmniboxFocusReason.TASKS_SURFACE_FAKE_BOX_TAP) {
mUrlFocusedFromFakebox = true;
}
if (mUrlHasFocus && mUrlFocusedWithoutAnimations) {
handleUrlFocusAnimation(mUrlHasFocus);
} else {
mUrlBar.requestFocus();
}
mUrlBar.requestFocus();
} else {
assert pastedText == null;
hideKeyboard();
mUrlBar.clearFocus();
}
if (pastedText != null) {
// This must be happen after requestUrlFocus(), which changes the selection.
mUrlCoordinator.setUrlBarData(UrlBarData.forNonUrlText(pastedText),
UrlBar.ScrollType.NO_SCROLL, UrlBarCoordinator.SelectionState.SELECT_END);
forceOnTextChanged();
}
}
@Override
......@@ -409,7 +382,7 @@ public class LocationBarLayout extends FrameLayout
@Override
public void clearOmniboxFocus() {
setUrlBarFocus(false, null, LocationBar.OmniboxFocusReason.UNFOCUS);
setUrlBarFocus(false);
}
@Override
......@@ -562,11 +535,23 @@ public class LocationBarLayout extends FrameLayout
@Override
public void requestUrlFocusFromFakebox(String pastedText) {
// TODO(crbug.com/1013693): Get rid of requestUrlFocusFromFakebox to let the caller uses
// setUrlBarFocus directly.
setUrlBarFocus(true, pastedText,
pastedText == null ? LocationBar.OmniboxFocusReason.FAKE_BOX_TAP
: LocationBar.OmniboxFocusReason.FAKE_BOX_LONG_PRESS);
mUrlFocusedFromFakebox = true;
if (mUrlHasFocus && mUrlFocusedWithoutAnimations) {
handleUrlFocusAnimation(mUrlHasFocus);
} else {
setUrlBarFocus(true);
}
if (pastedText != null) {
ToolbarManager.recordOmniboxFocusReason(
ToolbarManager.OmniboxFocusReason.FAKE_BOX_LONG_PRESS);
// This must be happen after requestUrlFocus(), which changes the selection.
mUrlCoordinator.setUrlBarData(UrlBarData.forNonUrlText(pastedText),
UrlBar.ScrollType.NO_SCROLL, UrlBarCoordinator.SelectionState.SELECT_END);
forceOnTextChanged();
} else {
ToolbarManager.recordOmniboxFocusReason(ToolbarManager.OmniboxFocusReason.FAKE_BOX_TAP);
}
}
@Override
......@@ -585,10 +570,7 @@ public class LocationBarLayout extends FrameLayout
if (mUrlHasFocus || mUrlFocusedFromFakebox) return;
mUrlFocusedWithoutAnimations = true;
// This interface should only be called to devices with a hardware keyboard attached as
// described in the LocationBar.
setUrlBarFocus(true, null, LocationBar.OmniboxFocusReason.DEFAULT_WITH_HARDWARE_KEYBOARD);
setUrlBarFocus(true);
}
/**
......@@ -817,7 +799,7 @@ public class LocationBarLayout extends FrameLayout
setUrlBarText(UrlBarData.forNonUrlText(query), UrlBar.ScrollType.NO_SCROLL,
SelectionState.SELECT_ALL);
setUrlBarFocus(true, null, LocationBar.OmniboxFocusReason.SEARCH_QUERY);
setUrlBarFocus(true);
mAutocompleteCoordinator.startAutocompleteForQuery(query);
post(new Runnable() {
@Override
......@@ -844,7 +826,7 @@ public class LocationBarLayout extends FrameLayout
@Override
public void backKeyPressed() {
setUrlBarFocus(false, null, LocationBar.OmniboxFocusReason.UNFOCUS);
setUrlBarFocus(false);
// Revert the URL to match the current page.
setUrlToPageUrl();
focusCurrentTab();
......@@ -863,12 +845,6 @@ public class LocationBarLayout extends FrameLayout
return mToolbarDataProvider.getDisplaySearchTerms() != null;
}
@Override
public void gestureDetected(boolean isLongPress) {
recordOmniboxFocusReason(isLongPress ? LocationBar.OmniboxFocusReason.OMNIBOX_LONG_PRESS
: LocationBar.OmniboxFocusReason.OMNIBOX_TAP);
}
/**
* @return Returns the original url of the page.
*/
......@@ -894,7 +870,7 @@ public class LocationBarLayout extends FrameLayout
// If we did not run the focus animations, then the user has not typed any text.
// So, clear the focus and accept whatever URL the page is currently attempting to
// display. If the NTP is showing, the current page's URL should not be displayed.
setUrlBarFocus(false, null, LocationBar.OmniboxFocusReason.UNFOCUS);
setUrlBarFocus(false);
} else {
return;
}
......@@ -1143,8 +1119,4 @@ public class LocationBarLayout extends FrameLayout
String textWithAutocomplete = mUrlCoordinator.getTextWithAutocomplete();
mAutocompleteCoordinator.onTextChanged(textWithoutAutocomplete, textWithAutocomplete);
}
private void recordOmniboxFocusReason(@OmniboxFocusReason int reason) {
ENUMERATED_FOCUS_REASON.record(reason);
}
}
chrome/android/java/src/org/chromium/chrome/browser/omnibox/UrlBar.java
View file @ c40a0262
......@@ -42,6 +42,7 @@ import org.chromium.base.SysUtils;
import org.chromium.base.ThreadUtils;
import org.chromium.base.metrics.CachedMetrics;
import org.chromium.chrome.browser.WindowDelegate;
import org.chromium.chrome.browser.toolbar.ToolbarManager;
import org.chromium.ui.KeyboardVisibilityDelegate;
import java.lang.annotation.Retention;
......@@ -209,12 +210,6 @@ public abstract class UrlBar extends AutocompleteEditText {
* whatever's in the URL bar verbatim.
*/
boolean shouldCutCopyVerbatim();
/**
* Called to notify that a tap or long press gesture has been detected.
* @param isLongPress Whether or not is a long press gesture.
*/
void gestureDetected(boolean isLongPress);
}
/** Provides updates about the URL text changes. */
......@@ -277,14 +272,16 @@ public abstract class UrlBar extends AutocompleteEditText {
new GestureDetector(getContext(), new GestureDetector.SimpleOnGestureListener() {
@Override
public void onLongPress(MotionEvent e) {
mUrlBarDelegate.gestureDetected(true);
ToolbarManager.recordOmniboxFocusReason(
ToolbarManager.OmniboxFocusReason.OMNIBOX_LONG_PRESS);
performLongClick();
}
@Override
public boolean onSingleTapUp(MotionEvent e) {
requestFocus();
mUrlBarDelegate.gestureDetected(false);
ToolbarManager.recordOmniboxFocusReason(
ToolbarManager.OmniboxFocusReason.OMNIBOX_TAP);
return true;
}
}, ThreadUtils.getUiThreadHandler());
......
chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarManager.java
View file @ c40a0262
......@@ -19,6 +19,7 @@ import android.view.View.OnClickListener;
import android.view.View.OnLongClickListener;
import android.view.ViewGroup;
import androidx.annotation.IntDef;
import androidx.annotation.Nullable;
import androidx.annotation.StringRes;
......@@ -26,6 +27,7 @@ import org.chromium.base.ApiCompatibilityUtils;
import org.chromium.base.Callback;
import org.chromium.base.VisibleForTesting;
import org.chromium.base.metrics.CachedMetrics.ActionEvent;
import org.chromium.base.metrics.CachedMetrics.EnumeratedHistogramSample;
import org.chromium.base.metrics.RecordHistogram;
import org.chromium.base.metrics.RecordUserAction;
import org.chromium.chrome.R;
......@@ -123,6 +125,8 @@ import org.chromium.ui.base.PageTransition;
import org.chromium.ui.widget.Toast;
import org.chromium.ui.widget.ViewRectProvider;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.util.List;
/**
......@@ -143,6 +147,23 @@ public class ToolbarManager implements ScrimObserver, ToolbarTabController, UrlF
void updateReloadButtonState(boolean isLoading);
}
/** A means of tracking which mechanism is being used to focus the omnibox. */
@IntDef({OmniboxFocusReason.OMNIBOX_TAP, OmniboxFocusReason.OMNIBOX_LONG_PRESS,
OmniboxFocusReason.FAKE_BOX_TAP, OmniboxFocusReason.FAKE_BOX_LONG_PRESS,
OmniboxFocusReason.ACCELERATOR_TAP, OmniboxFocusReason.TAB_SWITCHER_OMNIBOX_TAP})
@Retention(RetentionPolicy.SOURCE)
public @interface OmniboxFocusReason {
int OMNIBOX_TAP = 0;
int OMNIBOX_LONG_PRESS = 1;
int FAKE_BOX_TAP = 2;
int FAKE_BOX_LONG_PRESS = 3;
int ACCELERATOR_TAP = 4;
int TAB_SWITCHER_OMNIBOX_TAP = 5;
int NUM_ENTRIES = 6;
}
private static final EnumeratedHistogramSample ENUMERATED_FOCUS_REASON =
new EnumeratedHistogramSample(
"Android.OmniboxFocusReason", OmniboxFocusReason.NUM_ENTRIES);
private static final ActionEvent ACCELERATOR_BUTTON_TAP_ACTION =
new ActionEvent("MobileToolbarOmniboxAcceleratorTap");
......@@ -725,7 +746,7 @@ public class ToolbarManager implements ScrimObserver, ToolbarTabController, UrlF
@Override
public void onScrimClick() {
setUrlBarFocus(false, LocationBar.OmniboxFocusReason.UNFOCUS);
setUrlBarFocus(false);
}
/**
......@@ -735,6 +756,17 @@ public class ToolbarManager implements ScrimObserver, ToolbarTabController, UrlF
return mLocationBar.isUrlBarFocused();
}
/**
* @param reason A {@link OmniboxFocusReason} that the omnibox was focused.
*/
public static void recordOmniboxFocusReason(@OmniboxFocusReason int reason) {
if (OmniboxFocusReason.OMNIBOX_TAP == reason
&& ReturnToChromeExperimentsUtil.isInOverviewWithOmnibox()) {
reason = OmniboxFocusReason.TAB_SWITCHER_OMNIBOX_TAP;
}
ENUMERATED_FOCUS_REASON.record(reason);
}
/**
* Enable the bottom toolbar.
*/
......@@ -747,8 +779,9 @@ public class ToolbarManager implements ScrimObserver, ToolbarTabController, UrlF
final OnClickListener searchAcceleratorListener = v -> {
recordBottomToolbarUseForIPH();
recordOmniboxFocusReason(OmniboxFocusReason.ACCELERATOR_TAP);
ACCELERATOR_BUTTON_TAP_ACTION.record();
setUrlBarFocus(true, LocationBar.OmniboxFocusReason.ACCELERATOR_TAP);
setUrlBarFocus(true);
};
final OnClickListener shareButtonListener = v -> {
......@@ -1341,7 +1374,7 @@ public class ToolbarManager implements ScrimObserver, ToolbarTabController, UrlF
if (isVisible) {
// Defocus here to avoid handling focus in multiple places, e.g., when the
// forward button is pressed. (see crbug.com/414219)
setUrlBarFocus(false, LocationBar.OmniboxFocusReason.UNFOCUS);
setUrlBarFocus(false);
if (!mActivity.isInOverviewMode() && isShowingAppMenuUpdateBadge()) {
// The app menu badge should be removed the first time the menu is opened.
......@@ -1642,12 +1675,11 @@ public class ToolbarManager implements ScrimObserver, ToolbarTabController, UrlF
* If you request focus and the UrlBar was already focused, this will select all of the text.
*
* @param focused Whether URL bar should be focused.
* @param reason The given reason.
*/
public void setUrlBarFocus(boolean focused, @LocationBar.OmniboxFocusReason int reason) {
public void setUrlBarFocus(boolean focused) {
if (!isInitialized()) return;
boolean wasFocused = mLocationBar.isUrlBarFocused();
mLocationBar.setUrlBarFocus(focused, null, reason);
mLocationBar.setUrlBarFocus(focused);
if (wasFocused && focused) {
mLocationBar.selectAll();
}
......@@ -1657,10 +1689,9 @@ public class ToolbarManager implements ScrimObserver, ToolbarTabController, UrlF
* See {@link #setUrlBarFocus}, but if native is not loaded it will queue the request instead
* of dropping it.
*/
public void setUrlBarFocusOnceNativeInitialized(
boolean focused, @LocationBar.OmniboxFocusReason int reason) {
public void setUrlBarFocusOnceNativeInitialized(boolean focused) {
if (isInitialized()) {
setUrlBarFocus(focused, reason);
setUrlBarFocus(focused);
return;
}
......@@ -1669,7 +1700,7 @@ public class ToolbarManager implements ScrimObserver, ToolbarTabController, UrlF
// initialized. This is important for the Launch to Incognito Tab flow (see
// IncognitoTabLauncher.
mOnInitializedRunnable = () -> {
setUrlBarFocus(focused, reason);
setUrlBarFocus(focused);
};
} else {
mOnInitializedRunnable = null;
......@@ -1828,7 +1859,7 @@ public class ToolbarManager implements ScrimObserver, ToolbarTabController, UrlF
// Ensure the URL bar loses focus if the tab it was interacting with is changed from
// underneath it.
setUrlBarFocus(false, LocationBar.OmniboxFocusReason.UNFOCUS);
setUrlBarFocus(false);
// Place the cursor in the Omnibox if applicable. We always clear the focus above to
// ensure the shield placed over the content is dismissed when switching tabs. But if
......
chrome/android/java/src/org/chromium/chrome/browser/toolbar/top/CustomTabToolbar.java
View file @ c40a0262
......@@ -608,9 +608,6 @@ public class CustomTabToolbar extends ToolbarLayout implements View.OnLongClickL
return false;
}
@Override
public void gestureDetected(boolean isLongPress) {}
@Override
public void setShowTitle(boolean showTitle) {
if (showTitle) {
......@@ -806,8 +803,7 @@ public class CustomTabToolbar extends ToolbarLayout implements View.OnLongClickL
}
@Override
public void setUrlBarFocus(boolean shouldBeFocused, @Nullable String pastedText,
@LocationBar.OmniboxFocusReason int reason) {}
public void setUrlBarFocus(boolean shouldBeFocused) {}
@Override
public void showUrlBarCursorWithoutFocusAnimations() {}
......
chrome/android/java/src/org/chromium/chrome/browser/toolbar/top/ToolbarLayout.java
View file @ c40a0262
......@@ -797,9 +797,7 @@ public abstract class ToolbarLayout
* @return Whether or not the current Tab did go back.
*/
boolean back() {
if (getLocationBar() != null) {
getLocationBar().setUrlBarFocus(false, null, LocationBar.OmniboxFocusReason.UNFOCUS);
}
if (getLocationBar() != null) getLocationBar().setUrlBarFocus(false);
return mToolbarTabController != null && mToolbarTabController.back() != null;
}
......@@ -808,9 +806,7 @@ public abstract class ToolbarLayout
* @return Whether or not the current Tab did go forward.
*/
boolean forward() {
if (getLocationBar() != null) {
getLocationBar().setUrlBarFocus(false, null, LocationBar.OmniboxFocusReason.UNFOCUS);
}
if (getLocationBar() != null) getLocationBar().setUrlBarFocus(false);
return mToolbarTabController != null ? mToolbarTabController.forward() : false;
}
......@@ -821,9 +817,7 @@ public abstract class ToolbarLayout
* <p>The buttons of the toolbar will be updated as a result of making this call.
*/
void stopOrReloadCurrentTab() {
if (getLocationBar() != null) {
getLocationBar().setUrlBarFocus(false, null, LocationBar.OmniboxFocusReason.UNFOCUS);
}
if (getLocationBar() != null) getLocationBar().setUrlBarFocus(false);
if (mToolbarTabController != null) mToolbarTabController.stopOrReloadCurrentTab();
}
......@@ -831,9 +825,7 @@ public abstract class ToolbarLayout
* Opens hompage in the current tab.
*/
void openHomepage() {
if (getLocationBar() != null) {
getLocationBar().setUrlBarFocus(false, null, LocationBar.OmniboxFocusReason.UNFOCUS);
}
if (getLocationBar() != null) getLocationBar().setUrlBarFocus(false);
if (mToolbarTabController != null) mToolbarTabController.openHomepage();
}
......
chrome/android/java/src/org/chromium/chrome/browser/toolbar/top/ToolbarPhone.java
View file @ c40a0262
......@@ -2199,6 +2199,10 @@ public class ToolbarPhone extends ToolbarLayout implements Invalidator.Client, O
if (getToolbarDataProvider().shouldShowLocationBarInOverviewMode()) {
mLocationBar.updateStatusIcon();
if (getToolbarDataProvider().isInOverviewAndShowingOmnibox()) {
mUrlBar.setText("");
}
}
}
});
......
chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/LocationBarLayoutTest.java
View file @ c40a0262
......@@ -20,7 +20,6 @@ import org.junit.Test;
import org.junit.runner.RunWith;
import org.chromium.base.ContextUtils;
import org.chromium.base.metrics.RecordHistogram;
import org.chromium.base.test.util.CommandLineFlags;
import org.chromium.base.test.util.Feature;
import org.chromium.base.test.util.Restriction;
......@@ -251,48 +250,4 @@ public class LocationBarLayoutTest {
onView(withId(R.id.location_bar_status))
.check((view, e) -> Assert.assertEquals(iconView.getVisibility(), GONE));
}
@Test
@SmallTest
public void testSetUrlBarFocus() {
final LocationBarLayout locationBar = getLocationBar();
Assert.assertEquals(
0, RecordHistogram.getHistogramTotalCountForTesting("Android.OmniboxFocusReason"));
TestThreadUtils.runOnUiThreadBlocking(() -> {
locationBar.setUrlBarFocus(
true, SEARCH_TERMS_URL, LocationBar.OmniboxFocusReason.FAKE_BOX_LONG_PRESS);
});
Assert.assertTrue(locationBar.isUrlBarFocused());
Assert.assertTrue(locationBar.didFocusUrlFromFakebox());
Assert.assertEquals(SEARCH_TERMS_URL, getUrlText(getUrlBar()));
Assert.assertEquals(
1, RecordHistogram.getHistogramTotalCountForTesting("Android.OmniboxFocusReason"));
TestThreadUtils.runOnUiThreadBlocking(() -> {
locationBar.setUrlBarFocus(
true, SEARCH_TERMS, LocationBar.OmniboxFocusReason.SEARCH_QUERY);
});
Assert.assertTrue(locationBar.isUrlBarFocused());
Assert.assertTrue(locationBar.didFocusUrlFromFakebox());
Assert.assertEquals(SEARCH_TERMS, getUrlText(getUrlBar()));
Assert.assertEquals(
1, RecordHistogram.getHistogramTotalCountForTesting("Android.OmniboxFocusReason"));
TestThreadUtils.runOnUiThreadBlocking(() -> {
locationBar.setUrlBarFocus(false, null, LocationBar.OmniboxFocusReason.UNFOCUS);
});
Assert.assertFalse(locationBar.isUrlBarFocused());
Assert.assertFalse(locationBar.didFocusUrlFromFakebox());
Assert.assertEquals(
1, RecordHistogram.getHistogramTotalCountForTesting("Android.OmniboxFocusReason"));
TestThreadUtils.runOnUiThreadBlocking(() -> {
locationBar.setUrlBarFocus(true, null, LocationBar.OmniboxFocusReason.OMNIBOX_TAP);
});
Assert.assertTrue(locationBar.isUrlBarFocused());
Assert.assertFalse(locationBar.didFocusUrlFromFakebox());
Assert.assertEquals(
2, RecordHistogram.getHistogramTotalCountForTesting("Android.OmniboxFocusReason"));
}
}
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