[Contextual Search] Improve testing & add regressions tests

chrome/android/javatests/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManagerTest.java

Index: chrome/android/javatests/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManagerTest.java
diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManagerTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManagerTest.java
index 0dc28a8b4e641cca8daac1279053c0d3de5e85af..436586fab6b45f68a2be2f0b75e78ded251f10cf 100644
--- a/chrome/android/javatests/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManagerTest.java
+++ b/chrome/android/javatests/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManagerTest.java
@@ -61,7 +61,12 @@ import org.chromium.ui.touch_selection.SelectionEventType;
 
 import java.util.concurrent.TimeoutException;
 
-// TODO(pedrosimonetti): add tests for recent regressions crbug.com/543319.
+// TODO(pedrosimonetti): Create class with limited API to encapsulate the internals of simulations.
+// TODO(pedrosimonetti): Separate tests into different classes grouped by type of tests. Examples:
+// Gestures (Tap, LongPress), Search Term Resolution (resolves, expand selection, prevent preload,
+// translation), Panel interaction (tap, fling up/down, close), Content (creation, loading,
+// visibility, history, delayed load), Tab Promotion, Policy (add tests to check if policies
+// affect the behavior correctly), General (remaining tests), etc.
 
 /**
  * Tests the Contextual Search Manager using instrumentation tests.
@@ -98,18 +103,24 @@ public class ContextualSearchManagerTest extends ChromeActivityTestCaseBase<Chro
 
         if (mManager != null) {
             mPanel = mManager.getContextualSearchPanel();
-            mFakeServer = new ContextualSearchFakeServer(mManager,
+
+            mSelectionController = mManager.getSelectionController();
+            mPolicy = ContextualSearchPolicy.getInstance(getActivity());
+            mPolicy.overrideDecidedStateForTesting(true);
+            resetCounters();
+
+            mFakeServer = new ContextualSearchFakeServer(
+                    mPolicy,
+                    this,
+                    mManager,
                     mManager.getOverlayContentDelegate(),
                     new OverlayContentProgressObserver(),
                     getActivity());
 
             mPanel.setOverlayPanelContentFactory(mFakeServer);
             mManager.setNetworkCommunicator(mFakeServer);
-            mSelectionController = mManager.getSelectionController();
-            mPolicy = ContextualSearchPolicy.getInstance(getActivity());
 
-            mPolicy.overrideDecidedStateForTesting(true);
-            resetCounters();
+            registerFakeSearches();
         }
 
         IntentFilter filter = new IntentFilter(Intent.ACTION_VIEW);
@@ -119,64 +130,132 @@ public class ContextualSearchManagerTest extends ChromeActivityTestCaseBase<Chro
                 filter, new Instrumentation.ActivityResult(Activity.RESULT_OK, null), true);
     }
 
-    @Override
-    public void startMainActivity() throws InterruptedException {
-        startMainActivityWithURL(TEST_PAGE);
+    //============================================================================================
+    // Public API
+    //============================================================================================
+
+    /**
+     * Simulates a long-press on the given node.
+     * @param nodeId A string containing the node ID.
+     */
+    public void longPressNode(String nodeId) throws InterruptedException, TimeoutException {
+        Tab tab = getActivity().getActivityTab();
+        DOMUtils.longPressNode(this, tab.getContentViewCore(), nodeId);
+        waitForPanelToPeekAndAssert();
     }
 
     /**
      * Simulates a click on the given node.
      * @param nodeId A string containing the node ID.
      */
