Commit ee00d9b5 authored by Boris Sazonov's avatar Boris Sazonov Committed by Commit Bot

Revert "[ToolbarMVC] Observe incognito state changes in location bar."

This reverts commit 7c26f582.

Reason for revert: this CL depends depends on https://crrev.com/c/2495969 that has to be reverted due to test
failures and crashes (https://crbug.com/1149348, https://crbug.com/1149382). Please contact bsazonov@chromium.org or
pnoland@chromium.org if you have any questions.

Original change's description:
> [ToolbarMVC] Observe incognito state changes in location bar.
>
> This lets us remove LocationBar#updateMicButtonState() which was
> a "poke" method on the interface.
>
> Do not submit before M88 branch point (Nov 12th)
>
> Bug: 1142883
> Change-Id: If7564ac95e6ca69e28e6de7a609f0db9fa916c9e
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2518202
> Commit-Queue: bttk <bttk@chromium.org>
> Reviewed-by: Patrick Noland <pnoland@chromium.org>
> Reviewed-by: Filip Gorski <fgorski@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#827544}

TBR=fgorski@chromium.org,pnoland@chromium.org,bttk@chromium.org

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

Bug: 1142883, 11493484, 1149382
Change-Id: Ia30a5089a20687be350892b75411eec2a27f7691
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2540528Reviewed-by: default avatarBoris Sazonov <bsazonov@chromium.org>
Reviewed-by: default avatarbttk <bttk@chromium.org>
Reviewed-by: default avatarFilip Gorski <fgorski@chromium.org>
Commit-Queue: Patrick Noland <pnoland@chromium.org>
Cr-Commit-Position: refs/heads/master@{#827847}
parent 0faedaea
...@@ -745,9 +745,6 @@ public class CustomTabToolbar extends ToolbarLayout implements View.OnLongClickL ...@@ -745,9 +745,6 @@ public class CustomTabToolbar extends ToolbarLayout implements View.OnLongClickL
UrlBar.ScrollType.SCROLL_TO_TLD, SelectionState.SELECT_ALL); UrlBar.ScrollType.SCROLL_TO_TLD, SelectionState.SELECT_ALL);
} }
@Override
public void onIncognitoStateChanged() {}
@Override @Override
public void updateLoadingState(boolean updateUrl) { public void updateLoadingState(boolean updateUrl) {
if (updateUrl) onUrlChanged(); if (updateUrl) onUrlChanged();
...@@ -834,6 +831,9 @@ public class CustomTabToolbar extends ToolbarLayout implements View.OnLongClickL ...@@ -834,6 +831,9 @@ public class CustomTabToolbar extends ToolbarLayout implements View.OnLongClickL
@Override @Override
public void revertChanges() {} public void revertChanges() {}
@Override
public void updateMicButtonState() {}
@Nullable @Nullable
@Override @Override
public FakeboxDelegate getFakeboxDelegate() { public FakeboxDelegate getFakeboxDelegate() {
......
...@@ -76,12 +76,15 @@ public interface LocationBar extends Destroyable { ...@@ -76,12 +76,15 @@ public interface LocationBar extends Destroyable {
*/ */
View getSecurityIconView(); View getSecurityIconView();
/** Updates the state of the mic button if there is one. */
void updateMicButtonState();
/** Returns the {@link VoiceRecognitionHandler} associated with this LocationBar. */ /** Returns the {@link VoiceRecognitionHandler} associated with this LocationBar. */
@Nullable @Nullable
default VoiceRecognitionHandler getVoiceRecognitionHandler() { default VoiceRecognitionHandler getVoiceRecognitionHandler() {
return null; return null;
} }
/** /**
* Returns a (@link FakeboxDelegate}. * Returns a (@link FakeboxDelegate}.
* *
......
...@@ -243,6 +243,11 @@ public final class LocationBarCoordinator implements LocationBar, FakeboxDelegat ...@@ -243,6 +243,11 @@ public final class LocationBarCoordinator implements LocationBar, FakeboxDelegat
return mLocationBarLayout.getSecurityIconView(); return mLocationBarLayout.getSecurityIconView();
} }
@Override
public void updateMicButtonState() {
mLocationBarLayout.updateMicButtonState();
}
@Nullable @Nullable
@Override @Override
public View getViewForUrlBackFocus() { public View getViewForUrlBackFocus() {
...@@ -334,11 +339,6 @@ public final class LocationBarCoordinator implements LocationBar, FakeboxDelegat ...@@ -334,11 +339,6 @@ public final class LocationBarCoordinator implements LocationBar, FakeboxDelegat
return isTablet() ? mLocationBarLayout : null; return isTablet() ? mLocationBarLayout : null;
} }
@Override
public void onIncognitoStateChanged() {
mLocationBarLayout.updateMicButtonState();
}
/** /**
* Returns the {@link LocationBarCoordinatorPhone} for this coordinator. * Returns the {@link LocationBarCoordinatorPhone} for this coordinator.
* *
......
...@@ -31,10 +31,9 @@ public interface LocationBarDataProvider { ...@@ -31,10 +31,9 @@ public interface LocationBarDataProvider {
*/ */
interface Observer { interface Observer {
void onTitleChanged(); void onTitleChanged();
void onUrlChanged();
void onIncognitoStateChanged();
// TODO(https://crbug.com/1139481): Add methods for other LocationBarDataProvider // TODO(https://crbug.com/1139481): Add methods for other LocationBarDataProvider
// data, e.g. NTP and security state. // data, e.g. NTP and security state.
void onUrlChanged();
} }
/** Adds an observer of changes to LocationBarDataProvider's data. */ /** Adds an observer of changes to LocationBarDataProvider's data. */
......
...@@ -106,7 +106,6 @@ public class LocationBarModel implements ToolbarDataProvider, LocationBarDataPro ...@@ -106,7 +106,6 @@ public class LocationBarModel implements ToolbarDataProvider, LocationBarDataPro
/** /**
* Sets the tab that contains the information to be displayed in the toolbar. * Sets the tab that contains the information to be displayed in the toolbar.
*
* @param tab The tab associated currently with the toolbar. * @param tab The tab associated currently with the toolbar.
* @param isIncognito Whether the incognito model is currently selected, which must match the * @param isIncognito Whether the incognito model is currently selected, which must match the
* passed in tab if non-null. * passed in tab if non-null.
...@@ -114,10 +113,7 @@ public class LocationBarModel implements ToolbarDataProvider, LocationBarDataPro ...@@ -114,10 +113,7 @@ public class LocationBarModel implements ToolbarDataProvider, LocationBarDataPro
public void setTab(Tab tab, boolean isIncognito) { public void setTab(Tab tab, boolean isIncognito) {
assert tab == null || tab.isIncognito() == isIncognito; assert tab == null || tab.isIncognito() == isIncognito;
mTab = tab; mTab = tab;
if (mIsIncognito != isIncognito) { mIsIncognito = isIncognito;
mIsIncognito = isIncognito;
notifyIncognitoStateChanged();
}
updateUsingBrandColor(); updateUsingBrandColor();
notifyTitleChanged(); notifyTitleChanged();
notifyUrlChanged(); notifyUrlChanged();
...@@ -289,12 +285,6 @@ public class LocationBarModel implements ToolbarDataProvider, LocationBarDataPro ...@@ -289,12 +285,6 @@ public class LocationBarModel implements ToolbarDataProvider, LocationBarDataPro
return mIsIncognito; return mIsIncognito;
} }
private void notifyIncognitoStateChanged() {
for (LocationBarDataProvider.Observer observer : mLocationBarDataObservers) {
observer.onIncognitoStateChanged();
}
}
/** /**
* @return Whether the location bar is showing in overview mode. If the location bar should not * @return Whether the location bar is showing in overview mode. If the location bar should not
* currently be showing in overview mode, returns false. * currently be showing in overview mode, returns false.
......
...@@ -525,6 +525,7 @@ public abstract class ToolbarLayout ...@@ -525,6 +525,7 @@ public abstract class ToolbarLayout
*/ */
void onTabOrModelChanged() { void onTabOrModelChanged() {
mTabOrModelChangeRunnable.run(); mTabOrModelChangeRunnable.run();
getLocationBar().updateMicButtonState();
} }
/** /**
......
...@@ -4,12 +4,6 @@ ...@@ -4,12 +4,6 @@
package org.chromium.chrome.browser.toolbar; package org.chromium.chrome.browser.toolbar;
import static org.junit.Assert.assertEquals;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import android.support.test.InstrumentationRegistry; import android.support.test.InstrumentationRegistry;
import androidx.test.filters.MediumTest; import androidx.test.filters.MediumTest;
...@@ -22,34 +16,25 @@ import org.junit.Test; ...@@ -22,34 +16,25 @@ import org.junit.Test;
import org.junit.runner.RunWith; import org.junit.runner.RunWith;
import org.chromium.base.ContextUtils; import org.chromium.base.ContextUtils;
import org.chromium.base.test.params.ParameterAnnotations;
import org.chromium.base.test.params.ParameterProvider;
import org.chromium.base.test.params.ParameterSet;
import org.chromium.base.test.params.ParameterizedRunner;
import org.chromium.base.test.util.CommandLineFlags; import org.chromium.base.test.util.CommandLineFlags;
import org.chromium.base.test.util.Feature; import org.chromium.base.test.util.Feature;
import org.chromium.chrome.R; import org.chromium.chrome.R;
import org.chromium.chrome.browser.ChromeTabbedActivity; import org.chromium.chrome.browser.ChromeTabbedActivity;
import org.chromium.chrome.browser.flags.ChromeSwitches; import org.chromium.chrome.browser.flags.ChromeSwitches;
import org.chromium.chrome.browser.omnibox.LocationBarDataProvider;
import org.chromium.chrome.browser.omnibox.UrlBarData; import org.chromium.chrome.browser.omnibox.UrlBarData;
import org.chromium.chrome.browser.tab.MockTab; import org.chromium.chrome.browser.tab.MockTab;
import org.chromium.chrome.browser.tab.Tab; import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tab.TabSelectionType;
import org.chromium.chrome.browser.toolbar.top.ToolbarLayout; import org.chromium.chrome.browser.toolbar.top.ToolbarLayout;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.chrome.test.ChromeTabbedActivityTestRule; import org.chromium.chrome.test.ChromeTabbedActivityTestRule;
import org.chromium.chrome.test.util.ChromeTabUtils; import org.chromium.chrome.test.util.ChromeTabUtils;
import org.chromium.components.embedder_support.util.UrlConstants; import org.chromium.components.embedder_support.util.UrlConstants;
import org.chromium.content_public.browser.test.util.TestThreadUtils; import org.chromium.content_public.browser.test.util.TestThreadUtils;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
/** /**
* Tests for LocationBarModel. * Tests for LocationBarModel.
*/ */
@RunWith(ParameterizedRunner.class) @RunWith(ChromeJUnit4ClassRunner.class)
@CommandLineFlags.Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE}) @CommandLineFlags.Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE})
public class LocationBarModelTest { public class LocationBarModelTest {
@Rule @Rule
...@@ -72,9 +57,9 @@ public class LocationBarModelTest { ...@@ -72,9 +57,9 @@ public class LocationBarModelTest {
getCurrentTabId(mActivityTestRule.getActivity())); getCurrentTabId(mActivityTestRule.getActivity()));
ChromeTabUtils.closeCurrentTab( ChromeTabUtils.closeCurrentTab(
InstrumentationRegistry.getInstrumentation(), mActivityTestRule.getActivity()); InstrumentationRegistry.getInstrumentation(), mActivityTestRule.getActivity());
assertEquals("Didn't close all tabs.", 0, Assert.assertEquals("Didn't close all tabs.", 0,
ChromeTabUtils.getNumOpenTabs(mActivityTestRule.getActivity())); ChromeTabUtils.getNumOpenTabs(mActivityTestRule.getActivity()));
assertEquals("LocationBarModel is still trying to show a tab.", Tab.INVALID_TAB_ID, Assert.assertEquals("LocationBarModel is still trying to show a tab.", Tab.INVALID_TAB_ID,
getCurrentTabId(mActivityTestRule.getActivity())); getCurrentTabId(mActivityTestRule.getActivity()));
} }
...@@ -103,107 +88,13 @@ public class LocationBarModelTest { ...@@ -103,107 +88,13 @@ public class LocationBarModelTest {
}); });
} }
/** Provides parameters for different types of transitions between tabs. */
public static class IncognitoTransitionParamProvider implements ParameterProvider {
@Override
public Iterable<ParameterSet> getParameters() {
List<ParameterSet> result = new ArrayList<>(8);
for (boolean fromIncognito : Arrays.asList(true, false)) {
for (boolean toIncognito : Arrays.asList(true, false)) {
result.add(new ParameterSet()
.value(fromIncognito, toIncognito)
.name(String.format(
"from_%b_to_%b", fromIncognito, toIncognito)));
}
}
return result;
}
}
@Test
@MediumTest
@ParameterAnnotations.UseMethodParameter(IncognitoTransitionParamProvider.class)
public void testOnIncognitoStateChange_switchTab(boolean fromIncognito, boolean toIncognito) {
// Add a regular tab next to the one created in setup.
mActivityTestRule.loadUrlInNewTab("about:blank", /*incognito=*/false);
// Add two incognito tabs.
mActivityTestRule.loadUrlInNewTab("about:blank", /*incognito=*/true);
mActivityTestRule.loadUrlInNewTab("about:blank", /*incognito=*/true);
ChromeTabbedActivity activity = mActivityTestRule.getActivity();
LocationBarModel locationBarModel =
activity.getToolbarManager().getLocationBarModelForTesting();
LocationBarDataProvider.Observer observer = mock(LocationBarDataProvider.Observer.class);
doAnswer((invocation) -> {
assertEquals(toIncognito, locationBarModel.isIncognito());
return null;
})
.when(observer)
.onIncognitoStateChanged();
TestThreadUtils.runOnUiThreadBlocking(() -> {
mActivityTestRule.getActivity().getTabModelSelector().selectModel(fromIncognito);
locationBarModel.addObserver(observer);
// Switch to an existing tab.
mActivityTestRule.getActivity().getTabModelSelector().selectModel(/*incognito=*/
toIncognito);
mActivityTestRule.getActivity().getTabModelSelector().getCurrentModel().setIndex(
0, TabSelectionType.FROM_USER);
});
assertEquals(toIncognito, locationBarModel.isIncognito());
if (fromIncognito != toIncognito) {
verify(observer).onIncognitoStateChanged();
} else {
verify(observer, times(0)).onIncognitoStateChanged();
}
}
@Test
@MediumTest
@ParameterAnnotations.UseMethodParameter(IncognitoTransitionParamProvider.class)
public void testOnIncognitoStateChange_newTab(boolean fromIncognito, boolean toIncognito) {
// Add a regular tab next to the one created in setup.
mActivityTestRule.loadUrlInNewTab("about:blank", /*incognito=*/false);
// Add two incognito tabs.
mActivityTestRule.loadUrlInNewTab("about:blank", /*incognito=*/true);
mActivityTestRule.loadUrlInNewTab("about:blank", /*incognito=*/true);
ChromeTabbedActivity activity = mActivityTestRule.getActivity();
LocationBarModel locationBarModel =
activity.getToolbarManager().getLocationBarModelForTesting();
LocationBarDataProvider.Observer observer = mock(LocationBarDataProvider.Observer.class);
doAnswer((invocation) -> {
assertEquals(toIncognito, locationBarModel.isIncognito());
return null;
})
.when(observer)
.onIncognitoStateChanged();
TestThreadUtils.runOnUiThreadBlocking(() -> {
mActivityTestRule.getActivity().getTabModelSelector().selectModel(fromIncognito);
locationBarModel.addObserver(observer);
});
// Switch to a new tab.
mActivityTestRule.loadUrlInNewTab("about:blank", toIncognito);
assertEquals(toIncognito, locationBarModel.isIncognito());
if (fromIncognito != toIncognito) {
verify(observer).onIncognitoStateChanged();
} else {
verify(observer, times(0)).onIncognitoStateChanged();
}
}
private void assertDisplayAndEditText( private void assertDisplayAndEditText(
ToolbarDataProvider dataProvider, String displayText, String editText) { ToolbarDataProvider dataProvider, String displayText, String editText) {
TestThreadUtils.runOnUiThreadBlocking(() -> { TestThreadUtils.runOnUiThreadBlocking(() -> {
UrlBarData urlBarData = dataProvider.getUrlBarData(); UrlBarData urlBarData = dataProvider.getUrlBarData();
assertEquals( Assert.assertEquals(
"Display text did not match", displayText, urlBarData.displayText.toString()); "Display text did not match", displayText, urlBarData.displayText.toString());
assertEquals("Editing text did not match", editText, urlBarData.editingText); Assert.assertEquals("Editing text did not match", editText, urlBarData.editingText);
}); });
} }
......
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