Switch Site Isolation Windows FYI bot to run with --isolate-extensions.

Switch Site Isolation Windows FYI bot to run with --isolate-extensions.

BUG=417518
Committed: https://crrev.com/20ba43a9f5e6fa1650b9aa67aa1e3c212f5c6fa9
Cr-Commit-Position: refs/heads/master@{#367412}

[alexmos, 2015-12-17 23:25:42]

alexmos@chromium.org changed reviewers:
+ creis@chromium.org, nick@chromium.org

[alexmos, 2015-12-17 23:25:42]

Charlie or Nick: please take a look.  I'll wait for all my other
chromium.fyi.json CLs to land first and make sure the bots go green before
landing this.

[Charlie Reis, 2015-12-17 23:50:36]

Cool!

https://codereview.chromium.org/1533953003/diff/1/testing/buildbot/chromium.f...
File testing/buildbot/chromium.fyi.json (right):

https://codereview.chromium.org/1533953003/diff/1/testing/buildbot/chromium.f...
testing/buildbot/chromium.fyi.json:5784: "test": "ash_unittests"
Is --isolate-extensions meaningful in ash_unittests?  (I'm guessing so since
ChromeOS has extensions, but I'm not certain.)

https://codereview.chromium.org/1533953003/diff/1/testing/buildbot/chromium.f...
testing/buildbot/chromium.fyi.json:5807: "--isolate-extensions"
This flag doesn't mean anything in content_browsertests.  I would suggest either
keeping this as --site-per-process (with the existing filter) or using Nick's
new --isolate-sites-for-testing flag (for some arbitrary site like *.is).

https://codereview.chromium.org/1533953003/diff/1/testing/buildbot/chromium.f...
testing/buildbot/chromium.fyi.json:5813: "--isolate-extensions"
Same for content_unittests.

https://codereview.chromium.org/1533953003/diff/1/testing/buildbot/chromium.f...
testing/buildbot/chromium.fyi.json:5819: "--isolate-extensions"
Nick tells me that even extensions_browsertests don't know about
--isolate-extensions?  :(

[ncarter (slow), 2015-12-18 20:20:20]

https://codereview.chromium.org/1533953003/diff/1/testing/buildbot/chromium.f...
File testing/buildbot/chromium.fyi.json (right):

https://codereview.chromium.org/1533953003/diff/1/testing/buildbot/chromium.f...
testing/buildbot/chromium.fyi.json:5819: "--isolate-extensions"
On 2015/12/17 23:50:36, Charlie Reis (OOO till Jan 4) wrote:
> Nick tells me that even extensions_browsertests don't know about
> --isolate-extensions?  :(

Yeah; I think --isolate-extensions only matters if you install the
ChromeContentClient/ChromeContentBrowserClient (and it would be a fair amount of
work to make that happen).

HOWEVER! You can do the following:

   --isolate-sites-for-testing=chrome-extension://*

[Charlie Reis, 2015-12-18 20:22:51]

Anyway, LGTM once those issues are fixed.  I'm excited to have test coverage on
a bot for --isolate-extensions.

[alexmos, 2015-12-18 21:15:13]

Please take another look.  I probably won't land this right now, since I'm
leaving soon and won't be around to fix the Windows bot if it breaks.  Nick, I
think you were going to be around next week -- if this looks good, maybe you can
CQ it Monday morning and watch the Windows bot?

https://codereview.chromium.org/1533953003/diff/1/testing/buildbot/chromium.f...
File testing/buildbot/chromium.fyi.json (right):

https://codereview.chromium.org/1533953003/diff/1/testing/buildbot/chromium.f...
testing/buildbot/chromium.fyi.json:5784: "test": "ash_unittests"
On 2015/12/17 23:50:36, Charlie Reis (OOO till Jan 4) wrote:
> Is --isolate-extensions meaningful in ash_unittests?  (I'm guessing so since
> ChromeOS has extensions, but I'm not certain.)

I'm not really sure either.  I'll keep it enabled, since as you say, ChromeOS
has extensions and I see some references to, for example, the Cast extension
(though I'm not sure if any unittests actually hit that codepath).

https://codereview.chromium.org/1533953003/diff/1/testing/buildbot/chromium.f...
testing/buildbot/chromium.fyi.json:5807: "--isolate-extensions"
On 2015/12/17 23:50:36, Charlie Reis (OOO till Jan 4) wrote:
> This flag doesn't mean anything in content_browsertests.  I would suggest
either
> keeping this as --site-per-process (with the existing filter) or using Nick's
> new --isolate-sites-for-testing flag (for some arbitrary site like *.is).

Switched this back to --site-per-process and original filter.

https://codereview.chromium.org/1533953003/diff/1/testing/buildbot/chromium.f...
testing/buildbot/chromium.fyi.json:5813: "--isolate-extensions"
On 2015/12/17 23:50:36, Charlie Reis (OOO till Jan 4) wrote:
> Same for content_unittests.

Same fix as content_browsertests.

https://codereview.chromium.org/1533953003/diff/1/testing/buildbot/chromium.f...
testing/buildbot/chromium.fyi.json:5819: "--isolate-extensions"
On 2015/12/18 20:20:20, ncarter wrote:
> On 2015/12/17 23:50:36, Charlie Reis (OOO till Jan 4) wrote:
> > Nick tells me that even extensions_browsertests don't know about
> > --isolate-extensions?  :(
> 
> Yeah; I think --isolate-extensions only matters if you install the
> ChromeContentClient/ChromeContentBrowserClient (and it would be a fair amount
of
> work to make that happen).
> 
> HOWEVER! You can do the following:
> 
>    --isolate-sites-for-testing=chrome-extension://*
> 

Nice, I've switched both extensions_browsertests and extensions_unittests to
--isolate-sites-for-testing=chrome-extension://*

[Charlie Reis, 2016-01-04 20:09:32]

LGTM

https://codereview.chromium.org/1533953003/diff/1/testing/buildbot/chromium.f...
File testing/buildbot/chromium.fyi.json (right):

https://codereview.chromium.org/1533953003/diff/1/testing/buildbot/chromium.f...
testing/buildbot/chromium.fyi.json:5807: "--isolate-extensions"
On 2015/12/18 21:15:13, alexmos wrote:
> On 2015/12/17 23:50:36, Charlie Reis (OOO till Jan 4) wrote:
> > This flag doesn't mean anything in content_browsertests.  I would suggest
> either
> > keeping this as --site-per-process (with the existing filter) or using
Nick's
> > new --isolate-sites-for-testing flag (for some arbitrary site like *.is).
> 
> Switched this back to --site-per-process and original filter.

I think this makes sense for this CL.

After this lands, I think it will make sense to switch these content tests over
to something like --isolate-sites-for-testing=*.is.  That will let us have some
running test coverage for turning our infrastructure on without isolating
everything (like what we run in the trial).  Nick pointed out that there's a
decent chance we'll have some tests that fail that way (e.g., those that check
for kSitePerProcess), so it's a good idea to do that step separately.

https://codereview.chromium.org/1533953003/diff/1/testing/buildbot/chromium.f...
testing/buildbot/chromium.fyi.json:5819: "--isolate-extensions"
On 2015/12/18 21:15:13, alexmos wrote:
> On 2015/12/18 20:20:20, ncarter wrote:
> > On 2015/12/17 23:50:36, Charlie Reis (OOO till Jan 4) wrote:
> > > Nick tells me that even extensions_browsertests don't know about
> > > --isolate-extensions?  :(
> > 
> > Yeah; I think --isolate-extensions only matters if you install the
> > ChromeContentClient/ChromeContentBrowserClient (and it would be a fair
amount
> of
> > work to make that happen).
> > 
> > HOWEVER! You can do the following:
> > 
> >    --isolate-sites-for-testing=chrome-extension://*
> > 
> 
> Nice, I've switched both extensions_browsertests and extensions_unittests to
> --isolate-sites-for-testing=chrome-extension://*

Great, I like this.

[ncarter (slow), 2016-01-04 20:50:23]

lgtm

[alexmos, 2016-01-04 22:29:17]

The CQ bit was checked by alexmos@chromium.org

[commit-bot: I haz the power, 2016-01-04 22:30:01]

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

[alexmos, 2016-01-04 22:30:44]

https://codereview.chromium.org/1533953003/diff/1/testing/buildbot/chromium.f...
File testing/buildbot/chromium.fyi.json (right):

https://codereview.chromium.org/1533953003/diff/1/testing/buildbot/chromium.f...
testing/buildbot/chromium.fyi.json:5807: "--isolate-extensions"
On 2016/01/04 20:09:31, Charlie Reis wrote:
> On 2015/12/18 21:15:13, alexmos wrote:
> > On 2015/12/17 23:50:36, Charlie Reis (OOO till Jan 4) wrote:
> > > This flag doesn't mean anything in content_browsertests.  I would suggest
> > either
> > > keeping this as --site-per-process (with the existing filter) or using
> Nick's
> > > new --isolate-sites-for-testing flag (for some arbitrary site like *.is).
> > 
> > Switched this back to --site-per-process and original filter.
> 
> I think this makes sense for this CL.
> 
> After this lands, I think it will make sense to switch these content tests
over
> to something like --isolate-sites-for-testing=*.is.  That will let us have
some
> running test coverage for turning our infrastructure on without isolating
> everything (like what we run in the trial).  Nick pointed out that there's a
> decent chance we'll have some tests that fail that way (e.g., those that check
> for kSitePerProcess), so it's a good idea to do that step separately.

Acknowledged.

[commit-bot: I haz the power, 2016-01-04 23:40:51]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-01-04 23:42:48]

Description was changed from

==========
Switch Site Isolation Windows FYI bot to run with --isolate-extensions.

BUG=417518
==========

to

==========
Switch Site Isolation Windows FYI bot to run with --isolate-extensions.

BUG=417518

Committed: https://crrev.com/20ba43a9f5e6fa1650b9aa67aa1e3c212f5c6fa9
Cr-Commit-Position: refs/heads/master@{#367412}
==========

[commit-bot: I haz the power, 2016-01-04 23:42:48]

Patchset 4 (id:??) landed as
https://crrev.com/20ba43a9f5e6fa1650b9aa67aa1e3c212f5c6fa9
Cr-Commit-Position: refs/heads/master@{#367412}