-    private void clickNode(String nodeId) throws InterruptedException, TimeoutException {
+    public void clickNode(String nodeId) throws InterruptedException, TimeoutException {
         Tab tab = getActivity().getActivityTab();
         DOMUtils.clickNode(this, tab.getContentViewCore(), nodeId);
     }
 
     /**
-     * Simulates a click on the given word node.
-     * Waits for the bar to peek.
-     * @param nodeId A string containing the node ID.
+     * Waits for the selected text string to be the given string, and asserts.
+     * @param text The string to wait for the selection to become.
      */
-    private void clickWordNode(String nodeId) throws InterruptedException, TimeoutException {
-        clickNode(nodeId);
-        waitForPanelToPeekAndAssert();
+    public void waitForSelectionToBe(final String text) throws InterruptedException {
+        assertTrue("Bar never showed desired text.",
+                CriteriaHelper.pollForCriteria(new Criteria() {
+                    @Override
+                    public boolean isSatisfied() {
+                        return TextUtils.equals(text, getSelectedText());
+                    }
+                }, TEST_TIMEOUT, DEFAULT_POLLING_INTERVAL));
     }
 
     /**
-     * Simulates a key press.
-     * @param keycode The key's code.
+     * Waits for the FakeTapSearch to become ready.
+     * @param search A given FakeTapSearch.
      */
-    private void pressKey(int keycode) {
-        getInstrumentation().sendKeyDownUpSync(keycode);
-        getInstrumentation().waitForIdleSync();
+    public void waitForSearchTermResolutionToStart(
+            final ContextualSearchFakeServer.FakeTapSearch search) throws InterruptedException {
+        assertTrue("Fake Search Term Resolution never started.",
+                CriteriaHelper.pollForCriteria(new Criteria() {
+                    @Override
+                    public boolean isSatisfied() {
+                        return search.didStartSearchTermResolution();
+                    }
+                }, TEST_TIMEOUT, DEFAULT_POLLING_INTERVAL));
     }
 
     /**
-     * Simulates pressing back button.
+     * Waits for the FakeTapSearch to become ready.
+     * @param search A given FakeTapSearch.
      */
-    private void pressBackButton() {
-        pressKey(KeyEvent.KEYCODE_BACK);
+    public void waitForSearchTermResolutionToFinish(
+            final ContextualSearchFakeServer.FakeTapSearch search) throws InterruptedException {
+        assertTrue("Fake Search was never ready.",
+                CriteriaHelper.pollForCriteria(new Criteria() {
+                    @Override
+                    public boolean isSatisfied() {
+                        return search.didFinishSearchTermResolution();
+                    }
+                }, TEST_TIMEOUT, DEFAULT_POLLING_INTERVAL));
     }
 
     /**
-     * @return The selected text.
+     * Runs the given Runnable in the main thread.
+     * @param runnable The Runnable.
      */
-    private String getSelectedText() {
-        return mSelectionController.getSelectedText();
+    public void runOnMainSync(Runnable runnable) {
+        getInstrumentation().runOnMainSync(runnable);
     }
 
+    @Override
+    public void startMainActivity() throws InterruptedException {
+        startMainActivityWithURL(TEST_PAGE);
+    }
+
+    //============================================================================================
+    // Fake Searches Helpers
+    //============================================================================================
+
     /**
-     * Simulates a long-press on the given node.
-     * @param nodeId A string containing the node ID.
+     * Simulates a long-press triggered search.
+     *
+     * @param nodeId The id of the node to be long-pressed.
+     * @throws InterruptedException

[Donn Denman, 2015/10/27 19:18:52]

Nit: I think you can put both of these on one line.

[pedro (no code reviews), 2015/10/29 22:05:29]

By looking at other files using multiple @throws tag, it seems multiple tags are
being used, so I'll keep this way.

+     * @throws TimeoutException
      */
-    private void longPressNode(String nodeId) throws InterruptedException, TimeoutException {
-        Tab tab = getActivity().getActivityTab();
-        DOMUtils.longPressNode(this, tab.getContentViewCore(), nodeId);
+    private void simulateLongPressSearch(String nodeId)
+            throws InterruptedException, TimeoutException {
+        ContextualSearchFakeServer.FakeLongPressSearch search =
+                mFakeServer.getFakeLongPressSearch(nodeId);
+        search.simulate();
         waitForPanelToPeekAndAssert();
     }
 
     /**
+     * Simulates a tap triggered search.
+     *
+     * @param nodeId The id of the node to be tapped.
+     * @throws InterruptedException
+     * @throws TimeoutException
+     */
+    private void simulateTapSearch(String nodeId) throws InterruptedException, TimeoutException {
+        ContextualSearchFakeServer.FakeTapSearch search = mFakeServer.getFakeTapSearch(nodeId);
+        search.simulate();
+        assertLoadedSearchTermMatches(search.getSearchTerm());
+        waitForPanelToPeekAndAssert();
+    }
+
+    /**
+     * Registers all fake searches to be used in tests.
+     */
+    private void registerFakeSearches() {
+        mFakeServer.registerFakeSearches();
+    }
+
+    //============================================================================================
+    // Fake Response
+    // TODO(pedrosimonetti): remove these methods and use the new infrastructure instead.
+    //============================================================================================
+
+    /**
      * Posts a fake response on the Main thread.
      */
     private final class FakeResponseOnMainThread implements Runnable {
@@ -191,8 +270,8 @@ public class ContextualSearchManagerTest extends ChromeActivityTestCaseBase<Chro
         private final int mEndAdjust;
 
         public FakeResponseOnMainThread(boolean isNetworkUnavailable, int responseCode,
-                String searchTerm, String displayText, String alternateTerm,
-                boolean doPreventPreload, int startAdjust, int endAdjudst) {
+                                        String searchTerm, String displayText, String alternateTerm,
+                                        boolean doPreventPreload, int startAdjust, int endAdjudst) {
             mIsNetworkUnavailable = isNetworkUnavailable;
             mResponseCode = responseCode;
             mSearchTerm = searchTerm;
@@ -235,6 +314,151 @@ public class ContextualSearchManagerTest extends ChromeActivityTestCaseBase<Chro
         }
     }
 
+    //============================================================================================
+    // Content Helpers
+    //============================================================================================
+
+    /**
+     * @return The Panel's ContentViewCore.
+     */
+    private ContentViewCore getPanelContentViewCore() {
+        return mPanel.getContentViewCore();
+    }
+
+    /**
+     * @return Whether the Panel's ContentViewCore is visible.
+     */
+    private boolean isContentViewCoreVisible() {
+        ContextualSearchFakeServer.ContentViewCoreWrapper contentViewCore =
+                (ContextualSearchFakeServer.ContentViewCoreWrapper) getPanelContentViewCore();
+        return contentViewCore != null ? contentViewCore.isVisible() : false;
+    }
+
+    /**
+     * @return Whether the Panel's ContentViewCore onShow() method was ever called.
+     */
+    private boolean didEverCallContentViewCoreOnShow() {
+        ContextualSearchFakeServer.ContentViewCoreWrapper contentViewCore =
+                (ContextualSearchFakeServer.ContentViewCoreWrapper) getPanelContentViewCore();
+        return contentViewCore != null ? contentViewCore.didEverCallOnShow() : false;
+    }
+
+    /**
+     * Asserts that the Panel's ContentViewCore is created.
+     */
+    private void assertContentViewCoreCreated() {
+        assertNotNull(getPanelContentViewCore());
+    }
+
+    /**
+     * Asserts that the Panel's ContentViewCore is not created.
+     */
+    private void assertNoContentViewCore() {
+        assertNull(getPanelContentViewCore());
+    }
+
+    /**
+     * Asserts that the Panel's ContentViewCore is visible.
+     */
+    private void assertContentViewCoreVisible() {
+        assertTrue(isContentViewCoreVisible());
+    }
+
+    /**
+     * Asserts that the Panel's ContentViewCore is not visible.
+     */
+    private void assertContentViewCoreNotVisible() {
+        assertFalse(isContentViewCoreVisible());
+    }
+
+    /**
+     * Asserts that the Panel's ContentViewCore onShow() method was never called.
+     */
+    private void assertNeverCalledContentViewCoreOnShow() {
+        assertFalse(didEverCallContentViewCoreOnShow());
+    }
+
+    /**
+     * Fakes navigation of the Content View with the given httpResult code.

[Donn Denman, 2015/10/27 19:18:52]

Nit: Change "with the given httpResult code" to something like "with the given
failure boolean".

[pedro (no code reviews), 2015/10/29 22:05:29]

Changed the comment. No mention to the param anymore, since it's explained right
below.

+     * The URL of the navigation is the one requested previously.
+     * @param isFailure If the request resulted in a failure.
+     */
+    private void fakeContentViewDidNavigate(boolean isFailure) {
+        String url = mFakeServer.getLoadedUrl();
+        mManager.getOverlayContentDelegate().onMainFrameNavigation(url, isFailure);
+    }
+
+    /**
+     * A ContentViewCore that has some methods stubbed out for testing.
+     * TODO(pedrosimonetti): consider using the real ContentViewCore instead.
+     */
+    private static final class StubbedContentViewCore extends ContentViewCore {
+        private boolean mIsFocusedNodeEditable;
+
+        public StubbedContentViewCore(Context context) {
+            super(context);
+        }
+
+        public void setIsFocusedNodeEditableForTest(boolean isFocusedNodeEditable) {

[Donn Denman, 2015/10/27 19:18:52]

Nit: add simple javadoc.

[pedro (no code reviews), 2015/10/29 22:05:29]

Done.

+            mIsFocusedNodeEditable = isFocusedNodeEditable;
+        }
+
+        @Override
+        public boolean isFocusedNodeEditable() {
+            return mIsFocusedNodeEditable;
+        }
+    }
+
+    //============================================================================================
+    // Other Helpers
+    // TODO(pedrosimonetti): organize into sections.
+    //============================================================================================
+
+    /**
+     * Simulates a click on the given word node.
+     * Waits for the bar to peek.
+     * @param nodeId A string containing the node ID.
+     */
+    private void clickWordNode(String nodeId) throws InterruptedException, TimeoutException {
+        clickNode(nodeId);
+        waitForPanelToPeekAndAssert();
+    }
+
+    /**
+     * Simulates a key press.
+     * @param keycode The key's code.
+     */
+    private void pressKey(int keycode) {
+        getInstrumentation().sendKeyDownUpSync(keycode);
+        getInstrumentation().waitForIdleSync();
+    }
+
+    /**
+     * Simulates pressing back button.
+     */
+    private void pressBackButton() {
+        pressKey(KeyEvent.KEYCODE_BACK);
+    }
+
+    /**
+     * @return The selected text.
+     */
+    private String getSelectedText() {
+        return mSelectionController.getSelectedText();
+    }
+
+    /**
+     * Asserts that the loaded search term matches the provided value.
+     * @param searchTerm The provided search term.
+     */
+    private void assertLoadedSearchTermMatches(String searchTerm) {
+        boolean doesMatch = false;
+        if (mFakeServer != null) {
+            doesMatch = mFakeServer.getLoadedUrl().contains("q=" + searchTerm);
+        }
+        assertTrue(doesMatch);
+    }
+
     private void assertContainsParameters(String searchTerm, String alternateTerm) {
         assertTrue(mFakeServer == null || mFakeServer.getSearchTermRequested() == null
                 || mFakeServer.getLoadedUrl().contains(searchTerm)
@@ -277,7 +501,7 @@ public class ContextualSearchManagerTest extends ChromeActivityTestCaseBase<Chro
                 && mFakeServer.getLoadedUrl().contains(LOW_PRIORITY_SEARCH_ENDPOINT));
         assertTrue("Low priority request does not have the required prefetch parameter!",
                 mFakeServer.getLoadedUrl() != null
-                && mFakeServer.getLoadedUrl().contains(CONTEXTUAL_SEARCH_PREFETCH_PARAM));
+                        && mFakeServer.getLoadedUrl().contains(CONTEXTUAL_SEARCH_PREFETCH_PARAM));
     }
 
     private void assertLoadedNormalPriorityUrl() {
@@ -288,22 +512,14 @@ public class ContextualSearchManagerTest extends ChromeActivityTestCaseBase<Chro
                 && mFakeServer.getLoadedUrl().contains(NORMAL_PRIORITY_SEARCH_ENDPOINT));
         assertTrue("Normal priority request should not have the prefetch parameter, but did!",
                 mFakeServer.getLoadedUrl() != null
-                && !mFakeServer.getLoadedUrl().contains(CONTEXTUAL_SEARCH_PREFETCH_PARAM));
+                        && !mFakeServer.getLoadedUrl().contains(CONTEXTUAL_SEARCH_PREFETCH_PARAM));
     }
 
     private void assertNoSearchesLoaded() {
-        assertEquals(0, mFakeServer.loadedUrlCount());
+        assertEquals(0, mFakeServer.getLoadedUrlCount());
         assertLoadedNoUrl();
     }
 
-    private void assertContentViewCoreCreated() {
-        assertTrue(mFakeServer.didCreateContentView());
-    }
-
-    private void assertNoContentViewCore() {
-        assertFalse(mFakeServer.didCreateContentView());
-    }
-
     /**
      * Asserts that the tap triggered promo counter is enabled and at the specified count.
      */
@@ -321,16 +537,6 @@ public class ContextualSearchManagerTest extends ChromeActivityTestCaseBase<Chro
     }
 
     /**
-     * Fakes navigation of the Content View with the given httpResult code.
-     * The URL of the navigation is the one requested previously.
-     * @param isFailure If the request resulted in a failure.
-     */
-    private void fakeContentViewDidNavigate(boolean isFailure) {
-        String url = mFakeServer.getLoadedUrl();
-        mManager.getOverlayContentDelegate().onMainFrameNavigation(url, isFailure);
-    }
-
-    /**
      * Waits for the Search Panel (the Search Bar) to peek up from the bottom, and asserts that it
      * did peek.
      * @throws InterruptedException
@@ -417,20 +623,6 @@ public class ContextualSearchManagerTest extends ChromeActivityTestCaseBase<Chro
     }
 
     /**
-     * Waits for the selected text string to be the given string, and asserts.
-     * @param text The string to wait for the selection to become.
-     */
-    private void waitForSelectionToBe(final String text) throws InterruptedException {
-        assertTrue("Bar never showed desired text.",
-                CriteriaHelper.pollForCriteria(new Criteria() {
-                    @Override
-                    public boolean isSatisfied() {
-                        return TextUtils.equals(text, getSelectedText());
-                    }
-                }, TEST_TIMEOUT, DEFAULT_POLLING_INTERVAL));
-    }
-
-    /**
      * Waits for the selection to be dissolved.
      * Use this method any time a test repeatedly establishes and dissolves a selection to ensure
      * that the selection has been completely dissolved before simulating the next selection event.
@@ -454,24 +646,13 @@ public class ContextualSearchManagerTest extends ChromeActivityTestCaseBase<Chro
         waitForSelectionDissolved();
     }
 
-    /**
-     * A ContentViewCore that has some methods stubbed out for testing.
-     */
-    private static final class StubbedContentViewCore extends ContentViewCore {
-        private boolean mIsFocusedNodeEditable;
-
-        public StubbedContentViewCore(Context context) {
-            super(context);
-        }
-
-        public void setIsFocusedNodeEditableForTest(boolean isFocusedNodeEditable) {
-            mIsFocusedNodeEditable = isFocusedNodeEditable;
-        }
-
-        @Override
-        public boolean isFocusedNodeEditable() {
-            return mIsFocusedNodeEditable;
-        }
+    private void waitToPreventDoubleTapRecognition() throws InterruptedException {
+        // Avoid issues with double-tap detection by ensuring sequential taps
+        // aren't treated as such. Double-tapping can also select words much as
+        // longpress, in turn showing the pins and preventing contextual tap
+        // refinement from nearby taps. The double-tap timeout is sufficiently
+        // short that this shouldn't conflict with tap refinement by the user.
+        Thread.sleep(ViewConfiguration.getDoubleTapTimeout());
     }
 
     /**
@@ -492,14 +673,21 @@ public class ContextualSearchManagerTest extends ChromeActivityTestCaseBase<Chro
     }
 
     /**
-     * Swipes the panel up to it's expanded size.
+     * Swipes the panel up to its expanded state.
      */
     private void swipePanelUp() {
         swipe(0.5f, 0.95f, 0.5f, 0.55f, 1000);
     }
 
     /**
-     * Swipes the panel up to it's maximized size.
+     * Swipes the panel down to its peeked state.
+     */
+    private void swipePanelDown() {
+        swipe(0.5f, 0.55f, 0.5f, 0.95f, 1000);
+    }
+
+    /**
+     * Swipes the panel up to its maximized state.
      */
     private void swipePanelUpToTop() {
         swipe(0.5f, 0.95f, 0.5f, 0.05f, 1000);
@@ -516,7 +704,15 @@ public class ContextualSearchManagerTest extends ChromeActivityTestCaseBase<Chro
      * Taps the base page near the top.
      */
     private void tapBasePageToClosePanel() throws InterruptedException {
-        tapBasePage(0.1f, 0.1f);
+        // TODO(pedrosimonetti): This is not reliable. Find a better approach.
+        // We use the far right side (x == 0.9f) to prevent simulating a tap on top of an
+        // existing long-press selection (the pins are a tap target). This might not work on RTL.
+        // We are using y == 0.2f because otherwise it will fail for long press cases.
+        // It might be better to get the position of the Panel and tap just about outside
+        // the Panel. I suspect some Flaky tests are caused by this problem (ones involving
+        // long press and trying to close with the bar peeking, with a long press selection
+        // established).
+        tapBasePage(0.9f, 0.2f);
         waitForPanelToCloseAndAssert();
     }
 
@@ -639,6 +835,10 @@ public class ContextualSearchManagerTest extends ChromeActivityTestCaseBase<Chro
         }, TEST_TIMEOUT, DEFAULT_POLLING_INTERVAL);
     }
 
+    //============================================================================================
+    // Test Cases
+    //============================================================================================
+
     /**
      * Tests whether the contextual search panel hides when omnibox is clicked.
      */
@@ -747,13 +947,13 @@ public class ContextualSearchManagerTest extends ChromeActivityTestCaseBase<Chro
         fakeResponse(false, 200, "Intelligence", "United States Intelligence", "alternate-term",
                 false);
         assertContainsParameters("Intelligence", "alternate-term");
-        assertEquals(1, mFakeServer.loadedUrlCount());
+        assertEquals(1, mFakeServer.getLoadedUrlCount());
         assertLoadedLowPriorityUrl();
 
         waitForPanelToPeekAndAssert();
         swipePanelUp();
         waitForPanelToExpandAndAssert();
-        assertEquals(1, mFakeServer.loadedUrlCount());
+        assertEquals(1, mFakeServer.getLoadedUrlCount());
         assertLoadedLowPriorityUrl();
     }
 
