Commit 85cb329f authored by spdonghao's avatar spdonghao Committed by Chromium LUCI CQ

Make manually typing in chrome://newtab in the start surface work.

In ReturnToChromeExperimentsUtil, make "chrome://newtab" be handled
by Start Surface. When handling this url,
ToolbarPhone#handleOmniboxInOverviewMode is called, in which
ToolbarPhone#triggerUrlFocusAnimation is called. This function may cause
the keyboard to show unnecessarily. A check "shouldShowKeyboard" is
added to avoid this.

Bug: 1059406
Change-Id: I4abe73f3af16285334277b3a1fb9f74bd3c925ea
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2551695Reviewed-by: default avatarYusuf Ozuysal <yusufo@chromium.org>
Reviewed-by: default avatarXi Han <hanxi@chromium.org>
Reviewed-by: default avatarWei-Yin Chen (陳威尹) <wychen@chromium.org>
Commit-Queue: Hao Dong <spdonghao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#835374}
parent a3f885f3
......@@ -2069,7 +2069,14 @@ public class ChromeTabbedActivity extends ChromeActivity<ChromeActivityComponent
mStartSurfaceSupplier.get().getController().setOverviewState(state);
}
if (mOverviewModeController == null || mOverviewModeController.overviewVisible()) return;
if (mOverviewModeController == null) return;
if (mOverviewModeController.overviewVisible()) {
if (didFinishNativeInitialization()) {
getCompositorViewHolder().hideKeyboard(() -> {});
}
return;
}
Tab currentTab = getActivityTab();
// If we don't have a current tab, show the overview mode.
......
......@@ -80,9 +80,11 @@ public class LocationBarCoordinatorPhone implements LocationBarCoordinator.SubCo
* focus state to the other.
*
* @param hasFocus Whether the URL field has gained focus.
* @param shouldShowKeyboard Whether the keyboard should be shown. This value should be the same
* as hasFocus by default.
*/
public void finishUrlFocusChange(boolean hasFocus) {
mLocationBarPhone.finishUrlFocusChange(hasFocus);
public void finishUrlFocusChange(boolean hasFocus, boolean shouldShowKeyboard) {
mLocationBarPhone.finishUrlFocusChange(hasFocus, shouldShowKeyboard);
}
/** Sets whether the url bar should be focusable. */
......
......@@ -965,9 +965,11 @@ public class LocationBarLayout extends FrameLayout implements OnClickListener {
* change. This will be called after any animations are performed to transition from one
* focus state to the other.
* @param hasFocus Whether the URL field has gained focus.
* @param shouldShowKeyboard Whether the keyboard should be shown. This value should be the same
* as hasFocus by default.
*/
protected void finishUrlFocusChange(boolean hasFocus) {
mUrlCoordinator.setKeyboardVisibility(hasFocus, true);
protected void finishUrlFocusChange(boolean hasFocus, boolean shouldShowKeyboard) {
mUrlCoordinator.setKeyboardVisibility(hasFocus && shouldShowKeyboard, true);
setUrlFocusChangeInProgress(false);
updateShouldAnimateIconChanges();
}
......
......@@ -151,8 +151,8 @@ class LocationBarPhone extends LocationBarLayout {
}
@Override
public void finishUrlFocusChange(boolean hasFocus) {
super.finishUrlFocusChange(hasFocus);
public void finishUrlFocusChange(boolean hasFocus, boolean shouldShowKeyboard) {
super.finishUrlFocusChange(hasFocus, shouldShowKeyboard);
if (!hasFocus) {
mUrlActionContainer.setVisibility(GONE);
}
......
......@@ -174,7 +174,7 @@ class LocationBarTablet extends LocationBarLayout {
}
if (mLocationBarDataProvider.getNewTabPageDelegate().isCurrentlyVisible()) {
finishUrlFocusChange(hasFocus);
finishUrlFocusChange(hasFocus, /* shouldShowKeyboard= */ hasFocus);
return;
}
......@@ -189,7 +189,7 @@ class LocationBarTablet extends LocationBarLayout {
mUrlFocusChangeAnimator.addListener(new CancelAwareAnimatorListener() {
@Override
public void onEnd(Animator animator) {
finishUrlFocusChange(hasFocus);
finishUrlFocusChange(hasFocus, /* shouldShowKeyboard= */ hasFocus);
}
@Override
......
......@@ -14,6 +14,7 @@ import org.chromium.base.ApplicationStatus;
import org.chromium.base.ContextUtils;
import org.chromium.base.Log;
import org.chromium.base.StrictModeContext;
import org.chromium.base.library_loader.LibraryLoader;
import org.chromium.base.metrics.RecordHistogram;
import org.chromium.base.metrics.RecordUserAction;
import org.chromium.chrome.browser.app.ChromeActivity;
......@@ -28,6 +29,7 @@ import org.chromium.chrome.browser.tabmodel.TabModelSelector;
import org.chromium.chrome.browser.tasks.pseudotab.PseudoTab;
import org.chromium.chrome.browser.util.ChromeAccessibilityUtil;
import org.chromium.chrome.features.start_surface.StartSurfaceConfiguration;
import org.chromium.components.embedder_support.util.UrlUtilities;
import org.chromium.content_public.browser.LoadUrlParams;
import org.chromium.content_public.common.ResourceRequestBody;
import org.chromium.ui.base.DeviceFormFactor;
......@@ -166,7 +168,7 @@ public final class ReturnToChromeExperimentsUtil {
public static boolean willHandleLoadUrlWithPostDataFromStartSurface(String url,
@PageTransition int transition, @Nullable String postDataType,
@Nullable byte[] postData, @Nullable Boolean incognito, @Nullable Tab parentTab) {
ChromeActivity chromeActivity = getActivityPresentingOverviewWithOmnibox();
ChromeActivity chromeActivity = getActivityPresentingOverviewWithOmnibox(url);
if (chromeActivity == null) return false;
// Create a new unparented tab.
......@@ -202,23 +204,19 @@ public final class ReturnToChromeExperimentsUtil {
}
/**
* @return Whether the Tab Switcher is showing the omnibox.
*/
public static boolean isInOverviewWithOmnibox() {
return getActivityPresentingOverviewWithOmnibox() != null;
}
/**
* @param url The URL to load.
* @return The ChromeActivity if it is presenting the omnibox on the tab switcher, else null.
*/
private static ChromeActivity getActivityPresentingOverviewWithOmnibox() {
private static ChromeActivity getActivityPresentingOverviewWithOmnibox(String url) {
if (!StartSurfaceConfiguration.isStartSurfaceEnabled()) return null;
Activity activity = ApplicationStatus.getLastTrackedFocusedActivity();
if (!(activity instanceof ChromeActivity)) return null;
ChromeActivity chromeActivity = (ChromeActivity) activity;
if (!chromeActivity.isInOverviewMode()) return null;
assert LibraryLoader.getInstance().isInitialized();
if (!chromeActivity.isInOverviewMode() && !UrlUtilities.isNTPUrl(url)) return null;
return chromeActivity;
}
......
......@@ -472,12 +472,14 @@ public class ToolbarPhone extends ToolbarLayout implements OnClickListener, TabC
/**
* Calls the {@link triggerUrlFocusAnimation()} here if it is skipped in {@link
* handleOmniboxInOverviewMode()}. See https://crbug.com/1152306.
* handleOmniboxInOverviewMode()}. See https://crbug.com/1152306. Keyboard shouldn't be
* shown here.
*/
if (mPendingTriggerUrlFocusRequest) {
mPendingTriggerUrlFocusRequest = false;
triggerUrlFocusAnimation(
getToolbarDataProvider().isInOverviewAndShowingOmnibox() && !urlHasFocus());
getToolbarDataProvider().isInOverviewAndShowingOmnibox() && !urlHasFocus(),
/* shouldShowKeyboard= */ false);
}
updateVisualsForLocationBarState();
......@@ -1860,9 +1862,10 @@ public class ToolbarPhone extends ToolbarLayout implements OnClickListener, TabC
// The URL focusing animator set shouldn't be populated before native initialization. It is
// possible that this function is called before native initialization when Instant Start
// is enabled.
// is enabled. Keyboard shouldn't be shown here.
if (isNativeLibraryReady()) {
triggerUrlFocusAnimation(inTabSwitcherMode && !urlHasFocus());
triggerUrlFocusAnimation(
inTabSwitcherMode && !urlHasFocus(), /* shouldShowKeyboard= */ false);
} else {
mPendingTriggerUrlFocusRequest = true;
}
......@@ -2085,10 +2088,15 @@ public class ToolbarPhone extends ToolbarLayout implements OnClickListener, TabC
if (!hasFocus) return;
}
triggerUrlFocusAnimation(hasFocus);
triggerUrlFocusAnimation(hasFocus, /* shouldShowKeyboard= */ hasFocus);
}
private void triggerUrlFocusAnimation(final boolean hasFocus) {
/**
* @param hasFocus Whether the URL field has gained focus.
* @param shouldShowKeyboard Whether the keyboard should be shown. This value should be the same
* as hasFocus by default.
*/
private void triggerUrlFocusAnimation(final boolean hasFocus, boolean shouldShowKeyboard) {
TraceEvent.begin("ToolbarPhone.triggerUrlFocusAnimation");
if (mUrlFocusLayoutAnimator != null && mUrlFocusLayoutAnimator.isRunning()) {
mUrlFocusLayoutAnimator.cancel();
......@@ -2131,7 +2139,8 @@ public class ToolbarPhone extends ToolbarLayout implements OnClickListener, TabC
mLayoutLocationBarInFocusedMode = false;
requestLayout();
}
mLocationBar.getPhoneCoordinator().finishUrlFocusChange(hasFocus);
mLocationBar.getPhoneCoordinator().finishUrlFocusChange(
hasFocus, shouldShowKeyboard);
mUrlFocusChangeInProgress = false;
if (getToolbarDataProvider().shouldShowLocationBarInOverviewMode()) {
......@@ -2284,7 +2293,7 @@ public class ToolbarPhone extends ToolbarLayout implements OnClickListener, TabC
if (mTabSwitcherState == STATIC_TAB && previousNtpScrollFraction > 0f) {
mUrlFocusChangeFraction =
Math.max(previousNtpScrollFraction, mUrlFocusChangeFraction);
triggerUrlFocusAnimation(false);
triggerUrlFocusAnimation(/* hasFocus= */ false, /* shouldShowKeyboard= */ false);
}
requestLayout();
}
......
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