Commit 78f52b84 authored by Haiyang Pan's avatar Haiyang Pan Committed by Commit Bot

Revert "Reland "Reland "[Omnibox] Use ActivityState to check if we are in browser mode"""

This reverts commit 5d52e576.

Reason for revert: org.chromium.chrome.browser.omnibox.suggestions.SwitchToTabTest#testSwitchToTabSuggestion flakily fails with java.lang.NullPointerException
For example:
https://ci.chromium.org/p/chromium/builders/ci/android-pie-x86-rel/1524
https://ci.chromium.org/p/chromium/builders/ci/android-pie-x86-rel/1525
https://ci.chromium.org/p/chromium/builders/ci/android-marshmallow-x86-fyi-rel/1272

Also, it is flaky in android-pie-arm64-rel. See https://analysis.chromium.org/p/chromium/flake-portal/flakes/occurrences?key=ag9zfmZpbmRpdC1mb3ItbWVyhAELEgVGbGFrZSJ5Y2hyb21pdW1AY2hyb21lX3B1YmxpY190ZXN0X2Fwa0BvcmcuY2hyb21pdW0uY2hyb21lLmJyb3dzZXIub21uaWJveC5zdWdnZXN0aW9ucy5Td2l0Y2hUb1RhYlRlc3QjdGVzdFN3aXRjaFRvVGFiU3VnZ2VzdGlvbgw