@@ -768,6 +968,10 @@ public class ContextualSearchManagerTest extends ChromeActivityTestCaseBase<Chro
         longPressNode("intelligence");
         assertNoContentViewCore();
 
+        // TODO(pedrosimonetti): Long press does not resolve so we shouldn't be faking one.
+        // Consider changing the fake server to create a fake response automatically,
+        // when one is requested by the Manager.
+
         // Fake a search term resolution response.
         fakeResponse(false, 200, "Intelligence", "United States Intelligence", "alternate-term",
                 false);
@@ -780,11 +984,11 @@ public class ContextualSearchManagerTest extends ChromeActivityTestCaseBase<Chro
         waitForPanelToExpandAndAssert();
         assertContentViewCoreCreated();
         assertLoadedNormalPriorityUrl();
-        assertEquals(1, mFakeServer.loadedUrlCount());
+        assertEquals(1, mFakeServer.getLoadedUrlCount());
 
         // tap the base page to close.
         tapBasePageToClosePanel();
-        assertEquals(1, mFakeServer.loadedUrlCount());
+        assertEquals(1, mFakeServer.getLoadedUrlCount());
         assertNoContentViewCore();
     }
 
@@ -819,16 +1023,16 @@ public class ContextualSearchManagerTest extends ChromeActivityTestCaseBase<Chro
         // Fake a search term resolution response.
         fakeResponse(false, 200, "states", "United States Intelligence", "alternate-term", false);
         assertContainsParameters("states", "alternate-term");
