Add a test for FindInPage after selecting text.

Add a test for FindInPage after selecting text.

This adds a test case to verify that searching in text will start
at the currently selected text and will not lose context if the
search is later refined.

BUG=129552
TEST=browser_tests


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=155870

[Finnur, 2012-06-14 10:25:18]

Just nits and a question. If you remove the unnecessary call (see comment) and
everything works then consider this my LGTM.

https://chromiumcodereview.appspot.com/10533132/diff/1/chrome/browser/ui/find...
File chrome/browser/ui/find_bar/find_bar_host_browsertest.cc (right):

https://chromiumcodereview.appspot.com/10533132/diff/1/chrome/browser/ui/find...
chrome/browser/ui/find_bar/find_bar_host_browsertest.cc:391: // This tests that
we start searching after selected text
nit: End general comments with a period.

https://chromiumcodereview.appspot.com/10533132/diff/1/chrome/browser/ui/find...
chrome/browser/ui/find_bar/find_bar_host_browsertest.cc:398: // Search for a
text that exists within a link on the page.
This comment looks out of place.

https://chromiumcodereview.appspot.com/10533132/diff/1/chrome/browser/ui/find...
chrome/browser/ui/find_bar/find_bar_host_browsertest.cc:433:
tab->find_tab_helper()->StopFinding(FindBarController::kKeepSelectionOnPage);
I don't think this is needed for this test. Is it?

[mkosiba (inactive), 2012-06-14 13:46:30]

https://chromiumcodereview.appspot.com/10533132/diff/1/chrome/browser/ui/find...
File chrome/browser/ui/find_bar/find_bar_host_browsertest.cc (right):

https://chromiumcodereview.appspot.com/10533132/diff/1/chrome/browser/ui/find...
chrome/browser/ui/find_bar/find_bar_host_browsertest.cc:391: // This tests that
we start searching after selected text
On 2012/06/14 10:25:18, Finnur wrote:
> nit: End general comments with a period.

Done.

https://chromiumcodereview.appspot.com/10533132/diff/1/chrome/browser/ui/find...
chrome/browser/ui/find_bar/find_bar_host_browsertest.cc:398: // Search for a
text that exists within a link on the page.
On 2012/06/14 10:25:18, Finnur wrote:
> This comment looks out of place.

Done.

https://chromiumcodereview.appspot.com/10533132/diff/1/chrome/browser/ui/find...
chrome/browser/ui/find_bar/find_bar_host_browsertest.cc:433:
tab->find_tab_helper()->StopFinding(FindBarController::kKeepSelectionOnPage);
On 2012/06/14 10:25:18, Finnur wrote:
> I don't think this is needed for this test. Is it?

Done.

[Finnur, 2012-06-14 14:35:35]

LGTM.

[commit-bot: I haz the power, 2012-06-14 14:37:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mkosiba@chromium.org/10533132/4001

[commit-bot: I haz the power, 2012-06-14 14:37:34]

Presubmit check for 10533132-4001 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    chrome/browser/ui

Presubmit checks took 4.6s to calculate.

[mkosiba (inactive), 2012-06-14 15:52:50]

Peter,
    I need LGTM from an OWNER. Please take a look.

Thanks!
    Martin

[Peter Kasting, 2012-09-05 21:22:59]

Sorry I never saw this!  In the future please feel free to send more harassment
if more than a couple days pass without comment... hopefully this isn't totally
obsolete now.

OWNERS LGTM

http://codereview.chromium.org/10533132/diff/4001/chrome/browser/ui/find_bar/...
File chrome/browser/ui/find_bar/find_bar_host_browsertest.cc (right):

http://codereview.chromium.org/10533132/diff/4001/chrome/browser/ui/find_bar/...
chrome/browser/ui/find_bar/find_bar_host_browsertest.cc:395: GURL url =
GetURL(kStartAfterSelection);
Nit: Can inline into next statement

http://codereview.chromium.org/10533132/diff/4001/chrome/browser/ui/find_bar/...
chrome/browser/ui/find_bar/find_bar_host_browsertest.cc:399: ASSERT_TRUE(NULL !=
tab);
Nit: "tab != NULL" (or ASSERT_FALSE(tab == NULL)) is preferred

http://codereview.chromium.org/10533132/diff/4001/chrome/browser/ui/find_bar/...
chrome/browser/ui/find_bar/find_bar_host_browsertest.cc:406: L"",
Nit: Use std::wstring()

http://codereview.chromium.org/10533132/diff/4001/chrome/browser/ui/find_bar/...
chrome/browser/ui/find_bar/find_bar_host_browsertest.cc:412: EXPECT_EQ(4,
FindInPageWchar(tab,
Nit: Feel free to collapse args onto one line (3 places)

http://codereview.chromium.org/10533132/diff/4001/chrome/browser/ui/find_bar/...
chrome/browser/ui/find_bar/find_bar_host_browsertest.cc:417: // Refine the
search, current active match should not change (bug 129552).
Nit: Don't bother with the bug number.

[mkosiba (inactive), 2012-09-10 18:45:23]

http://codereview.chromium.org/10533132/diff/4001/chrome/browser/ui/find_bar/...
File chrome/browser/ui/find_bar/find_bar_host_browsertest.cc (right):

http://codereview.chromium.org/10533132/diff/4001/chrome/browser/ui/find_bar/...
chrome/browser/ui/find_bar/find_bar_host_browsertest.cc:395: GURL url =
GetURL(kStartAfterSelection);
On 2012/09/05 21:22:59, Peter Kasting wrote:
> Nit: Can inline into next statement

Done.

http://codereview.chromium.org/10533132/diff/4001/chrome/browser/ui/find_bar/...
chrome/browser/ui/find_bar/find_bar_host_browsertest.cc:399: ASSERT_TRUE(NULL !=
tab);
On 2012/09/05 21:22:59, Peter Kasting wrote:
> Nit: "tab != NULL" (or ASSERT_FALSE(tab == NULL)) is preferred

Done.

http://codereview.chromium.org/10533132/diff/4001/chrome/browser/ui/find_bar/...
chrome/browser/ui/find_bar/find_bar_host_browsertest.cc:406: L"",
On 2012/09/05 21:22:59, Peter Kasting wrote:
> Nit: Use std::wstring()

Done.

http://codereview.chromium.org/10533132/diff/4001/chrome/browser/ui/find_bar/...
chrome/browser/ui/find_bar/find_bar_host_browsertest.cc:412: EXPECT_EQ(4,
FindInPageWchar(tab,
On 2012/09/05 21:22:59, Peter Kasting wrote:
> Nit: Feel free to collapse args onto one line (3 places)

Done.

http://codereview.chromium.org/10533132/diff/4001/chrome/browser/ui/find_bar/...
chrome/browser/ui/find_bar/find_bar_host_browsertest.cc:417: // Refine the
search, current active match should not change (bug 129552).
On 2012/09/05 21:22:59, Peter Kasting wrote:
> Nit: Don't bother with the bug number.

Done.

[commit-bot: I haz the power, 2012-09-10 20:36:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mkosiba@chromium.org/10533132/12001

[commit-bot: I haz the power, 2012-09-10 23:45:27]

Change committed as 155870