CrOS EULA: added 2 browser tests

CrOS EULA: added 2 browser tests.
LoadOnline tries to load online chrome://terms.
LoadOffline disconnects host and shows offline version.


BUG=236068
TBR=estade@chromium.org

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

[vasilii, 2013-05-07 12:59:02]

Hi Nikita,

please review the tests.

[Nikita (slow), 2013-05-07 17:20:25]

https://codereview.chromium.org/14746014/diff/1/chrome/browser/chromeos/login...
File chrome/browser/chromeos/login/eula_browsertest.cc (right):

https://codereview.chromium.org/14746014/diff/1/chrome/browser/chromeos/login...
chrome/browser/chromeos/login/eula_browsertest.cc:26: EXPECT_EQ(0,
ui_test_utils::FindInPage(web_contents,
This test may become flaky pretty soon. Imagine it running on a debug build on a
buildbot where different tests are executed at the same time. In that case
hitting real network may not finish in that 7 second time.

You should use MockUrlFetcher and check that it is called with the URL you
expect that to be called.

[vasilii, 2013-05-08 13:12:50]

https://codereview.chromium.org/14746014/diff/1/chrome/browser/chromeos/login...
File chrome/browser/chromeos/login/eula_browsertest.cc (right):

https://codereview.chromium.org/14746014/diff/1/chrome/browser/chromeos/login...
chrome/browser/chromeos/login/eula_browsertest.cc:26: EXPECT_EQ(0,
ui_test_utils::FindInPage(web_contents,
On 2013/05/07 17:20:25, Nikita Kostylev wrote:
> This test may become flaky pretty soon. Imagine it running on a debug build on
a
> buildbot where different tests are executed at the same time. In that case
> hitting real network may not finish in that 7 second time.
> 
> You should use MockUrlFetcher and check that it is called with the URL you
> expect that to be called.

Done.

[vasilii, 2013-05-10 13:36:36]

please review

[Nikita (slow), 2013-05-13 09:25:40]

lgtm

sorry for delay - public holidays on Thu/Fri.

https://codereview.chromium.org/14746014/diff/20001/chrome/app/chromeos_strin...
File chrome/app/chromeos_strings.grdp (right):

https://codereview.chromium.org/14746014/diff/20001/chrome/app/chromeos_strin...
chrome/app/chromeos_strings.grdp:1479:
https://www.google.com/intl/[GRITLANGCODE]/chrome/eula_text.html
This changes seems needs a merge with https://codereview.chromium.org/14990003/

https://codereview.chromium.org/14746014/diff/20001/chrome/browser/chromeos/l...
File chrome/browser/chromeos/login/eula_browsertest.cc (right):

https://codereview.chromium.org/14746014/diff/20001/chrome/browser/chromeos/l...
chrome/browser/chromeos/login/eula_browsertest.cc:88: EXPECT_NE(std::string(),
body);
How about checking for some specific offline content?

[vasilii, 2013-05-13 09:52:35]

https://codereview.chromium.org/14746014/diff/20001/chrome/app/chromeos_strin...
File chrome/app/chromeos_strings.grdp (right):

https://codereview.chromium.org/14746014/diff/20001/chrome/app/chromeos_strin...
chrome/app/chromeos_strings.grdp:1479:
https://www.google.com/intl/[GRITLANGCODE]/chrome/eula_text.html
On 2013/05/13 09:25:40, Nikita Kostylev wrote:
> This changes seems needs a merge with
https://codereview.chromium.org/14990003/

Done.

https://codereview.chromium.org/14746014/diff/20001/chrome/browser/chromeos/l...
File chrome/browser/chromeos/login/eula_browsertest.cc (right):

https://codereview.chromium.org/14746014/diff/20001/chrome/browser/chromeos/l...
chrome/browser/chromeos/login/eula_browsertest.cc:88: EXPECT_NE(std::string(),
body);
On 2013/05/13 09:25:40, Nikita Kostylev wrote:
> How about checking for some specific offline content?

That would be great. But the content of chrome://terms inside test is "This
Space Intentionally Blank
In official builds this space will show the terms of service.". See "Experiment"
patch set.

[Nikita (slow), 2013-05-13 11:36:56]

On 2013/05/13 09:52:35, vasilii wrote:
>
https://codereview.chromium.org/14746014/diff/20001/chrome/app/chromeos_strin...
> File chrome/app/chromeos_strings.grdp (right):
> 
>
https://codereview.chromium.org/14746014/diff/20001/chrome/app/chromeos_strin...
> chrome/app/chromeos_strings.grdp:1479:
> https://www.google.com/intl/%5BGRITLANGCODE%5D/chrome/eula_text.html
> On 2013/05/13 09:25:40, Nikita Kostylev wrote:
> > This changes seems needs a merge with
> https://codereview.chromium.org/14990003/
> 
> Done.
> 
>
https://codereview.chromium.org/14746014/diff/20001/chrome/browser/chromeos/l...
> File chrome/browser/chromeos/login/eula_browsertest.cc (right):
> 
>
https://codereview.chromium.org/14746014/diff/20001/chrome/browser/chromeos/l...
> chrome/browser/chromeos/login/eula_browsertest.cc:88: EXPECT_NE(std::string(),
> body);
> On 2013/05/13 09:25:40, Nikita Kostylev wrote:
> > How about checking for some specific offline content?
> 
> That would be great. But the content of chrome://terms inside test is "This
> Space Intentionally Blank
> In official builds this space will show the terms of service.". See
"Experiment"
> patch set.

#if defined(GOOGLE_CHROME_BUILD)
  // check for official build content
#else
  // check for default chromium build content
#endif

[vasilii, 2013-05-13 13:48:24]

On 2013/05/13 11:36:56, Nikita Kostylev wrote:
> On 2013/05/13 09:52:35, vasilii wrote:
> >
>
https://codereview.chromium.org/14746014/diff/20001/chrome/app/chromeos_strin...
> > File chrome/app/chromeos_strings.grdp (right):
> > 
> >
>
https://codereview.chromium.org/14746014/diff/20001/chrome/app/chromeos_strin...
> > chrome/app/chromeos_strings.grdp:1479:
> > https://www.google.com/intl/%255BGRITLANGCODE%255D/chrome/eula_text.html
> > On 2013/05/13 09:25:40, Nikita Kostylev wrote:
> > > This changes seems needs a merge with
> > https://codereview.chromium.org/14990003/
> > 
> > Done.
> > 
> >
>
https://codereview.chromium.org/14746014/diff/20001/chrome/browser/chromeos/l...
> > File chrome/browser/chromeos/login/eula_browsertest.cc (right):
> > 
> >
>
https://codereview.chromium.org/14746014/diff/20001/chrome/browser/chromeos/l...
> > chrome/browser/chromeos/login/eula_browsertest.cc:88:
EXPECT_NE(std::string(),
> > body);
> > On 2013/05/13 09:25:40, Nikita Kostylev wrote:
> > > How about checking for some specific offline content?
> > 
> > That would be great. But the content of chrome://terms inside test is "This
> > Space Intentionally Blank
> > In official builds this space will show the terms of service.". See
> "Experiment"
> > patch set.
> 
> #if defined(GOOGLE_CHROME_BUILD)
>   // check for official build content
> #else
>   // check for default chromium build content
> #endif

Done.

[Nikita (slow), 2013-05-23 14:07:48]

Please land this CL so that we'll have test coverage for online EULA.

[commit-bot: I haz the power, 2013-05-23 14:09:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vasilii@chromium.org/14746014/35002

[commit-bot: I haz the power, 2013-05-23 18:39:36]

Retried try job too often on win_rel for step(s) chrome_frame_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-05-24 07:24:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vasilii@chromium.org/14746014/35002

[commit-bot: I haz the power, 2013-05-24 10:39:52]

Change committed as 202043