-        assertEquals(1, mFakeServer.loadedUrlCount());
+        assertEquals(1, mFakeServer.getLoadedUrlCount());
         assertLoadedLowPriorityUrl();
         assertContentViewCoreCreated();
         tapPeekingBarToExpandAndAssert();
         assertLoadedLowPriorityUrl();
-        assertEquals(1, mFakeServer.loadedUrlCount());
+        assertEquals(1, mFakeServer.getLoadedUrlCount());
 
         // tap the base page to close.
         tapBasePageToClosePanel();
-        assertEquals(1, mFakeServer.loadedUrlCount());
+        assertEquals(1, mFakeServer.getLoadedUrlCount());
         assertNoContentViewCore();
     }
 
@@ -844,20 +1048,20 @@ public class ContextualSearchManagerTest extends ChromeActivityTestCaseBase<Chro
 
         // We should not make a second-request until we get a good response from the first-request.
         assertLoadedNoUrl();
-        assertEquals(0, mFakeServer.loadedUrlCount());
+        assertEquals(0, mFakeServer.getLoadedUrlCount());
         fakeResponse(false, 200, "states", "United States Intelligence", "alternate-term", false);
         assertLoadedLowPriorityUrl();
-        assertEquals(1, mFakeServer.loadedUrlCount());
+        assertEquals(1, mFakeServer.getLoadedUrlCount());
 
         // When the second request succeeds, we should not issue a new request.
         fakeContentViewDidNavigate(false);
         assertLoadedLowPriorityUrl();