Original change's description:
> Reland "Reland "[Omnibox] Use ActivityState to check if we are in browser mode""
> 
> This is a reland of 5ae112d1
> 
> Original change's description:
> > Reland "[Omnibox] Use ActivityState to check if we are in browser mode"
> > 
> > This is a reland of be1e9636
> > 
> > Update the test to run on UI thread.
> > 
> > Original change's description:
> > > [Omnibox] Use ActivityState to check if we are in browser mode
> > >
> > >
> > > In multi windows mode, check if we are in browser mode by WindowAndroid
> > > is not working, should check ActivityState instead.
> > >
> > > Bug: 1102640
> > > Change-Id: I35a720a2aa6f337cc9f9ae43e39d83c9368fcb5a
> > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2286478
> > > Commit-Queue: Gang Wu <gangwu@chromium.org>
> > > Reviewed-by: Ted Choc <tedchoc@chromium.org>
> > > Cr-Commit-Position: refs/heads/master@{#786636}
> > 
> > Bug: 1102640
> > Change-Id: Iefba5a5d0c181535bd7fb86cf0e073be2ebdbcdb
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2293074
> > Reviewed-by: Ted Choc <tedchoc@chromium.org>
> > Commit-Queue: Gang Wu <gangwu@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#788816}
> 
> Bug: 1102640
> Change-Id: I6da3c7ad8402cef83083a53f7ffc517f975720f8
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2301285
> Commit-Queue: Gang Wu <gangwu@chromium.org>
> Reviewed-by: Ted Choc <tedchoc@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#789727}

TBR=tedchoc@chromium.org,gangwu@chromium.org

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

Bug: 1102640
Change-Id: I2227da6e687932a6a90c3eb73f2a5386ba2c6d9f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2306749Reviewed-by: default avatarHaiyang Pan <hypan@google.com>
Commit-Queue: Haiyang Pan <hypan@google.com>
Cr-Commit-Position: refs/heads/master@{#789802}
parent 8a5cd716
......@@ -20,7 +20,6 @@ import androidx.annotation.Px;
import androidx.annotation.StringRes;
import androidx.annotation.VisibleForTesting;
import org.chromium.base.ActivityState;
import org.chromium.base.Callback;
import org.chromium.base.ContextUtils;
import org.chromium.base.IntentUtils;
......@@ -577,10 +576,9 @@ class AutocompleteMediator implements OnSuggestionsReceivedListener, StartStopWi
}
// When invoked directly from a browser, we want to trigger switch to tab animation.
// If invoked from other activities, ex. searchActivity, we do not need to trigger the
// If invoded from other activitiies, ex. searchActivity, we do not need to trigger the
// animation since Android will show the animation for switching apps.
if (tab.getWindowAndroid().getActivityState() != ActivityState.STOPPED
&& tab.getWindowAndroid().getActivityState() != ActivityState.DESTROYED) {
if (mWindowAndroid.equals(tab.getWindowAndroid())) {
// TODO(1097292): Do not use Activity to get TabModelSelector.
assert tab.getWindowAndroid().getActivity().get() instanceof ChromeActivity;
......@@ -591,13 +589,13 @@ class AutocompleteMediator implements OnSuggestionsReceivedListener, StartStopWi
chromeActivity.getTabModelSelector().getCurrentModel().setIndex(
tabIndex, TabSelectionType.FROM_OMNIBOX);
} else {
// Browser is in background, bring to to foreground and switch to the tab.
Intent newIntent = ChromeIntentUtil.createBringTabToFrontIntent(tab.getId());
if (newIntent != null) {
newIntent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
IntentUtils.safeStartActivity(ContextUtils.getApplicationContext(), newIntent);
}
}
recordMetrics(position, WindowOpenDisposition.SWITCH_TO_TAB, suggestion);
}
......
......@@ -8,11 +8,11 @@ import android.app.Activity;
import android.app.Instrumentation;
import android.app.Instrumentation.ActivityMonitor;
import android.content.Intent;
import android.os.Build;
import android.support.test.InstrumentationRegistry;
import android.text.TextUtils;
import android.util.Pair;
import android.view.ViewGroup;
import android.widget.ImageView;
import android.widget.TextView;
import androidx.test.filters.MediumTest;
......@@ -24,9 +24,8 @@ import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.chromium.base.ActivityState;
import org.chromium.base.ThreadUtils;
import org.chromium.base.test.util.CommandLineFlags;
import org.chromium.base.test.util.DisableIf;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.ChromeActivity;
import org.chromium.chrome.browser.flags.ChromeSwitches;
......@@ -43,7 +42,6 @@ import org.chromium.chrome.test.util.WaitForFocusHelper;
import org.chromium.chrome.test.util.browser.Features.EnableFeatures;
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.CriteriaNotSatisfiedException;
import org.chromium.content_public.browser.test.util.TestThreadUtils;
import org.chromium.content_public.browser.test.util.TestTouchUtils;
import org.chromium.net.test.EmbeddedTestServer;
......@@ -92,105 +90,46 @@ public class SwitchToTabTest {
}
/**
* Type the |text| into |activity|'s URL bar, and wait for switch to tab suggestion shows up.
* Type the |text| into |activity|'s url_bar, and wait for switch to tab suggestion shows up.
*
* @param activity The Activity which URL bar is in.
* @param activity The Activity which url_bar is in.
* @param locationBarLayout The layout which omnibox suggestions will show in.
* @param tab The tab will be switched to.
* @param text The text will be typed into url_bar.
*/
private void typeAndClickMatchingTabMatchSuggestion(Activity activity,
LocationBarLayout locationBarLayout, Tab tab) throws InterruptedException {
typeInOmnibox(activity, tab.getTitle());
private void typeAndWaitForTabMatchSuggestions(Activity activity,
LocationBarLayout locationBarLayout, String input) throws InterruptedException {
typeInOmnibox(activity, input);
OmniboxTestUtils.waitForOmniboxSuggestions(locationBarLayout);
// waitForOmniboxSuggestions only wait until one suggestion shows up, we need to wait util
// autocomplete return more suggestions.
CriteriaHelper.pollUiThread(() -> {
OmniboxSuggestion matchSuggestion =
findTabMatchOmniboxSuggestion(locationBarLayout, tab);
Criteria.checkThat(matchSuggestion, Matchers.notNullValue());
OmniboxSuggestionsDropdown suggestionsDropdown =
AutocompleteCoordinatorTestUtils.getSuggestionsDropdown(
locationBarLayout.getAutocompleteCoordinator());
// Make sure data populated to UI
int index = findIndexOfTabMatchSuggestionView(suggestionsDropdown, matchSuggestion);
Criteria.checkThat(index, Matchers.not(INVALID_INDEX));
try {
clickSuggestionActionAt(suggestionsDropdown, index);
} catch (InterruptedException e) {
throw new CriteriaNotSatisfiedException(e);
}
Criteria.checkThat(findFirstTabMatchOmniboxSuggestion(locationBarLayout).first,
Matchers.not(INVALID_INDEX));
});
}
/**
* Find the switch to tab suggestion which suggests the |tab|, and return the
* suggestion. This method needs to run on the UI thread.
* Find the first switch to tab suggestion in the omnibox suggestion list, and return the
* suggestion and its index.
*
* @param locationBarLayout The layout which omnibox suggestions will show in.
* @param tab The tab which the OmniboxSuggestion should suggest.
* @return The suggesstion which suggests the |tab|.
*/
private OmniboxSuggestion findTabMatchOmniboxSuggestion(
LocationBarLayout locationBarLayout, Tab tab) {
ThreadUtils.assertOnUiThread();
AutocompleteCoordinator coordinator = locationBarLayout.getAutocompleteCoordinator();
// Find the first matching suggestion.
for (int i = 0; i < coordinator.getSuggestionCount(); ++i) {
OmniboxSuggestion suggestion = coordinator.getSuggestionAt(i);
if (suggestion != null && suggestion.hasTabMatch()
&& TextUtils.equals(suggestion.getDescription(), tab.getTitle())
&& TextUtils.equals(suggestion.getUrl().getSpec(), tab.getUrl().getSpec())) {
return suggestion;
}
}
return null;
}
/**
* Find the index of the tab match suggestion in OmniboxSuggestionsDropdown. This method needs
* to run on the UI thread.
*
* @param suggestionsDropdown The OmniboxSuggestionsDropdown contains all the suggestions.
* @param suggestion The OmniboxSuggestion we are looking for in the view.
* @return The matching suggestion's index.
* @return The the first switch to tab suggestion's index, and the suggesstion.
*/
private int findIndexOfTabMatchSuggestionView(
OmniboxSuggestionsDropdown suggestionsDropdown, OmniboxSuggestion suggestion) {
ThreadUtils.assertOnUiThread();
ViewGroup viewGroup = suggestionsDropdown.getViewGroup();
if (viewGroup == null) {
return INVALID_INDEX;
}
for (int i = 0; i < viewGroup.getChildCount(); i++) {
BaseSuggestionView baseSuggestionView = (BaseSuggestionView) viewGroup.getChildAt(i);
if (baseSuggestionView == null) {
continue;
}
TextView line1 = baseSuggestionView.findViewById(R.id.line_1);
TextView line2 = baseSuggestionView.findViewById(R.id.line_2);
if (!TextUtils.equals(suggestion.getDescription(), line1.getText())
|| !TextUtils.equals(suggestion.getDisplayText(), line2.getText())) {
continue;
}
List<ImageView> buttonsList = baseSuggestionView.getActionButtons();
if (buttonsList != null && buttonsList.size() == 1
&& TextUtils.equals(baseSuggestionView.getResources().getString(
R.string.accessibility_omnibox_switch_to_tab),
buttonsList.get(0).getContentDescription())) {
return i;
private Pair<Integer, OmniboxSuggestion> findFirstTabMatchOmniboxSuggestion(
LocationBarLayout locationBarLayout) {
OmniboxSuggestionsDropdown suggestionsDropdown =
AutocompleteCoordinatorTestUtils.getSuggestionsDropdown(
locationBarLayout.getAutocompleteCoordinator());
// Find the index of the first matching suggestion.
for (int i = 0; i < suggestionsDropdown.getItemCount(); ++i) {
OmniboxSuggestion suggestion = AutocompleteCoordinatorTestUtils.getOmniboxSuggestionAt(
locationBarLayout.getAutocompleteCoordinator(), i);
if (suggestion != null && suggestion.hasTabMatch()) {
return new Pair<Integer, OmniboxSuggestion>(i, suggestion);
}
}
return INVALID_INDEX;
return new Pair<Integer, OmniboxSuggestion>(INVALID_INDEX, null);
}
/**
......@@ -201,12 +140,10 @@ public class SwitchToTabTest {
*/
private void clickSuggestionActionAt(OmniboxSuggestionsDropdown suggestionsDropdown, int index)
throws InterruptedException {
// Wait a bit since the button may not able to click.
ViewGroup viewGroup = suggestionsDropdown.getViewGroup();
BaseSuggestionView baseSuggestionView = (BaseSuggestionView) viewGroup.getChildAt(index);
Assert.assertNotNull("Null suggestion for index: " + index, baseSuggestionView);
List<ImageView> buttonsList = baseSuggestionView.getActionButtons();
Assert.assertNotNull(buttonsList);
Assert.assertEquals(buttonsList.size(), 1);
TestTouchUtils.performClickOnMainSync(
InstrumentationRegistry.getInstrumentation(), buttonsList.get(0));
......@@ -234,6 +171,9 @@ public class SwitchToTabTest {
@Test
@MediumTest
@DisableIf.Build(message = "https://crbug.com/1101433",
sdk_is_greater_than = Build.VERSION_CODES.LOLLIPOP_MR1,
sdk_is_less_than = Build.VERSION_CODES.N)
@EnableFeatures("OmniboxTabSwitchSuggestions")
public void
testSwitchToTabSuggestion() throws InterruptedException {
......@@ -243,23 +183,34 @@ public class SwitchToTabTest {
final String testHttpsUrl1 = mTestServer.getURL("/chrome/test/data/android/about.html");
final String testHttpsUrl2 = mTestServer.getURL("/chrome/test/data/android/ok.txt");
final String testHttpsUrl3 = mTestServer.getURL("/chrome/test/data/android/test.html");
final Tab aboutTab = mActivityTestRule.loadUrlInNewTab(testHttpsUrl1);
mActivityTestRule.loadUrlInNewTab(testHttpsUrl1);
mActivityTestRule.loadUrlInNewTab(testHttpsUrl2);
mActivityTestRule.loadUrlInNewTab(testHttpsUrl3);
LocationBarLayout locationBarLayout =
(LocationBarLayout) mActivityTestRule.getActivity().findViewById(R.id.location_bar);
typeAndClickMatchingTabMatchSuggestion(
mActivityTestRule.getActivity(), locationBarLayout, aboutTab);
typeAndWaitForTabMatchSuggestions(
mActivityTestRule.getActivity(), locationBarLayout, "about");
Pair<Integer, OmniboxSuggestion> matchSuggestion =
findFirstTabMatchOmniboxSuggestion(locationBarLayout);
Assert.assertNotEquals(INVALID_INDEX, (int) matchSuggestion.first);
Assert.assertNotNull("No Switch to Tab suggestion returned.", matchSuggestion.second);
Assert.assertEquals(matchSuggestion.second.getUrl().getSpec(), testHttpsUrl1);
OmniboxSuggestionsDropdown suggestionsDropdown =
AutocompleteCoordinatorTestUtils.getSuggestionsDropdown(
locationBarLayout.getAutocompleteCoordinator());
clickSuggestionActionAt(suggestionsDropdown, (int) matchSuggestion.first);
CriteriaHelper.pollUiThread(() -> {
Tab tab = mActivityTestRule.getActivity().getActivityTab();
Criteria.checkThat(tab, Matchers.notNullValue());
Criteria.checkThat(tab, Matchers.is(aboutTab));
if (tab == null) return false;
// Make sure tab is in either upload page or result page. cannot only verify one of
// them since on fast device tab jump to result page really quick but on slow device
// may stay on upload page for a really long time.
Criteria.checkThat(tab.getUrlString(), Matchers.is(testHttpsUrl1));
return tab.getUrlString().equals(testHttpsUrl1);
});
}
......@@ -274,7 +225,7 @@ public class SwitchToTabTest {
final String testHttpsUrl2 = mTestServer.getURL("/chrome/test/data/android/ok.txt");
final String testHttpsUrl3 = mTestServer.getURL("/chrome/test/data/android/test.html");
// Open the url trying to match in incognito mode.
final Tab aboutTab = mActivityTestRule.loadUrlInNewTab(testHttpsUrl1, true);
mActivityTestRule.loadUrlInNewTab(testHttpsUrl1, true);
mActivityTestRule.loadUrlInNewTab(testHttpsUrl2);
mActivityTestRule.loadUrlInNewTab(testHttpsUrl3);
......@@ -284,51 +235,52 @@ public class SwitchToTabTest {
mActivityTestRule.typeInOmnibox("about", false);
OmniboxTestUtils.waitForOmniboxSuggestions(locationBarLayout);
CriteriaHelper.pollUiThread(() -> {
OmniboxSuggestion matchSuggestion =
findTabMatchOmniboxSuggestion(locationBarLayout, aboutTab);
Criteria.checkThat(matchSuggestion, Matchers.nullValue());
});
Pair<Integer, OmniboxSuggestion> matchSuggestion =
findFirstTabMatchOmniboxSuggestion(locationBarLayout);
Assert.assertNull(
"Should no Switch to Incognito Tab from normal tab.", matchSuggestion.second);
}
@Test
@MediumTest
@EnableFeatures("OmniboxTabSwitchSuggestions")
public void testSwitchToTabInSearchActivity() throws InterruptedException {
public void testSwitchToTabInSearchActiviy() throws InterruptedException {
mTestServer = EmbeddedTestServer.createAndStartHTTPSServer(
InstrumentationRegistry.getInstrumentation().getContext(),
ServerCertificate.CERT_OK);
final String testHttpsUrl1 = mTestServer.getURL("/chrome/test/data/android/about.html");
final String testHttpsUrl2 = mTestServer.getURL("/chrome/test/data/android/ok.txt");
final String testHttpsUrl3 = mTestServer.getURL("/chrome/test/data/android/test.html");
final Tab aboutTab = mActivityTestRule.loadUrlInNewTab(testHttpsUrl1);
mActivityTestRule.loadUrlInNewTab(testHttpsUrl1);
mActivityTestRule.loadUrlInNewTab(testHttpsUrl2);
mActivityTestRule.loadUrlInNewTab(testHttpsUrl3);
Assert.assertNotEquals(mActivityTestRule.getActivity().getActivityTab(), aboutTab);
final SearchActivity searchActivity = startSearchActivity();
CriteriaHelper.pollUiThread(() -> {
Tab tab = mActivityTestRule.getActivity().getActivityTab();
Criteria.checkThat(tab, Matchers.notNullValue());
// Make sure chrome fully in background.
Criteria.checkThat(tab.getWindowAndroid().getActivityState(),
Matchers.isOneOf(ActivityState.STOPPED, ActivityState.DESTROYED));
});
final LocationBarLayout locationBarLayout =
(LocationBarLayout) searchActivity.findViewById(R.id.search_location_bar);
typeAndClickMatchingTabMatchSuggestion(searchActivity, locationBarLayout, aboutTab);
typeAndWaitForTabMatchSuggestions(searchActivity, locationBarLayout, "about");
Pair<Integer, OmniboxSuggestion> matchSuggestion =
findFirstTabMatchOmniboxSuggestion(locationBarLayout);
Assert.assertNotEquals(INVALID_INDEX, (int) matchSuggestion.first);
Assert.assertNotNull("No Switch to Tab suggestion returned.", matchSuggestion.second);
Assert.assertEquals(matchSuggestion.second.getUrl().getSpec(), testHttpsUrl1);
OmniboxSuggestionsDropdown suggestionsDropdown =
AutocompleteCoordinatorTestUtils.getSuggestionsDropdown(
locationBarLayout.getAutocompleteCoordinator());
clickSuggestionActionAt(suggestionsDropdown, (int) matchSuggestion.first);
CriteriaHelper.pollUiThread(() -> {
Tab tab = mActivityTestRule.getActivity().getActivityTab();
Criteria.checkThat(tab, Matchers.notNullValue());
Criteria.checkThat(tab, Matchers.is(aboutTab));
Criteria.checkThat(tab.getUrlString(), Matchers.is(testHttpsUrl1));
// Make sure tab is loaded and in foreground.
Criteria.checkThat(
tab.getWindowAndroid().getActivityState(), Matchers.is(ActivityState.RESUMED));
Assert.assertEquals(tab, aboutTab);
if (tab == null) return false;
// Make sure tab is in either upload page or result page. cannot only verify one of
// them since on fast device tab jump to result page really quick but on slow device
// may stay on upload page for a really long time.
return tab.getUrlString().equals(testHttpsUrl1);
});
}
}
......@@ -203,7 +203,7 @@ public class TestTouchUtils {
* @param v The view to call performClick on.
*/
public static void performClickOnMainSync(Instrumentation instrumentation, final View v) {
TestThreadUtils.runOnUiThreadBlocking(() -> { v.performClick(); });
instrumentation.runOnMainSync(() -> v.performClick());
}
/**
......
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