Get headless_shell building on Windows.

Set up the sandbox specifically for Windows, move non-windows functionality behind an ifdef.

BUG=686608
Review-Url: https://codereview.chromium.org/2624343002
Cr-Commit-Position: refs/heads/master@{#455640}
Committed: https://chromium.googlesource.com/chromium/src/+/0570f08c526622c02b11d76a87c8bd444691e8e6

[paulirish, 2017-01-12 05:53:47]

Description was changed from

==========
Fix git cl upload issues


Initial hacky implementation for windows headless shell

BUG=
==========

to

==========
Initial hacky implementation for windows headless shell

BUG=
==========

[dvallet, 2017-02-13 03:17:04]

Description was changed from

==========
Initial hacky implementation for windows headless shell

BUG=
==========

to

==========
Initial hacky implementation for windows headless shell

BUG=686608
==========

[dvallet, 2017-02-13 03:17:04]

dvallet@chromium.org changed reviewers:
+ dvallet@chromium.org, skyostil@chromium.org

[dvallet, 2017-02-13 03:20:24]

Apologies for the late review!

We just landed https://codereview.chromium.org/2666503002 which should allow you
to land this CL. 

Please take into account my and Sami's comments, there's some stuff that you no
longer need in order to make it work.

You'll need to rebase-update and there will be some conflicts, please follow my
comments to check whether you need to keep your code or not.

Thanks!

https://codereview.chromium.org/2624343002/diff/1/headless/app/headless_shell.cc
File headless/app/headless_shell.cc (right):

https://codereview.chromium.org/2624343002/diff/1/headless/app/headless_shell...
headless/app/headless_shell.cc:109: //if (args.empty())
These is no longer needed

https://codereview.chromium.org/2624343002/diff/1/headless/app/headless_shell...
headless/app/headless_shell.cc:449: base::CommandLine::Init(0, nullptr);
Ditto, this is not needed anymore

https://codereview.chromium.org/2624343002/diff/1/headless/app/headless_shell...
File headless/app/headless_shell_main.cc (right):

https://codereview.chromium.org/2624343002/diff/1/headless/app/headless_shell...
headless/app/headless_shell_main.cc:12: namespace content {
afaik Ppapi is only included in component build, so you don't need to add this
if add "is_component_build = true") to your args)

https://codereview.chromium.org/2624343002/diff/1/headless/lib/browser/headle...
File headless/lib/browser/headless_browser_impl.cc (right):

https://codereview.chromium.org/2624343002/diff/1/headless/lib/browser/headle...
headless/lib/browser/headless_browser_impl.cc:41: sandbox::SandboxInterfaceInfo
sandbox_info = {0};
Please change to:
#if defined(OS_WIN)
  sandbox::SandboxInterfaceInfo sandbox_info = {0};
  content::InitializeSandboxInfo(&sandbox_info);
  params.sandbox_info = &sandbox_info;
#elif !defined(OS_ANDROID)
  params.argc = options.argc;
  params.argv = options.argv;
#endif

https://codereview.chromium.org/2624343002/diff/1/headless/lib/browser/headle...
headless/lib/browser/headless_browser_impl.cc:199: base::CommandLine
command_line(0, nullptr);
Revert thes changes.

https://codereview.chromium.org/2624343002/diff/1/headless/lib/browser/headle...
headless/lib/browser/headless_browser_impl.cc:203:
HeadlessBrowser::Options::Builder builder(0, nullptr);
ditto, revert this

https://codereview.chromium.org/2624343002/diff/1/headless/lib/headless_conte...
File headless/lib/headless_content_main_delegate.h (right):

https://codereview.chromium.org/2624343002/diff/1/headless/lib/headless_conte...
headless/lib/headless_content_main_delegate.h:33: void ZygoteForked();
Please change to:
#if defined(OS_WIN)
  void ZygoteForked();
#else
  void ZygoteForked() override;
#endif

[Sami, 2017-02-13 18:46:31]

https://codereview.chromium.org/2624343002/diff/1/AUTHORS
File AUTHORS (right):

https://codereview.chromium.org/2624343002/diff/1/AUTHORS#newcode802
AUTHORS:802: Charles Vaughn <cvaughn@gmail.com>
Just checking: have you signed the contributor agreement as described here?
https://www.chromium.org/developers/contributing-code