-        assertEquals(1, mFakeServer.loadedUrlCount());
+        assertEquals(1, mFakeServer.getLoadedUrlCount());
 
         // When the bar opens, we should not make any additional request.
         tapPeekingBarToExpandAndAssert();
         assertLoadedLowPriorityUrl();
-        assertEquals(1, mFakeServer.loadedUrlCount());
+        assertEquals(1, mFakeServer.getLoadedUrlCount());
         assertLoadedLowPriorityUrl();
     }
 
@@ -874,20 +1078,20 @@ public class ContextualSearchManagerTest extends ChromeActivityTestCaseBase<Chro
 
         // We should not make a SERP request until we get a good response from the resolve request.
         assertLoadedNoUrl();
-        assertEquals(0, mFakeServer.loadedUrlCount());
+        assertEquals(0, mFakeServer.getLoadedUrlCount());
         fakeResponse(false, 200, "states", "United States Intelligence", "alternate-term", false);
         assertLoadedLowPriorityUrl();
-        assertEquals(1, mFakeServer.loadedUrlCount());
+        assertEquals(1, mFakeServer.getLoadedUrlCount());
 
         // When the second request fails, we should not issue a new request.
         fakeContentViewDidNavigate(true);
         assertLoadedLowPriorityUrl();
-        assertEquals(1, mFakeServer.loadedUrlCount());
+        assertEquals(1, mFakeServer.getLoadedUrlCount());
 
         // Once the bar opens, we make a new request at normal priority.
         tapPeekingBarToExpandAndAssert();
         assertLoadedNormalPriorityUrl();
