Regression test and clean up.

Regression test and clean up.

Added a test that would have caught the zero-suggest issue. Also removed the
now unused API.

BUG=604076
Committed: https://crrev.com/661c3e6e7786eb6c9071a91427ecb4c07e382b65
Cr-Commit-Position: refs/heads/master@{#390811}

[Maria, 2016-04-29 22:14:30]

mariakhomenko@chromium.org changed reviewers:
+ tedchoc@chromium.org

[Ted C, 2016-04-29 22:24:51]

https://codereview.chromium.org/1941473002/diff/1/chrome/android/javatests/sr...
File
chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/OmniboxTest.java
(right):

https://codereview.chromium.org/1941473002/diff/1/chrome/android/javatests/sr...
chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/OmniboxTest.java:130:
startMainActivityWithURL("http://www.example.com/");
We already call start startMainActivityOnBlankPage(); in startMainActivity.  Why
do we need a URL here?  Does about:blank not trigger zero suggest?

In general, this looks like it requires network access (even if it doesn't).  It
would be good to find something that can load a local URL.

https://codereview.chromium.org/1941473002/diff/1/chrome/android/javatests/sr...
chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/OmniboxTest.java:159:
CriteriaHelper.pollUiThread(Criteria.equals(Boolean.FALSE, new
Callable<Boolean>() {
Do we actually want to poll this?

Seems like we just want assertFalse instead.

https://codereview.chromium.org/1941473002/diff/1/chrome/test/android/javates...
File
chrome/test/android/javatests/src/org/chromium/chrome/test/util/OmniboxTestUtils.java
(right):

https://codereview.chromium.org/1941473002/diff/1/chrome/test/android/javates...
chrome/test/android/javatests/src/org/chromium/chrome/test/util/OmniboxTestUtils.java:186:
mStartAutocompleteCalled = false;
do we want this?  Seems like I could still break this by calling stop (if focus
causes a start and stop we still wouldn't have zero suggest and the test would
pass)

maybe instead we want a separate method to clear this boolean than to trigger it
off of stop

[Maria, 2016-04-29 22:42:14]

https://codereview.chromium.org/1941473002/diff/1/chrome/android/javatests/sr...
File
chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/OmniboxTest.java
(right):

https://codereview.chromium.org/1941473002/diff/1/chrome/android/javatests/sr...
chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/OmniboxTest.java:130:
startMainActivityWithURL("http://www.example.com/");
On 2016/04/29 22:24:51, Ted C wrote:
> We already call start startMainActivityOnBlankPage(); in startMainActivity. 
Why
> do we need a URL here?  Does about:blank not trigger zero suggest?
> 
> In general, this looks like it requires network access (even if it doesn't). 
It
> would be good to find something that can load a local URL.

The reason we need this is the UI affordance where focused omnibox shows http://
in the URL while unfocused doesn't. Then the text field changes on focus and
triggers the bug. It doesn't happen with about:blank. Wouldn't work with file://
either since we don't hide it.

https://codereview.chromium.org/1941473002/diff/1/chrome/android/javatests/sr...
chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/OmniboxTest.java:159:
CriteriaHelper.pollUiThread(Criteria.equals(Boolean.FALSE, new
Callable<Boolean>() {
On 2016/04/29 22:24:51, Ted C wrote:
> Do we actually want to poll this?
> 
> Seems like we just want assertFalse instead.

Done.

https://codereview.chromium.org/1941473002/diff/1/chrome/test/android/javates...
File
chrome/test/android/javatests/src/org/chromium/chrome/test/util/OmniboxTestUtils.java
(right):

https://codereview.chromium.org/1941473002/diff/1/chrome/test/android/javates...
chrome/test/android/javatests/src/org/chromium/chrome/test/util/OmniboxTestUtils.java:186:
mStartAutocompleteCalled = false;
On 2016/04/29 22:24:51, Ted C wrote:
> do we want this?  Seems like I could still break this by calling stop (if
focus
> causes a start and stop we still wouldn't have zero suggest and the test would
> pass)
> 
> maybe instead we want a separate method to clear this boolean than to trigger
it
> off of stop

Done. Just removing it for now. If at some point it needs to be reset/cleaned up
we can do it then.

[Ted C, 2016-04-29 22:49:22]

https://codereview.chromium.org/1941473002/diff/20001/chrome/android/javatest...
File
chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/OmniboxTest.java
(right):

https://codereview.chromium.org/1941473002/diff/20001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/OmniboxTest.java:130:
startMainActivityWithURL("http://www.example.com/");
We should be using some sort of loadUrl then instead of startMainActivity....

Can we try to use the EmbeddedTestServer instead (I think it will keep the
http:// bit)

Something like:
<test server>.getURL("/chrome/test/data/android/test.html")

[Maria, 2016-04-29 23:16:29]

PTAL

https://codereview.chromium.org/1941473002/diff/20001/chrome/android/javatest...
File
chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/OmniboxTest.java
(right):

https://codereview.chromium.org/1941473002/diff/20001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/OmniboxTest.java:130:
startMainActivityWithURL("http://www.example.com/");
On 2016/04/29 22:49:22, Ted C wrote:
> We should be using some sort of loadUrl then instead of startMainActivity....
> 
> Can we try to use the EmbeddedTestServer instead (I think it will keep the
> http:// bit)
> 
> Something like:
> <test server>.getURL("/chrome/test/data/android/test.html")

I want to avoid loading an extra page. Fixed this to just set the URL we want on
the URL bar, which is all we really need.

[Ted C, 2016-04-29 23:22:45]

On 2016/04/29 23:16:29, Maria wrote:
> PTAL
> 
>
https://codereview.chromium.org/1941473002/diff/20001/chrome/android/javatest...
> File
>
chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/OmniboxTest.java
> (right):
> 
>
https://codereview.chromium.org/1941473002/diff/20001/chrome/android/javatest...
>
chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/OmniboxTest.java:130:
> startMainActivityWithURL("http://www.example.com/");
> On 2016/04/29 22:49:22, Ted C wrote:
> > We should be using some sort of loadUrl then instead of
startMainActivity....
> > 
> > Can we try to use the EmbeddedTestServer instead (I think it will keep the
> > http:// bit)
> > 
> > Something like:
> > <test server>.getURL("/chrome/test/data/android/test.html")
> 
> I want to avoid loading an extra page. Fixed this to just set the URL we want
on
> the URL bar, which is all we really need.

lgtm

[Maria, 2016-04-29 23:26:48]

The CQ bit was checked by mariakhomenko@chromium.org

[commit-bot: I haz the power, 2016-04-29 23:27:10]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1941473002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1941473002/40001

[commit-bot: I haz the power, 2016-04-30 00:07:35]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-04-30 17:30:19]

Patchset 3 (id:??) landed as
https://crrev.com/661c3e6e7786eb6c9071a91427ecb4c07e382b65
Cr-Commit-Position: refs/heads/master@{#390811}

[commit-bot: I haz the power, 2016-04-30 18:03:36]

Description was changed from

==========
Regression test and clean up.

Added a test that would have caught the zero-suggest issue. Also removed the
now unused API.

BUG=604076
==========

to

==========
Regression test and clean up.

Added a test that would have caught the zero-suggest issue. Also removed the
now unused API.

BUG=604076

Committed: https://crrev.com/661c3e6e7786eb6c9071a91427ecb4c07e382b65
Cr-Commit-Position: refs/heads/master@{#390811}
==========