[cvaughn, 2017-02-17 21:14:36]

On 2017/02/13 18:46:31, Sami wrote:
> https://codereview.chromium.org/2624343002/diff/1/AUTHORS
> File AUTHORS (right):
> 
> https://codereview.chromium.org/2624343002/diff/1/AUTHORS#newcode802
> AUTHORS:802: Charles Vaughn <mailto:cvaughn@gmail.com>
> Just checking: have you signed the contributor agreement as described here?
> https://www.chromium.org/developers/contributing-code

Yes, the corporate agreement as well should either be signed by now, or will be
soon.

[cvaughn, 2017-02-17 21:15:24]

Description was changed from

==========
Initial hacky implementation for windows headless shell

BUG=686608
==========

to

==========
Set up the sandbox specifically for Windows, move non-windows functionality
behind an ifdef.

BUG=686608
==========

[dvallet, 2017-02-19 22:48:47]

The CQ bit was checked by dvallet@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-19 22:48:53]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-19 23:18:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-19 23:18:18]

Dry run: Try jobs failed on following builders:
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[dvallet, 2017-02-20 00:52:36]

On 2017/02/17 at 21:14:36, cvaughn wrote:
> On 2017/02/13 18:46:31, Sami wrote:
> > https://codereview.chromium.org/2624343002/diff/1/AUTHORS
> > File AUTHORS (right):
> > 
> > https://codereview.chromium.org/2624343002/diff/1/AUTHORS#newcode802
> > AUTHORS:802: Charles Vaughn <mailto:cvaughn@gmail.com>
> > Just checking: have you signed the contributor agreement as described here?
> > https://www.chromium.org/developers/contributing-code
> 
> Yes, the corporate agreement as well should either be signed by now, or will
be soon.
LGTM

Looks like I missed this change to avoid build failure of Chrome in Windows: 
Could you move the windows specific dependencies:
https://cs.chromium.org/chromium/src/headless/BUILD.gn?rcl=e3c137153d7e9995c5...
To the headless_shell executable target?
https://cs.chromium.org/chromium/src/headless/BUILD.gn?rcl=e3c137153d7e9995c5...

The linux try fail I think is due to test flakiness, if you update to upstream
they should pass.

Thanks!

[Sami, 2017-02-20 16:50:23]

lgtm with a couple of nits.

https://codereview.chromium.org/2624343002/diff/20001/headless/lib/DEPS
File headless/lib/DEPS (left):

https://codereview.chromium.org/2624343002/diff/20001/headless/lib/DEPS#oldcode6
headless/lib/DEPS:6: 
Could you revert this whitespace change please?

https://codereview.chromium.org/2624343002/diff/20001/headless/lib/browser/he...
File headless/lib/browser/headless_content_browser_client.cc (right):

https://codereview.chromium.org/2624343002/diff/20001/headless/lib/browser/he...
headless/lib/browser/headless_content_browser_client.cc:184: #endif
nit: Please add a comment at the end of long ifdef blocks like these:

#endif  // defined(OS_POSIX) && !defined(OS_MACOSX)

[cvaughn, 2017-02-21 21:02:59]

On 2017/02/20 00:52:36, dvallet wrote:
> On 2017/02/17 at 21:14:36, cvaughn wrote:
> > On 2017/02/13 18:46:31, Sami wrote:
> > > https://codereview.chromium.org/2624343002/diff/1/AUTHORS
> > > File AUTHORS (right):
> > > 
> > > https://codereview.chromium.org/2624343002/diff/1/AUTHORS#newcode802
> > > AUTHORS:802: Charles Vaughn <mailto:cvaughn@gmail.com>
> > > Just checking: have you signed the contributor agreement as described
here?
> > > https://www.chromium.org/developers/contributing-code
> > 
> > Yes, the corporate agreement as well should either be signed by now, or will
> be soon.
> LGTM
> 
> Looks like I missed this change to avoid build failure of Chrome in Windows: 
> Could you move the windows specific dependencies:
>
https://cs.chromium.org/chromium/src/headless/BUILD.gn?rcl=e3c137153d7e9995c5...
> To the headless_shell executable target?
>
https://cs.chromium.org/chromium/src/headless/BUILD.gn?rcl=e3c137153d7e9995c5...
> 
> The linux try fail I think is due to test flakiness, if you update to upstream
> they should pass.
> 
> Thanks!