-        assertEquals(2, mFakeServer.loadedUrlCount());
+        assertEquals(2, mFakeServer.getLoadedUrlCount());
     }
 
     /**
@@ -1454,6 +1658,7 @@ public class ContextualSearchManagerTest extends ChromeActivityTestCaseBase<Chro
         assertContainsParameters("states", "alternate-term");
         assertLoadedNormalPriorityUrl();
         assertContentViewCoreCreated();
+        assertContentViewCoreVisible();
     }
 
     /**
@@ -1995,4 +2200,347 @@ public class ContextualSearchManagerTest extends ChromeActivityTestCaseBase<Chro
         waitForPanelToPeekAndAssert();
         assertFalse(mPanel.isPeekPromoVisible());
     }
+
+    //============================================================================================
+    // Content Tests
+    //============================================================================================
+
+    /**
+     * Tests that tap followed by expand makes Content visible.
+     *
+     * @SmallTest

[Donn Denman, 2015/10/27 19:18:51]

Why are these annotations in the comment?  Looks like this test is not disabled,
so I'm confused by this.

[pedro (no code reviews), 2015/10/29 22:05:28]

Thanks for noticing this. Moved to the proper place.

+     * @Feature({"ContextualSearch"})
+     */
+    @Restriction(RESTRICTION_TYPE_NON_LOW_END_DEVICE)
+    public void testTapContentVisibility() throws InterruptedException, TimeoutException {
+        // Simulate a tap and make sure Content is not visible.
+        simulateTapSearch("search");
+        assertContentViewCoreCreated();
+        assertContentViewCoreNotVisible();
+        assertNeverCalledContentViewCoreOnShow();
+        assertEquals(1, mFakeServer.getLoadedUrlCount());

[Donn Denman, 2015/10/27 19:18:52]

I don't think we should be checking the loadedUrlCount here, since you have a
separate test for that below.

[pedro (no code reviews), 2015/10/29 22:05:29]

Good point.

+
+        // Expand Panel and check that the Content is now visible.
+        tapPeekingBarToExpandAndAssert();
+        assertContentViewCoreVisible();

[Donn Denman, 2015/10/27 19:18:52]

Isn't it an invariant that whenever we expand, the contentViewCore is made
visible?  I would think we'd want to just include this assert in
tapPeekingBarToExpandAndAssert.

[pedro (no code reviews), 2015/10/29 22:05:29]

I think that is an implementation detail of the Content and therefore we should
test them separately. Otherwise a bug in the Content visibility could affect all
tests that expand the Panel.

+        assertEquals(1, mFakeServer.getLoadedUrlCount());
+
+        // Close the Panel and check that the Content was destroyed and nothing else was loaded.
+        tapBasePageToClosePanel();
+        assertNoContentViewCore();

[Donn Denman, 2015/10/27 19:18:52]

Similarly once the panel is closed there should be no CVC, so this assert should
be rolled in to the tapBasePageToClosePanel call?

[pedro (no code reviews), 2015/10/29 22:05:29]

Same as above.

+        assertEquals(1, mFakeServer.getLoadedUrlCount());
+    }
+
+    /**
+     * Tests that long press followed by expand creates Content and makes it visible.
+     *
+     * @SmallTest

[Donn Denman, 2015/10/27 19:18:52]

Here too.

[pedro (no code reviews), 2015/10/29 22:05:29]

Done.

+     * @Feature({"ContextualSearch"})
+     */
+    @Restriction(RESTRICTION_TYPE_NON_LOW_END_DEVICE)
+    public void testLongPressContentVisibility() throws InterruptedException, TimeoutException {
+        // Simulate a long press and make sure no Content is created.
+        simulateLongPressSearch("search");
+        assertNoContentViewCore();
+        assertNoSearchesLoaded();
+
+        // Expand Panel and check that the Content is now created and visible.

[Donn Denman, 2015/10/27 19:18:52]

Here's an example of the annoying nit about test-step descriptions focusing on
what we're checking rather than what's being done (which should be evident from
the code).  Maybe change to "The CVC should be visible once the panel has been
expanded".

[pedro (no code reviews), 2015/10/29 22:05:29]

I changed comments to reflect that.

+        tapPeekingBarToExpandAndAssert();
+        assertContentViewCoreCreated();
+        assertContentViewCoreVisible();
+        assertEquals(1, mFakeServer.getLoadedUrlCount());
+
+        // Close the Panel and check that the Content was destroyed and nothing else was loaded.
+        tapBasePageToClosePanel();
+        assertNoContentViewCore();
+        assertEquals(1, mFakeServer.getLoadedUrlCount());

[Donn Denman, 2015/10/27 19:18:51]

I don't think we need this line either.

[pedro (no code reviews), 2015/10/29 22:05:29]

Done.

+    }
+
+    /**
+     * Tests swiping the overlay open, after an initial tap that activates the peeking card.

[Donn Denman, 2015/10/27 19:18:52]

Nit: update to say what were testing, rather than how we're testing it.  The
title does that pretty well!  :-)

[pedro (no code reviews), 2015/10/29 22:05:29]

Done.

+     */
+    @SmallTest
+    @Feature({"ContextualSearch"})
+    @Restriction(RESTRICTION_TYPE_NON_LOW_END_DEVICE)
+    public void testTapMultipleSwipeOnlyLoadsContentOnce()
+            throws InterruptedException, TimeoutException {
+        // Simulate a tap and make sure Content is not visible.
+        simulateTapSearch("search");
+        assertContentViewCoreCreated();
+        assertContentViewCoreNotVisible();
+        assertNeverCalledContentViewCoreOnShow();
+        assertEquals(1, mFakeServer.getLoadedUrlCount());
+
+        // Swipe Panel up and check that the Content is visible and nothing else was loaded.
+        swipePanelUp();

[Donn Denman, 2015/10/27 19:18:51]

Here's an example of where we could make these tests more readable.  We should
have a single method swipePanelUpAndAssert, or maybe even update swipePanelUp to
always assert that it did expand.

[pedro (no code reviews), 2015/10/29 22:05:29]

I agree but I think this belongs to a separate CL, since this one is already too
big and these changes would require changing existing tests, which I'm trying to
avoid to do now.

Also, I think we should have a small number of helper methods and use them
consistently. I think we have too many methods, and variations of similar
methods.

I already have a TODO to address that.

+        waitForPanelToExpandAndAssert();
+        assertContentViewCoreVisible();
+        assertEquals(1, mFakeServer.getLoadedUrlCount());
+
+        // Swipe Panel down and check that the Content remains visible and nothing else was loaded.
+        swipePanelDown();

[Donn Denman, 2015/10/27 19:18:52]

Here too, it would be nicer to have just one action line and one or more asserts
rather than the boilerplate swipePanelDown followed by
waitForPanelToPeekAndAssert.

[pedro (no code reviews), 2015/10/29 22:05:29]

Same as above.

+        waitForPanelToPeekAndAssert();
+        assertContentViewCoreVisible();
+        assertEquals(1, mFakeServer.getLoadedUrlCount());
+
+        // Swipe Panel up and check that the Content remains visible and nothing else was loaded.
+        swipePanelUp();
+        waitForPanelToExpandAndAssert();
+        assertContentViewCoreVisible();
+        assertEquals(1, mFakeServer.getLoadedUrlCount());
+
+        // Close the Panel and check that the Content was destroyed and nothing else was loaded.
+        tapBasePageToClosePanel();
+        assertNoContentViewCore();
+        assertEquals(1, mFakeServer.getLoadedUrlCount());
+    }
+
+    /**
+     * Tests swiping the overlay open, after an initial long-press that activates the peeking card,
+     * followed by closing the panel.
+     */
+    @SmallTest
+    @Feature({"ContextualSearch"})
+    @Restriction(RESTRICTION_TYPE_NON_LOW_END_DEVICE)
+    public void testLongPressMultipleSwipeOnlyLoadsContentOnce()
+            throws InterruptedException, TimeoutException {
+        // Simulate a long press and make sure no Content is created.
+        simulateLongPressSearch("search");
+        assertNoContentViewCore();
+        assertNoSearchesLoaded();
+
+        // Expand Panel and check that the Content is now created and visible.
+        swipePanelUp();
+        waitForPanelToExpandAndAssert();
+        assertContentViewCoreCreated();
+        assertContentViewCoreVisible();
+        assertEquals(1, mFakeServer.getLoadedUrlCount());
+
+        // Swipe Panel down and check that the Content remains visible and nothing else was loaded.
+        swipePanelDown();
+        waitForPanelToPeekAndAssert();
+        assertContentViewCoreVisible();
+        assertEquals(1, mFakeServer.getLoadedUrlCount());
+
+        // Swipe Panel up and check that the Content remains visible and nothing else was loaded.
+        swipePanelUp();
+        waitForPanelToExpandAndAssert();
+        assertContentViewCoreVisible();
+        assertEquals(1, mFakeServer.getLoadedUrlCount());
+
+        // Close the Panel and check that the Content was destroyed and nothing else was loaded.
+        tapBasePageToClosePanel();
+        assertNoContentViewCore();
+        assertEquals(1, mFakeServer.getLoadedUrlCount());
+    }
+
+    /**
+     * Tests that chained tap searches create new Content.
+     */
+    @SmallTest
+    @Feature({"ContextualSearch"})
+    @Restriction(RESTRICTION_TYPE_NON_LOW_END_DEVICE)
+    public void testChainedSearchCreatesNewContent()
+            throws InterruptedException, TimeoutException {
+        // Simulate a tap and make sure Content is not visible.
+        simulateTapSearch("search");
+        assertContentViewCoreCreated();

[Donn Denman, 2015/10/27 19:18:52]

Maybe these three can be combined?  I would think that
assertNeverCalledContentViewCoreOnShow would imply that it's not visible, and
probably that it was created.

[pedro (no code reviews), 2015/10/29 22:05:29]

Good point. There's now a assertContentViewCoreCreatedButNeverMadeVisible()
which makes three asserts.

I also noticed that the assertNeverCalledContentViewCoreOnShow() wasn't actually
testing what it was supposed to test. In theory a content could be created, made
visible, and destroyed, then another content is created, and we would not know
that the first content was made visible. So I fixed that as well, and now the
FakeServer keeps track if onShow() was ever called in any ContentViewCore.

+        assertContentViewCoreNotVisible();
+        assertNeverCalledContentViewCoreOnShow();
+        assertEquals(1, mFakeServer.getLoadedUrlCount());
+        ContentViewCore cvc1 = getPanelContentViewCore();
+
+        waitToPreventDoubleTapRecognition();
+
+        // Simulate a new tap and make sure a new Content is created.
+        simulateTapSearch("term");
+        assertContentViewCoreCreated();
+        assertContentViewCoreNotVisible();
+        assertNeverCalledContentViewCoreOnShow();
+        assertEquals(2, mFakeServer.getLoadedUrlCount());
+        ContentViewCore cvc2 = getPanelContentViewCore();
+        assertNotSame(cvc1, cvc2);
+
+        waitToPreventDoubleTapRecognition();
+
+        // Simulate a new tap and make sure a new Content is created.
+        simulateTapSearch("resolution");
+        assertContentViewCoreCreated();
+        assertContentViewCoreNotVisible();
+        assertNeverCalledContentViewCoreOnShow();
+        assertEquals(3, mFakeServer.getLoadedUrlCount());
+        ContentViewCore cvc3 = getPanelContentViewCore();
+        assertNotSame(cvc2, cvc3);
+
+        // Close the Panel and check that the Content was destroyed and nothing else was loaded.
+        tapBasePageToClosePanel();
+        assertNoContentViewCore();
+        assertEquals(3, mFakeServer.getLoadedUrlCount());
+    }
+
+    /**
+     * Tests that chained searches load correctly.
+     */
+    @SmallTest
+    @Feature({"ContextualSearch"})
+    @Restriction({RESTRICTION_TYPE_NON_LOW_END_DEVICE})
+    public void testChainedSearchLoadsContentCorrectly()

