Skip to content

GitLab

T
tangled
Commit 86c32d98 authored by Scott Violet's avatar Scott Violet Committed by Commit Bot
Browse files
Options

weblayer: forces top-view shown during load 

Prior to this patch, if the top-view was configured to only expand
at top, then on page load the top-view would not be shown.

During a load if current_browser_controls_visibility_constraint_ is BOTH,
then cc is called with current and constraints of BOTH. cc doesn't do
anything in this case. As we need to ensure the top-view is shown in
this scenario this changes the code to supply SHOWN.

This code path is only hit if top-view is configured to
only-expand-at-top, as when only-expand-at-top is false,
current_browser_controls_visibility_constraint_ is set to SHOWN.

I experimented with fixing in cc, but cc has no notion of page
loading. So, this fix is better done in WebLayer.

BUG=1135964
TEST=BrowserControlsTest

Change-Id: I7834ade5cd80de6946af0c2f917fb0ea1996e1f0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2476725
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: default avatarBo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#817985}
parent c5f95272
Hide whitespace changes
Inline Side-by-side
Showing with 122 additions and 7 deletions
weblayer/browser/android/javatests/src/org/chromium/weblayer/test/BrowserControlsHelper.java
View file @ 86c32d98
......@@ -32,7 +32,7 @@ public final class BrowserControlsHelper {
// Blocks until browser controls are fully initialized. Should only be created in a test's
// setUp() method; see BrowserControlsHelper#createInSetUp().
private BrowserControlsHelper(InstrumentationActivity activity) throws Throwable {
private BrowserControlsHelper(InstrumentationActivity activity) throws Exception {
Assert.assertTrue(CommandLine.isInitialized());
Assert.assertTrue(CommandLine.getInstance().hasSwitch("enable-features"));
String enabledFeatures = CommandLine.getInstance().getSwitchValue("enable-features");
......@@ -73,7 +73,7 @@ public final class BrowserControlsHelper {
}
// Ensures that browser controls are fully initialized and ready for scrolls to be processed.
private void waitForBrowserControlsInitialization() throws Throwable {
private void waitForBrowserControlsInitialization() throws Exception {
// Poll until the top view becomes visible.
waitForBrowserControlsViewToBeVisible(mActivity.getTopContentsContainer());
TestThreadUtils.runOnUiThreadBlocking(() -> {
......@@ -88,7 +88,7 @@ public final class BrowserControlsHelper {
// Creates a BrowserControlsHelper instance and blocks until browser controls are fully
// initialized. Should be called from a test's setUp() method.
static BrowserControlsHelper createAndBlockUntilBrowserControlsInitializedInSetUp(
InstrumentationActivity activity) throws Throwable {
InstrumentationActivity activity) throws Exception {
return new BrowserControlsHelper(activity);
}
......
weblayer/browser/android/javatests/src/org/chromium/weblayer/test/BrowserControlsTest.java
View file @ 86c32d98
......@@ -10,6 +10,7 @@ import static androidx.test.espresso.matcher.ViewMatchers.isDisplayed;
import static androidx.test.espresso.matcher.ViewMatchers.withText;
import android.os.Build;
import android.os.Bundle;
import android.os.RemoteException;
import android.view.View;
import android.view.ViewGroup;
......@@ -20,16 +21,17 @@ import androidx.test.filters.SmallTest;
import org.hamcrest.Matchers;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.chromium.base.test.util.CallbackHelper;
import org.chromium.base.test.util.CommandLineFlags;
import org.chromium.base.test.util.MinAndroidSdkLevel;
import org.chromium.content_public.browser.test.util.Criteria;
import org.chromium.content_public.browser.test.util.CriteriaHelper;
import org.chromium.content_public.browser.test.util.TestThreadUtils;
import org.chromium.weblayer.BrowserControlsOffsetCallback;
import org.chromium.weblayer.Tab;
import org.chromium.weblayer.TestWebLayer;
import org.chromium.weblayer.shell.InstrumentationActivity;
......@@ -88,8 +90,7 @@ public class BrowserControlsTest {
});
}
@Before
public void setUp() throws Throwable {
private void createActivityWithTopView() throws Exception {
final String url = mActivityTestRule.getTestDataURL("tall_page.html");
InstrumentationActivity activity = mActivityTestRule.launchShellWithUrl(url);
......@@ -106,6 +107,7 @@ public class BrowserControlsTest {
@Test
@SmallTest
public void testTopAndBottom() throws Exception {
createActivityWithTopView();
InstrumentationActivity activity = mActivityTestRule.getActivity();
View bottomView = TestThreadUtils.runOnUiThreadBlocking(() -> {
TextView view = new TextView(activity);
......@@ -164,6 +166,7 @@ public class BrowserControlsTest {
@Test
@SmallTest
public void testBottomOnly() throws Exception {
createActivityWithTopView();
InstrumentationActivity activity = mActivityTestRule.getActivity();
// Remove the top-view.
TestThreadUtils.runOnUiThreadBlocking(() -> { activity.getBrowser().setTopView(null); });
......@@ -219,6 +222,7 @@ public class BrowserControlsTest {
@Test
@SmallTest
public void testTopOnly() throws Exception {
createActivityWithTopView();
InstrumentationActivity activity = mActivityTestRule.getActivity();
View topView = activity.getTopContentsContainer();
......@@ -249,6 +253,7 @@ public class BrowserControlsTest {
@Test
@SmallTest
public void testTopMinHeight() throws Exception {
createActivityWithTopView();
final int minHeight = 20;
InstrumentationActivity activity = mActivityTestRule.getActivity();
View topContents = activity.getTopContentsContainer();
......@@ -288,6 +293,7 @@ public class BrowserControlsTest {
@Test
@SmallTest
public void testOnlyExpandTopControlsAtPageTop() throws Exception {
createActivityWithTopView();
InstrumentationActivity activity = mActivityTestRule.getActivity();
View topContents = activity.getTopContentsContainer();
TestThreadUtils.runOnUiThreadBlocking(
......@@ -333,6 +339,7 @@ public class BrowserControlsTest {
@Test
@SmallTest
public void testAlertDoesntShowTopControlsIfOnlyExpandTopControlsAtPageTop() throws Exception {
createActivityWithTopView();
InstrumentationActivity activity = mActivityTestRule.getActivity();
View topContents = activity.getTopContentsContainer();
TestThreadUtils.runOnUiThreadBlocking(
......@@ -371,6 +378,7 @@ public class BrowserControlsTest {
@Test
@SmallTest
public void testAlertShowsTopControls() throws Exception {
createActivityWithTopView();
InstrumentationActivity activity = mActivityTestRule.getActivity();
// Move by the size of the top-controls.
......@@ -400,6 +408,7 @@ public class BrowserControlsTest {
@Test
@SmallTest
public void testAccessibility() throws Exception {
createActivityWithTopView();
InstrumentationActivity activity = mActivityTestRule.getActivity();
// Scroll such that top-controls are hidden.
......@@ -434,6 +443,7 @@ public class BrowserControlsTest {
@Test
@SmallTest
public void testRemoveAllFromTopView() throws Exception {
createActivityWithTopView();
InstrumentationActivity activity = mActivityTestRule.getActivity();
// Install a different top-view.
......@@ -465,4 +475,64 @@ public class BrowserControlsTest {
Criteria.checkThat(getVisiblePageHeight(), Matchers.not(mPageHeightWithTopView));
});
}
private void registerBrowserControlsOffsetCallbackForOffset(
CallbackHelper helper, int targetOffset) {
InstrumentationActivity activity = mActivityTestRule.getActivity();
TestThreadUtils.runOnUiThreadBlocking(() -> {
activity.getBrowser().registerBrowserControlsOffsetCallback(
new BrowserControlsOffsetCallback() {
@Override
public void onTopViewOffsetChanged(int offset) {
if (offset == targetOffset) {
activity.getBrowser().unregisterBrowserControlsOffsetCallback(this);
helper.notifyCalled();
}
}
});
});
}
@MinAndroidSdkLevel(Build.VERSION_CODES.M)
@Test
@SmallTest
public void testTopExpandedWhenOnlyExpandAtTop() throws Exception {
Bundle extras = new Bundle();
extras.putBoolean(InstrumentationActivity.EXTRA_ONLY_EXPAND_CONTROLS_AT_TOP, true);
final String url = mActivityTestRule.getTestDataURL("tall_page.html");
InstrumentationActivity activity = mActivityTestRule.launchShell(extras);
CallbackHelper helper = new CallbackHelper();
registerBrowserControlsOffsetCallbackForOffset(helper, 0);
mActivityTestRule.navigateAndWait(url);
helper.waitForFirst();
}
@MinAndroidSdkLevel(Build.VERSION_CODES.M)
@Test
@SmallTest
public void testTopExpandedOnLoadWhenOnlyExpandAtTop() throws Exception {
Bundle extras = new Bundle();
extras.putBoolean(InstrumentationActivity.EXTRA_ONLY_EXPAND_CONTROLS_AT_TOP, true);
InstrumentationActivity activity = mActivityTestRule.launchShell(extras);
CallbackHelper helper = new CallbackHelper();
registerBrowserControlsOffsetCallbackForOffset(helper, 0);
mActivityTestRule.navigateAndWait(mActivityTestRule.getTestDataURL("tall_page.html"));
int callCount = 0;
helper.waitForCallback(callCount++);
mTopViewHeight = TestThreadUtils.runOnUiThreadBlocking(
() -> { return activity.getTopContentsContainer().getHeight(); });
Assert.assertNotEquals(mTopViewHeight, 0);
// Scroll such that top-controls are hidden.
registerBrowserControlsOffsetCallbackForOffset(helper, -mTopViewHeight);
EventUtils.simulateDragFromCenterOfView(
activity.getWindow().getDecorView(), 0, -mTopViewHeight);
helper.waitForCallback(callCount++);
// Load a new page. The top-controls should be shown again.
registerBrowserControlsOffsetCallbackForOffset(helper, 0);
mActivityTestRule.navigateAndWait(mActivityTestRule.getTestDataURL("simple_page.html"));
helper.waitForCallback(callCount++);
}
}
weblayer/browser/controls_visibility_reason.h
View file @ 86c32d98
......@@ -10,6 +10,10 @@ namespace weblayer {
// This enum represents actions or UI conditions that affect the visibility of
// top UI, and is used to track concurrent concerns and to allow native and Java
// code to coordinate.
//
// WARNING: only a subset of these are used if OnlyExpandTopControlsAtPageTop
// is true.
//
// GENERATED_JAVA_ENUM_PACKAGE: org.chromium.weblayer_private
// GENERATED_JAVA_CLASS_NAME_OVERRIDE: ImplControlsVisibilityReason
enum class ControlsVisibilityReason {
......
weblayer/browser/tab_impl.cc
View file @ 86c32d98
......@@ -1194,6 +1194,30 @@ void TabImpl::OnUpdateBrowserControlsStateBecauseOfProcessSwitch(
// bounce around.
UpdateBrowserControlsState(content::BROWSER_CONTROLS_STATE_SHOWN, false);
} else {
if (did_commit && current_browser_controls_visibility_constraint_ ==
content::BROWSER_CONTROLS_STATE_BOTH) {
// If the current state is BROWSER_CONTROLS_STATE_BOTH, then
// TabImpl::UpdateBrowserControlsState() is going to call
// WebContents::UpdateBrowserControlsState() with both current and
// constraints set to BROWSER_CONTROLS_STATE_BOTH. cc does
// nothing in this case. During a navigation the top-view needs to be
// shown. To force the top-view to show, supply
// BROWSER_CONTROLS_STATE_SHOWN. This path is only hit if top-view
// is configured to only-expand-at-top, as in this case the top-view isn't
// forced shown during a page load.
//
// It's entirely possible the scroll offset is changed as part of the
// loading process (such as happens with back/forward navigation or
// links part way down a page). Trying to detect this and compensate
// here is likely to be racy, so the top-view is always shown.
const bool animate =
!base::FeatureList::IsEnabled(kImmediatelyHideBrowserControlsForTest);
web_contents_->GetMainFrame()->UpdateBrowserControlsState(
content::BROWSER_CONTROLS_STATE_BOTH,
content::BROWSER_CONTROLS_STATE_SHOWN, animate);
// This falls through to call UpdateBrowserControlsState() again to
// ensure the constraint is set back to BOTH.
}
UpdateBrowserControlsState(
content::BROWSER_CONTROLS_STATE_BOTH,
current_browser_controls_visibility_constraint_ !=
......
weblayer/shell/android/shell_apk/src/org/chromium/weblayer/shell/InstrumentationActivity.java
View file @ 86c32d98
......@@ -43,6 +43,8 @@ import java.util.List;
/**
* Activity for running instrumentation tests.
*/
// This isn't part of Chrome, so using explicit colors/sizes is ok.
@SuppressWarnings("checkstyle:SetTextColorAndSetTextSizeCheck")
public class InstrumentationActivity extends FragmentActivity {
private static final String TAG = "WLInstrumentation";
private static final String KEY_MAIN_VIEW_ID = "mainViewId";
......@@ -55,6 +57,10 @@ public class InstrumentationActivity extends FragmentActivity {
// True by default. If set to false, the test should call loadWebLayerSync.
public static final String EXTRA_CREATE_WEBLAYER = "EXTRA_CREATE_WEBLAYER";
public static final String EXTRA_TOP_VIEW_MIN_HEIGHT = "EXTRA_TOP_VIEW_MIN_HEIGHT";
public static final String EXTRA_ONLY_EXPAND_CONTROLS_AT_TOP =
"EXTRA_ONLY_EXPAND_CONTROLS_AT_TOP";
// Used in tests to specify whether WebLayer URL bar should set default click listeners
// that show Page Info UI on its TextView.
public static final String EXTRA_URLBAR_TEXT_CLICKABLE = "EXTRA_URLBAR_TEXT_CLICKABLE";
......@@ -266,7 +272,18 @@ public class InstrumentationActivity extends FragmentActivity {
mBrowser = Browser.fromFragment(mFragment);
mProfile = mBrowser.getProfile();
mBrowser.setTopView(mTopContentsContainer);
final boolean onlyExpandControlsAtTop =
getIntent().getBooleanExtra(EXTRA_ONLY_EXPAND_CONTROLS_AT_TOP, false);
final int minTopViewHeight = getIntent().getIntExtra(EXTRA_TOP_VIEW_MIN_HEIGHT, -1);
if (onlyExpandControlsAtTop || minTopViewHeight != -1) {
// This was added in 86.
mBrowser.setTopView(mTopContentsContainer, Math.max(0, minTopViewHeight),
onlyExpandControlsAtTop,
/* animate */ false);
} else {
mBrowser.setTopView(mTopContentsContainer);
}
mRendererCrashListener = new TabCallback() {
@Override
......
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