Browser and unit tests for the SB interstitial V3

This makes the Safe Browsing interstitial unit & browser tests work for both the V2 and V3 versions. For the unit tests, I did this by using typed tests. Typed tests aren't supported for browser tests, so for the browser tests I moved the tests into methods that could be invoked twice.

Also includes a small bugfix in a string that was causing a failure.

BUG=381260
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=277749

[felt, 2014-06-13 18:46:43]

Hi Matt, here are tests for the V3. PTAL.

[felt, 2014-06-13 20:04:54]

On 2014/06/13 18:46:43, felt wrote:
> Hi Matt, here are tests for the V3. PTAL.

fyi, tests are failing on bots because i'm ahead of LKGR.

[mattm, 2014-06-13 20:20:31]

https://codereview.chromium.org/330753002/diff/80001/chrome/browser/safe_brow...
File chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc (right):

https://codereview.chromium.org/330753002/diff/80001/chrome/browser/safe_brow...
chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc:673: // TEST
CASES.
Could you use parameterized tests to avoid having to move all the test bodies
into the class? (IN_PROC_BROWSER_TEST_P)

https://codereview.chromium.org/330753002/diff/80001/chrome/browser/safe_brow...
chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc:973:
IN_PROC_BROWSER_TEST_F(SafeBrowsingInterstitialTestV2,
You'll need to update the test names in
tools/valgrind/gtest_exclude/browser_tests.gtest-memcheck.txt (and add the v3
versions)

[felt, 2014-06-14 01:08:04]

https://codereview.chromium.org/330753002/diff/80001/chrome/browser/safe_brow...
File chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc (right):

https://codereview.chromium.org/330753002/diff/80001/chrome/browser/safe_brow...
chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc:673: // TEST
CASES.
On 2014/06/13 20:20:31, mattm wrote:
> Could you use parameterized tests to avoid having to move all the test bodies
> into the class? (IN_PROC_BROWSER_TEST_P)

Sweet, I didn't know about IN_PROC_BROWSER_TEST_P. Done, much simpler now.

https://codereview.chromium.org/330753002/diff/80001/chrome/browser/safe_brow...
chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc:973:
IN_PROC_BROWSER_TEST_F(SafeBrowsingInterstitialTestV2,
On 2014/06/13 20:20:31, mattm wrote:
> You'll need to update the test names in
> tools/valgrind/gtest_exclude/browser_tests.gtest-memcheck.txt (and add the v3
> versions)

Done.

[mattm, 2014-06-14 02:00:20]

https://codereview.chromium.org/330753002/diff/120001/chrome/browser/safe_bro...
File chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc (right):

https://codereview.chromium.org/330753002/diff/120001/chrome/browser/safe_bro...
chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc:937:
SendCommand("proceed");
Why is this SendCommand instead of ClickAndWaitForDetach?

https://codereview.chromium.org/330753002/diff/120001/tools/valgrind/gtest_ex...
File tools/valgrind/gtest_exclude/browser_tests.gtest-memcheck.txt (right):

https://codereview.chromium.org/330753002/diff/120001/tools/valgrind/gtest_ex...
tools/valgrind/gtest_exclude/browser_tests.gtest-memcheck.txt:56:
SafeBrowsingInterstitialVersions/SafeBrowsingBlockingPageBrowserTest.ProceedDisabled
hm, there's no 2 or 3 in the test names?

[felt, 2014-06-14 02:10:27]

https://codereview.chromium.org/330753002/diff/120001/chrome/browser/safe_bro...
File chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc (right):

https://codereview.chromium.org/330753002/diff/120001/chrome/browser/safe_bro...
chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc:937:
SendCommand("proceed");
On 2014/06/14 02:00:20, mattm wrote:
> Why is this SendCommand instead of ClickAndWaitForDetach?

When a warning is non-overridable, I don't attach a click handler to the proceed
link. As a result, there's no click handler to trigger for the test. However, my
understanding is that the point of this test is to check that proceeding doesn't
actually work -- which SendCommand should also test.

https://codereview.chromium.org/330753002/diff/120001/tools/valgrind/gtest_ex...
File tools/valgrind/gtest_exclude/browser_tests.gtest-memcheck.txt (right):

https://codereview.chromium.org/330753002/diff/120001/tools/valgrind/gtest_ex...
tools/valgrind/gtest_exclude/browser_tests.gtest-memcheck.txt:56:
SafeBrowsingInterstitialVersions/SafeBrowsingBlockingPageBrowserTest.ProceedDisabled
On 2014/06/14 02:00:20, mattm wrote:
> hm, there's no 2 or 3 in the test names?

They are named like:

SafeBrowsingInterstitialVersions/SafeBrowsingBlockingPageBrowserTest.PhishingProceed/0
SafeBrowsingInterstitialVersions/SafeBrowsingBlockingPageBrowserTest.PhishingProceed/1

With 0 being V2 and 1 being V3. I don't know how these rules work; I assumed I
didn't need the last /0 or /1, but I don't know for sure. (I figure if I see
failures I can twiddle with it.) I've added an * to the end.

[mattm, 2014-06-16 20:13:26]

lgtm

[felt, 2014-06-17 02:38:38]

The CQ bit was checked by felt@chromium.org

[commit-bot: I haz the power, 2014-06-17 02:41:11]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/felt@chromium.org/330753002/200001

[felt, 2014-06-17 05:47:22]

The CQ bit was unchecked by felt@chromium.org

[felt, 2014-06-17 05:48:23]

hi bernhard, can I get an  owners review for two very small changes to
chrome/browser/resources/ssl/interstitial_v2.*?

[Bernhard Bauer, 2014-06-17 08:22:44]

c/b/resources LGTM

[felt, 2014-06-17 13:09:14]

The CQ bit was checked by felt@chromium.org

[commit-bot: I haz the power, 2014-06-17 13:09:43]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/felt@chromium.org/330753002/200001

[commit-bot: I haz the power, 2014-06-17 13:14:32]

Change committed as 277749

[mattm, 2014-06-17 18:38:49]

https://codereview.chromium.org/330753002/diff/200001/chrome/browser/safe_bro...
File chrome/browser/safe_browsing/safe_browsing_blocking_page.cc (right):

https://codereview.chromium.org/330753002/diff/200001/chrome/browser/safe_bro...
chrome/browser/safe_browsing/safe_browsing_blocking_page.cc:498: LOG(ERROR) <<
"5";
looks like some debug logging snuck in before this was committed

https://codereview.chromium.org/330753002/diff/200001/chrome/browser/safe_bro...
File chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc
(right):

https://codereview.chromium.org/330753002/diff/200001/chrome/browser/safe_bro...
chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc:714:
DISABLED_MalwareReportsTransitionEnabled) {
and this got marked disabled?

[felt, 2014-06-17 18:40:14]

https://codereview.chromium.org/330753002/diff/200001/chrome/browser/safe_bro...
File chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc
(right):

https://codereview.chromium.org/330753002/diff/200001/chrome/browser/safe_bro...
chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc:714:
DISABLED_MalwareReportsTransitionEnabled) {
On 2014/06/17 18:38:48, mattm wrote:
> and this got marked disabled?

This had to be disabled because the checkbox isn't implemented yet. 339503004
enables it.

[felt, 2014-06-17 18:42:25]

https://codereview.chromium.org/330753002/diff/200001/chrome/browser/safe_bro...
File chrome/browser/safe_browsing/safe_browsing_blocking_page.cc (right):

https://codereview.chromium.org/330753002/diff/200001/chrome/browser/safe_bro...
chrome/browser/safe_browsing/safe_browsing_blocking_page.cc:498: LOG(ERROR) <<
"5";
On 2014/06/17 18:38:48, mattm wrote:
> looks like some debug logging snuck in before this was committed

pls review fix: https://codereview.chromium.org/338653004/