[Donn Denman, 2015/10/27 19:18:51]

nit: I'd rename this to something like testChainedSearchLoadsCorrectSearchTerm,
since we can't really check everything associated with loading content
correctly.

[pedro (no code reviews), 2015/10/29 22:05:29]

Done.

+            throws InterruptedException, TimeoutException {
+        // Simulate a tap and make sure Content is not visible.
+        simulateTapSearch("search");
+        assertContentViewCoreCreated();
+        assertContentViewCoreNotVisible();
+        assertNeverCalledContentViewCoreOnShow();
+        assertEquals(1, mFakeServer.getLoadedUrlCount());
+        ContentViewCore cvc1 = getPanelContentViewCore();
+
+        // Swipe Panel up and check that the Content is visible and nothing else was loaded.
+        swipePanelUp();
+        waitForPanelToExpandAndAssert();
+        assertContentViewCoreVisible();
+        assertEquals(1, mFakeServer.getLoadedUrlCount());
+
+        // Swipe Panel down and check that the Content remains visible and nothing else was loaded.
+        swipePanelDown();
+        waitForPanelToPeekAndAssert();
+        assertContentViewCoreVisible();
+        assertEquals(1, mFakeServer.getLoadedUrlCount());
+
+        waitToPreventDoubleTapRecognition();
+
+        // Now simulate a long press, without closing the Panel first.

[Donn Denman, 2015/10/27 19:18:52]

Nit: change "without closing the Panel first" to "leaving the Panel peeking"?

[pedro (no code reviews), 2015/10/29 22:05:28]

Done.

+        simulateLongPressSearch("resolution");
+
+        // Swipe Panel up and confirm it loaded the newly selected text in a new Content.
+        swipePanelUp();
+        waitForPanelToExpandAndAssert();
+        assertContentViewCoreCreated();
+        assertContentViewCoreVisible();
+        assertEquals(2, mFakeServer.getLoadedUrlCount());
+        assertLoadedSearchTermMatches("Resolution");
+        ContentViewCore cvc2 = getPanelContentViewCore();
+        assertNotSame(cvc1, cvc2);
+
+        // Close the Panel and check that the Content was destroyed and nothing else was loaded.
+        tapBasePageToClosePanel();
+        assertNoContentViewCore();
+        assertEquals(2, mFakeServer.getLoadedUrlCount());
+    }
+
+    /**
+     * Tests that chained searches make Content visible when opening the Panel.
+     */
+    @SmallTest
+    @Feature({"ContextualSearch"})
+    @Restriction(RESTRICTION_TYPE_NON_LOW_END_DEVICE)
+    public void testChainedSearchContentVisibility()
+            throws InterruptedException, TimeoutException {
+        // Simulate a tap and make sure Content is not visible.
+        simulateTapSearch("search");
+        assertContentViewCoreCreated();
+        assertContentViewCoreNotVisible();
+        assertNeverCalledContentViewCoreOnShow();
+        assertEquals(1, mFakeServer.getLoadedUrlCount());
+        ContentViewCore cvc1 = getPanelContentViewCore();
+
+        waitToPreventDoubleTapRecognition();
+
+        // Now simulate a long press, without closing the Panel first.
+        simulateLongPressSearch("resolution");
+        assertNeverCalledContentViewCoreOnShow();
+        assertEquals(1, mFakeServer.getLoadedUrlCount());
+
+        // Swipe Panel up and confirm it loaded the newly selected text in a new Content.
+        swipePanelUp();
+        waitForPanelToExpandAndAssert();
+        assertContentViewCoreCreated();
+        assertContentViewCoreVisible();
+        assertEquals(2, mFakeServer.getLoadedUrlCount());
+        assertLoadedSearchTermMatches("Resolution");
+        ContentViewCore cvc2 = getPanelContentViewCore();
+        assertNotSame(cvc1, cvc2);
+    }
+
+    //============================================================================================
+    // History Removal Tests
+    //============================================================================================
+
+    /**
+     * Tests that a tap followed by closing the Panel removes the loaded URL from history.
+     */
+    @SmallTest
+    @Feature({"ContextualSearch"})
+    @Restriction(RESTRICTION_TYPE_NON_LOW_END_DEVICE)
+    public void testTapCloseRemovedFromHistory()
+            throws InterruptedException, TimeoutException {
+        // Simulate a tap and make sure a URL was loaded.
+        simulateTapSearch("search");
+        assertEquals(1, mFakeServer.getLoadedUrlCount());
+        String url = mFakeServer.getLoadedUrl();
+
+        // Close the Panel without seeing the Content.
+        tapBasePageToClosePanel();
+
+        // Now check that the URL has been removed from history.
+        assertTrue(mFakeServer.hasRemovedUrl(url));
+    }
+
+    /**
+     * Tests that a tap followed by opening the Panel does not remove the loaded URL from history.
+     */
+    @SmallTest
+    @Feature({"ContextualSearch"})
+    @Restriction(RESTRICTION_TYPE_NON_LOW_END_DEVICE)
+    public void testTapExpandNotRemovedFromHistory()
+            throws InterruptedException, TimeoutException {
+        // Simulate a tap and make sure a URL was loaded.
+        simulateTapSearch("search");
+        assertEquals(1, mFakeServer.getLoadedUrlCount());
+        String url = mFakeServer.getLoadedUrl();
+
+        // Expand Panel so that the Content becomes visible.
+        tapPeekingBarToExpandAndAssert();
+
+        // Close the Panel.
+        tapBasePageToClosePanel();
+
+        // Now check that the URL has not been removed from history, since the Content was seen.
+        assertFalse(mFakeServer.hasRemovedUrl(url));
+    }
+
+    /**
+     * Tests that chained searches without opening the Panel removes all loaded URLs from history.
+     */
+    @SmallTest
+    @Feature({"ContextualSearch"})
+    @Restriction(RESTRICTION_TYPE_NON_LOW_END_DEVICE)
+    public void testChainedTapsRemovedFromHistory()
+            throws InterruptedException, TimeoutException {
+        // Simulate a tap and make sure a URL was loaded.
+        simulateTapSearch("search");
+        assertEquals(1, mFakeServer.getLoadedUrlCount());

[Donn Denman, 2015/10/27 19:18:52]

Wouldn't it be better to skip this check, and instead check that the url was not
null when we get url1?

[pedro (no code reviews), 2015/10/29 22:05:29]

Done.

+        String url1 = mFakeServer.getLoadedUrl();
+
+        waitToPreventDoubleTapRecognition();
+
+        // Simulate another tap and make sure another URL was loaded.
+        simulateTapSearch("term");
+        assertEquals(2, mFakeServer.getLoadedUrlCount());

[Donn Denman, 2015/10/27 19:18:52]

Similarly, I'd skip this and check that url2 is not the same as url1 here.

[pedro (no code reviews), 2015/10/29 22:05:28]

Done.

+        String url2 = mFakeServer.getLoadedUrl();
+
+        waitToPreventDoubleTapRecognition();
+
+        // Simulate another tap and make sure another URL was loaded.
+        simulateTapSearch("resolution");
+        assertEquals(3, mFakeServer.getLoadedUrlCount());

[Donn Denman, 2015/10/27 19:18:51]

I'd move this down to the bottom with the other final asserts.

[pedro (no code reviews), 2015/10/29 22:05:29]

Done.

+        String url3 = mFakeServer.getLoadedUrl();

[Donn Denman, 2015/10/27 19:18:52]

Similarly check that url3 is unique, otherwise the hasRemovedUrl calls might not
tell us what we need to know.

[pedro (no code reviews), 2015/10/29 22:05:29]

Done.

+
+        // Close the Panel without seeing any Content.
+        tapBasePageToClosePanel();
+
+        // Now check that all three URLs have been removed from history.
+        assertTrue(mFakeServer.hasRemovedUrl(url1));
+        assertTrue(mFakeServer.hasRemovedUrl(url2));
+        assertTrue(mFakeServer.hasRemovedUrl(url3));
+    }
 }