Not sure I follow. Do you mean take the contents of the is_win block and move
them specifically under headless_shell block?

[dvallet, 2017-02-21 21:53:57]

On 2017/02/21 at 21:02:59, cvaughn wrote:
> On 2017/02/20 00:52:36, dvallet wrote:
> > On 2017/02/17 at 21:14:36, cvaughn wrote:
> > > On 2017/02/13 18:46:31, Sami wrote:
> > > > https://codereview.chromium.org/2624343002/diff/1/AUTHORS
> > > > File AUTHORS (right):
> > > > 
> > > > https://codereview.chromium.org/2624343002/diff/1/AUTHORS#newcode802
> > > > AUTHORS:802: Charles Vaughn <mailto:cvaughn@gmail.com>
> > > > Just checking: have you signed the contributor agreement as described
here?
> > > > https://www.chromium.org/developers/contributing-code
> > > 
> > > Yes, the corporate agreement as well should either be signed by now, or
will
> > be soon.
> > LGTM
> > 
> > Looks like I missed this change to avoid build failure of Chrome in Windows:

> > Could you move the windows specific dependencies:
> >
https://cs.chromium.org/chromium/src/headless/BUILD.gn?rcl=e3c137153d7e9995c5...
> > To the headless_shell executable target?
> >
https://cs.chromium.org/chromium/src/headless/BUILD.gn?rcl=e3c137153d7e9995c5...
> > 
> > The linux try fail I think is due to test flakiness, if you update to
upstream
> > they should pass.
> > 
> > Thanks!
> 
> Not sure I follow. Do you mean take the contents of the is_win block and move
them specifically under headless_shell block?

Yes, otherwise the windows chrome build fails (and trybots fail)

[dvallet, 2017-02-27 22:51:00]

On 2017/02/21 21:53:57, dvallet wrote:
> On 2017/02/21 at 21:02:59, cvaughn wrote:
> > On 2017/02/20 00:52:36, dvallet wrote:
> > > On 2017/02/17 at 21:14:36, cvaughn wrote:
> > > > On 2017/02/13 18:46:31, Sami wrote:
> > > > > https://codereview.chromium.org/2624343002/diff/1/AUTHORS
> > > > > File AUTHORS (right):
> > > > > 
> > > > > https://codereview.chromium.org/2624343002/diff/1/AUTHORS#newcode802
> > > > > AUTHORS:802: Charles Vaughn <mailto:cvaughn@gmail.com>
> > > > > Just checking: have you signed the contributor agreement as described
> here?
> > > > > https://www.chromium.org/developers/contributing-code
> > > > 
> > > > Yes, the corporate agreement as well should either be signed by now, or
> will
> > > be soon.
> > > LGTM
> > > 
> > > Looks like I missed this change to avoid build failure of Chrome in
Windows:
> 
> > > Could you move the windows specific dependencies:
> > >
>
https://cs.chromium.org/chromium/src/headless/BUILD.gn?rcl=e3c137153d7e9995c5...
> > > To the headless_shell executable target?
> > >
>
https://cs.chromium.org/chromium/src/headless/BUILD.gn?rcl=e3c137153d7e9995c5...
> > > 
> > > The linux try fail I think is due to test flakiness, if you update to
> upstream
> > > they should pass.
> > > 
> > > Thanks!
> > 
> > Not sure I follow. Do you mean take the contents of the is_win block and
move
> them specifically under headless_shell block?
> 
> Yes, otherwise the windows chrome build fails (and trybots fail)

Just checking how's the progress on this? If it makes your live easier, I'm
happy to patch this CL and submit it for you.

[cvaughn, 2017-02-27 22:55:42]

Oh sorry, Is patch set 4 what you had in mind?

On Mon, Feb 27, 2017 at 2:51 PM <dvallet@chromium.org> wrote:

On 2017/02/21 21:53:57, dvallet wrote:
> On 2017/02/21 at 21:02:59, cvaughn wrote:
> > On 2017/02/20 00:52:36, dvallet wrote:
> > > On 2017/02/17 at 21:14:36, cvaughn wrote:
> > > > On 2017/02/13 18:46:31, Sami wrote:
> > > > > https://codereview.chromium.org/2624343002/diff/1/AUTHORS
> > > > > File AUTHORS (right):
> > > > >
> > > > >
https://codereview.chromium.org/2624343002/diff/1/AUTHORS#newcode802
> > > > > AUTHORS:802: Charles Vaughn <mailto:cvaughn@gmail.com>
> > > > > Just checking: have you signed the contributor agreement as
described
> here?
> > > > > https://www.chromium.org/developers/contributing-code
> > > >
> > > > Yes, the corporate agreement as well should either be signed by
now, or
> will
> > > be soon.
> > > LGTM
> > >
> > > Looks like I missed this change to avoid build failure of Chrome in
Windows:
>
> > > Could you move the windows specific dependencies:
> > >
>
https://cs.chromium.org/chromium/src/headless/BUILD.gn?rcl=e3c137153d7e9995c5...
> > > To the headless_shell executable target?
> > >
>
https://cs.chromium.org/chromium/src/headless/BUILD.gn?rcl=e3c137153d7e9995c5...
> > >
> > > The linux try fail I think is due to test flakiness, if you update to
> upstream
> > > they should pass.
> > >
> > > Thanks!
> >
> > Not sure I follow. Do you mean take the contents of the is_win block and
move
> them specifically under headless_shell block?
>
> Yes, otherwise the windows chrome build fails (and trybots fail)

Just checking how's the progress on this? If it makes your live easier, I'm
happy to patch this CL and submit it for you.

https://codereview.chromium.org/2624343002/

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[dvallet, 2017-02-27 23:00:23]

On 2017/02/27 22:55:42, cvaughn wrote:
> Oh sorry, Is patch set 4 what you had in mind?
> 
> On Mon, Feb 27, 2017 at 2:51 PM <mailto:dvallet@chromium.org> wrote:
> 
> On 2017/02/21 21:53:57, dvallet wrote:
> > On 2017/02/21 at 21:02:59, cvaughn wrote:
> > > On 2017/02/20 00:52:36, dvallet wrote:
> > > > On 2017/02/17 at 21:14:36, cvaughn wrote:
> > > > > On 2017/02/13 18:46:31, Sami wrote:
> > > > > > https://codereview.chromium.org/2624343002/diff/1/AUTHORS
> > > > > > File AUTHORS (right):
> > > > > >
> > > > > >
> https://codereview.chromium.org/2624343002/diff/1/AUTHORS#newcode802
> > > > > > AUTHORS:802: Charles Vaughn <mailto:cvaughn@gmail.com>
> > > > > > Just checking: have you signed the contributor agreement as
> described
> > here?
> > > > > > https://www.chromium.org/developers/contributing-code
> > > > >
> > > > > Yes, the corporate agreement as well should either be signed by
> now, or
> > will
> > > > be soon.
> > > > LGTM
> > > >
> > > > Looks like I missed this change to avoid build failure of Chrome in
> Windows:
> >
> > > > Could you move the windows specific dependencies:
> > > >
> >
>
https://cs.chromium.org/chromium/src/headless/BUILD.gn?rcl=e3c137153d7e9995c5...
> > > > To the headless_shell executable target?
> > > >
> >
>
https://cs.chromium.org/chromium/src/headless/BUILD.gn?rcl=e3c137153d7e9995c5...
> > > >
> > > > The linux try fail I think is due to test flakiness, if you update to
> > upstream
> > > > they should pass.
> > > >
> > > > Thanks!
> > >
> > > Not sure I follow. Do you mean take the contents of the is_win block and
> move
> > them specifically under headless_shell block?
> >
> > Yes, otherwise the windows chrome build fails (and trybots fail)
> 
> Just checking how's the progress on this? If it makes your live easier, I'm
> happy to patch this CL and submit it for you.
> 
> https://codereview.chromium.org/2624343002/
> 
> -- 
> You received this message because you are subscribed to the Google Groups
> "Chromium-reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

Yes, thanks!, you might need to git rebase-update && gclient sync
After that you can commit

[cvaughn, 2017-02-27 23:02:22]

Great, will do that shortly

On Mon, Feb 27, 2017 at 3:00 PM <dvallet@chromium.org> wrote:

> On 2017/02/27 22:55:42, cvaughn wrote:
> > Oh sorry, Is patch set 4 what you had in mind?
> >
> > On Mon, Feb 27, 2017 at 2:51 PM <mailto:dvallet@chromium.org> wrote:
> >
> > On 2017/02/21 21:53:57, dvallet wrote:
> > > On 2017/02/21 at 21:02:59, cvaughn wrote:
> > > > On 2017/02/20 00:52:36, dvallet wrote:
> > > > > On 2017/02/17 at 21:14:36, cvaughn wrote:
> > > > > > On 2017/02/13 18:46:31, Sami wrote:
> > > > > > > https://codereview.chromium.org/2624343002/diff/1/AUTHORS
> > > > > > > File AUTHORS (right):
> > > > > > >
> > > > > > >
> > https://codereview.chromium.org/2624343002/diff/1/AUTHORS#newcode802
> > > > > > > AUTHORS:802: Charles Vaughn <mailto:cvaughn@gmail.com>
> > > > > > > Just checking: have you signed the contributor agreement as
> > described
> > > here?
> > > > > > > https://www.chromium.org/developers/contributing-code
> > > > > >
> > > > > > Yes, the corporate agreement as well should either be signed by
> > now, or
> > > will
> > > > > be soon.
> > > > > LGTM
> > > > >
> > > > > Looks like I missed this change to avoid build failure of Chrome in
> > Windows:
> > >
> > > > > Could you move the windows specific dependencies:
> > > > >
> > >
> >
>
>
https://cs.chromium.org/chromium/src/headless/BUILD.gn?rcl=e3c137153d7e9995c5...
> > > > > To the headless_shell executable target?
> > > > >
> > >
> >
>
>
https://cs.chromium.org/chromium/src/headless/BUILD.gn?rcl=e3c137153d7e9995c5...
> > > > >
> > > > > The linux try fail I think is due to test flakiness, if you update
> to
> > > upstream
> > > > > they should pass.
> > > > >
> > > > > Thanks!
> > > >
> > > > Not sure I follow. Do you mean take the contents of the is_win block
> and
> > move
> > > them specifically under headless_shell block?
> > >
> > > Yes, otherwise the windows chrome build fails (and trybots fail)
> >
> > Just checking how's the progress on this? If it makes your live easier,
> I'm
> > happy to patch this CL and submit it for you.
> >
> > https://codereview.chromium.org/2624343002/
> >
> > --
> > You received this message because you are subscribed to the Google Groups
> > "Chromium-reviews" group.
> > To unsubscribe from this group and stop receiving emails from it, send an
> email
> > to mailto:chromium-reviews+unsubscribe@chromium.org.
>
> Yes, thanks!, you might need to git rebase-update && gclient sync
> After that you can commit
>
> https://codereview.chromium.org/2624343002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[cvaughn, 2017-02-28 18:57:33]

The CQ bit was checked by cvaughn@gmail.com to run a CQ dry run

[commit-bot: I haz the power, 2017-02-28 18:58:14]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-28 19:00:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-28 19:00:48]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[cvaughn, 2017-02-28 19:23:47]

The CQ bit was checked by cvaughn@gmail.com to run a CQ dry run

[commit-bot: I haz the power, 2017-02-28 19:24:20]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-28 20:43:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-28 20:43:03]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[cvaughn, 2017-02-28 21:05:05]

Looks like link errors in the main chrome build when headless_lib is
included. At a little bit of a loss here. Looks like it's linking in 2
copies of content_switches

On Mon, Feb 27, 2017 at 7:02 AM Charles Vaughn <cvaughn@gmail.com> wrote:

> Great, will do that shortly
>
> On Mon, Feb 27, 2017 at 3:00 PM <dvallet@chromium.org> wrote:
>
> On 2017/02/27 22:55:42, cvaughn wrote:
> > Oh sorry, Is patch set 4 what you had in mind?
> >
> > On Mon, Feb 27, 2017 at 2:51 PM <mailto:dvallet@chromium.org> wrote:
> >
> > On 2017/02/21 21:53:57, dvallet wrote:
> > > On 2017/02/21 at 21:02:59, cvaughn wrote:
> > > > On 2017/02/20 00:52:36, dvallet wrote:
> > > > > On 2017/02/17 at 21:14:36, cvaughn wrote:
> > > > > > On 2017/02/13 18:46:31, Sami wrote:
> > > > > > > https://codereview.chromium.org/2624343002/diff/1/AUTHORS
> > > > > > > File AUTHORS (right):
> > > > > > >
> > > > > > >
> > https://codereview.chromium.org/2624343002/diff/1/AUTHORS#newcode802
> > > > > > > AUTHORS:802: Charles Vaughn <mailto:cvaughn@gmail.com>
> > > > > > > Just checking: have you signed the contributor agreement as
> > described
> > > here?
> > > > > > > https://www.chromium.org/developers/contributing-code
> > > > > >
> > > > > > Yes, the corporate agreement as well should either be signed by
> > now, or
> > > will
> > > > > be soon.
> > > > > LGTM
> > > > >
> > > > > Looks like I missed this change to avoid build failure of Chrome in
> > Windows:
> > >
> > > > > Could you move the windows specific dependencies:
> > > > >
> > >
> >
>
>
https://cs.chromium.org/chromium/src/headless/BUILD.gn?rcl=e3c137153d7e9995c5...
> > > > > To the headless_shell executable target?
> > > > >
> > >
> >
>
>
https://cs.chromium.org/chromium/src/headless/BUILD.gn?rcl=e3c137153d7e9995c5...
> > > > >
> > > > > The linux try fail I think is due to test flakiness, if you update
> to
> > > upstream
> > > > > they should pass.
> > > > >
> > > > > Thanks!
> > > >
> > > > Not sure I follow. Do you mean take the contents of the is_win block
> and
> > move
> > > them specifically under headless_shell block?
> > >
> > > Yes, otherwise the windows chrome build fails (and trybots fail)
> >
> > Just checking how's the progress on this? If it makes your live easier,
> I'm
> > happy to patch this CL and submit it for you.
> >
> > https://codereview.chromium.org/2624343002/
> >
> > --
> > You received this message because you are subscribed to the Google Groups
> > "Chromium-reviews" group.
> > To unsubscribe from this group and stop receiving emails from it, send an
> email
> > to mailto:chromium-reviews+unsubscribe@chromium.org.
>
> Yes, thanks!, you might need to git rebase-update && gclient sync
> After that you can commit
>
> https://codereview.chromium.org/2624343002/
>
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[dvallet, 2017-03-01 02:40:39]

https://codereview.chromium.org/2624343002/diff/120001/chrome/BUILD.gn
File chrome/BUILD.gn (right):

https://codereview.chromium.org/2624343002/diff/120001/chrome/BUILD.gn#newcod...
chrome/BUILD.gn:196: # For headless mode.
This is breaking the component_build, and it requires actually some work on
reorganizing headless libraries to make this work. 
I suggest that you revert this file and submit the CL as is and I'll work on how
to add the dependency for component builds.

[cvaughn, 2017-03-01 02:48:29]

Will do.

On Tue, Feb 28, 2017, 18:40 <dvallet@chromium.org> wrote:

>
> https://codereview.chromium.org/2624343002/diff/120001/chrome/BUILD.gn
> File chrome/BUILD.gn (right):
>
>
>
https://codereview.chromium.org/2624343002/diff/120001/chrome/BUILD.gn#newcod...
> chrome/BUILD.gn:196: # For headless mode.
> This is breaking the component_build, and it requires actually some work
> on reorganizing headless libraries to make this work.
> I suggest that you revert this file and submit the CL as is and I'll
> work on how to add the dependency for component builds.
>
> https://codereview.chromium.org/2624343002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[cvaughn, 2017-03-01 22:18:53]

The CQ bit was checked by cvaughn@gmail.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-01 22:19:31]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-02 01:05:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-02 01:05:03]

Dry run: This issue passed the CQ dry run.

[cvaughn, 2017-03-02 01:06:59]

The CQ bit was checked by cvaughn@gmail.com

[cvaughn, 2017-03-02 01:06:59]

The patchset sent to the CQ was uploaded after l-g-t-m from
dvallet@chromium.org, skyostil@chromium.org
Link to the patchset: https://codereview.chromium.org/2624343002/#ps140001
(title: "Remove headless_shell addition to chrome windows build")

[commit-bot: I haz the power, 2017-03-02 01:07:32]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[cvaughn, 2017-03-02 01:10:03]

cvaughn@gmail.com changed reviewers:
+ jschuh@chromium.org

[commit-bot: I haz the power, 2017-03-02 01:16:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-02 01:16:45]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[dvallet, 2017-03-02 02:02:14]

dvallet@chromium.org changed reviewers:
+ forshaw@chromium.orf

[dvallet, 2017-03-02 02:02:15]

+forshaw: Could you please review regarding adding +sandbox/win/src to run
headless chrome in Windows?

[dvallet, 2017-03-06 05:07:27]

dvallet@chromium.org changed reviewers:
+ forshaw@chromium.org
- forshaw@chromium.orf

[dvallet, 2017-03-06 05:08:23]

dvallet@chromium.org changed reviewers:
- forshaw@chromium.org

[dvallet, 2017-03-06 05:08:23]

dvallet@chromium.org changed reviewers:
- forshaw@chromium.org

[dvallet, 2017-03-06 05:08:59]

@jschuh: Could you please review regarding adding +sandbox/win/src to run
headless chrome in Windows?

[dvallet, 2017-03-07 22:40:50]

https://codereview.chromium.org/2624343002/diff/140001/headless/lib/browser/DEPS
File headless/lib/browser/DEPS (right):

https://codereview.chromium.org/2624343002/diff/140001/headless/lib/browser/D...
headless/lib/browser/DEPS:5: "+sandbox/win/src",
This actually may not be needed. Go ahead and revert, and you should be able to
submit this time.

[cvaughn, 2017-03-08 17:24:13]

headless_content_main_delegate.cc
<https://codereview.chromium.org/2624343002/diff/140001/headless/lib/headless_...>
pulls it in for the windows specific sandbox types

On Tue, Mar 7, 2017 at 2:40 PM <dvallet@chromium.org> wrote:

>
>
>
https://codereview.chromium.org/2624343002/diff/140001/headless/lib/browser/DEPS
> File headless/lib/browser/DEPS (right):
>
>
>
https://codereview.chromium.org/2624343002/diff/140001/headless/lib/browser/D...
> headless/lib/browser/DEPS:5: "+sandbox/win/src",
> This actually may not be needed. Go ahead and revert, and you should be
> able to submit this time.
>
> https://codereview.chromium.org/2624343002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[dvallet, 2017-03-08 23:32:08]

dvallet@chromium.org changed reviewers:
+ wfh@chromium.org
- jschuh@chromium.org

[Will Harris, 2017-03-08 23:37:06]

lgtm for the addition of sandbox/win/src to headless/lib/browser/DEPS

[cvaughn, 2017-03-09 00:41:07]

The CQ bit was checked by cvaughn@gmail.com

[cvaughn, 2017-03-09 00:41:28]

The CQ bit was unchecked by cvaughn@gmail.com

[commit-bot: I haz the power, 2017-03-09 00:41:44]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[cvaughn, 2017-03-09 00:41:51]

The CQ bit was checked by cvaughn@gmail.com

[commit-bot: I haz the power, 2017-03-09 00:42:17]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[cvaughn, 2017-03-09 00:42:24]

The CQ bit was unchecked by cvaughn@gmail.com

[cvaughn, 2017-03-09 00:42:32]

The CQ bit was checked by cvaughn@gmail.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-09 00:43:12]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-09 02:05:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-09 02:05:43]

Dry run: This issue passed the CQ dry run.

[cvaughn, 2017-03-09 02:07:18]

The CQ bit was checked by cvaughn@gmail.com

[commit-bot: I haz the power, 2017-03-09 02:07:57]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-09 02:13:24]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1489025238386440,
"parent_rev": "85c3c4f9fdb6de0dcda6f8c5e9d460ef09dd292d", "commit_rev":
"0570f08c526622c02b11d76a87c8bd444691e8e6"}

[commit-bot: I haz the power, 2017-03-09 02:14:06]

Description was changed from

==========
Set up the sandbox specifically for Windows, move non-windows functionality
behind an ifdef.

BUG=686608
==========

to

==========
Set up the sandbox specifically for Windows, move non-windows functionality
behind an ifdef.

BUG=686608

Review-Url: https://codereview.chromium.org/2624343002
Cr-Commit-Position: refs/heads/master@{#455640}
Committed:
https://chromium.googlesource.com/chromium/src/+/0570f08c526622c02b11d76a87c8...
==========

[commit-bot: I haz the power, 2017-03-09 02:14:07]

Committed patchset #8 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/0570f08c526622c02b11d76a87c8...