Move gin and v8 portion of profiling to child from common

Move gin and v8 portion of profiling to child from common

This breaks up profiling.cc into separate common and child-only
parts to remove the dependency of the browser DLL on Windows on v8.

R=thakis@chromium.org
TBR=jochen@chromium.org
BUG=581766
Review-Url: https://codereview.chromium.org/2627753004
Cr-Commit-Position: refs/heads/master@{#444443}
Committed: https://chromium.googlesource.com/chromium/src/+/608e24db64fd84c9ebaff87bf07d50c3d34059d7

[scottmg, 2017-01-12 04:35:53]

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

[commit-bot: I haz the power, 2017-01-12 04:36:29]

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

[scottmg, 2017-01-12 04:38:38]

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

[commit-bot: I haz the power, 2017-01-12 04:39:04]

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-01-12 04:41:14]

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

[commit-bot: I haz the power, 2017-01-12 04:41:14]

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-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  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...)

[scottmg, 2017-01-13 18:43:10]

Description was changed from

==========
Move gin/v8 portion of profiling to child from common

Part of removing dependency of Windows browser DLL on v8.

BUG=581766
==========

to

==========
Move gin/v8 portion of profiling to child from common

This breaks up profiling.cc into separate common and child-only parts to remove
the dependency of the browser DLL on Windows on v8.

BUG=581766
==========

[scottmg, 2017-01-13 18:43:24]

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

[scottmg, 2017-01-13 18:43:37]

Description was changed from

==========
Move gin/v8 portion of profiling to child from common

This breaks up profiling.cc into separate common and child-only parts to remove
the dependency of the browser DLL on Windows on v8.

BUG=581766
==========

to

==========
Move gin/v8 portion of profiling to child from common

This breaks up profiling.cc into separate common and child-only
parts to remove the dependency of the browser DLL on Windows on v8.

BUG=581766
==========

[commit-bot: I haz the power, 2017-01-13 18:43:52]

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

[scottmg, 2017-01-13 18:44:23]

Description was changed from

==========
Move gin/v8 portion of profiling to child from common

This breaks up profiling.cc into separate common and child-only
parts to remove the dependency of the browser DLL on Windows on v8.

BUG=581766
==========

to

==========
Move gin/v8 portion of profiling to child from common

This breaks up profiling.cc into separate common and child-only
parts to remove the dependency of the browser DLL on Windows on v8.

R=thakis@chromium.org
BUG=581766
==========

[scottmg, 2017-01-13 18:44:23]

scottmg@chromium.org changed reviewers:
+ thakis@chromium.org

[scottmg, 2017-01-13 18:45:37]

Description was changed from

==========
Move gin/v8 portion of profiling to child from common

This breaks up profiling.cc into separate common and child-only
parts to remove the dependency of the browser DLL on Windows on v8.

R=thakis@chromium.org
BUG=581766
==========

to

==========
Move gin and v8 portion of profiling to child from common

This breaks up profiling.cc into separate common and child-only
parts to remove the dependency of the browser DLL on Windows on v8.

R=thakis@chromium.org
BUG=581766
==========

[commit-bot: I haz the power, 2017-01-13 18:58:40]

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

[commit-bot: I haz the power, 2017-01-13 18:58:41]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[scottmg, 2017-01-13 19:24:18]

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

[commit-bot: I haz the power, 2017-01-13 19:25: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-01-13 19:40:23]

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

[commit-bot: I haz the power, 2017-01-13 19:40:24]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[scottmg, 2017-01-13 19:42:50]

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

[commit-bot: I haz the power, 2017-01-13 19:44:09]

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

[Nico, 2017-01-13 20:48:29]

https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...
looks like it might be real.

Is this the last bit of what keeps v8 alive?

Why are there two calls to Init in chrome_main_delegate.cc now?

[Nico, 2017-01-13 20:48:45]

On 2017/01/13 20:48:29, Nico wrote:
>
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...
> looks like it might be real.
> 
> Is this the last bit of what keeps v8 alive?

(in the dead-code-stripping sense)

[scottmg, 2017-01-13 20:58:39]

On 2017/01/13 20:48:29, Nico wrote:
>
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...
> looks like it might be real.

Hrm, that feature (link error) is defined in gin, so that dependency removal was
too aggressive. Undid that.

> 
> Is this the last bit of what keeps v8 alive?

Not yet, there's some more paths to v8.

> 
> Why are there two calls to Init in chrome_main_delegate.cc now?

Do you mean the ProcessStarted() calls? I was just mechanically breaking up
Profiling, so I put the ChildProfiling::ProcessStarted calls before the
Profiling::ProcessStarted calls to attempt to not change behaviour.

[scottmg, 2017-01-13 20:58:50]

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

[commit-bot: I haz the power, 2017-01-13 20:59:36]

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

[Nico, 2017-01-13 21:01:19]

https://codereview.chromium.org/2627753004/diff/100001/chrome/app/chrome_main...
File chrome/app/chrome_main_delegate.cc (right):

https://codereview.chromium.org/2627753004/diff/100001/chrome/app/chrome_main...
chrome/app/chrome_main_delegate.cc:532: #if
!defined(CHROME_MULTIPLE_DLL_BROWSER)
if this is mechanical, why does this have the !defined but the other place below
doesn't? (feels like a stupid question, sorry)

[scottmg, 2017-01-13 21:55:08]

https://codereview.chromium.org/2627753004/diff/100001/chrome/app/chrome_main...
File chrome/app/chrome_main_delegate.cc (right):

https://codereview.chromium.org/2627753004/diff/100001/chrome/app/chrome_main...
chrome/app/chrome_main_delegate.cc:532: #if
!defined(CHROME_MULTIPLE_DLL_BROWSER)
On 2017/01/13 21:01:19, Nico wrote:
> if this is mechanical, why does this have the !defined but the other place
below
> doesn't? (feels like a stupid question, sorry)

You're right, not entirely mechanical I guess. This one runs on all
platforms(-ish). This code is built twice on Windows, once for child and once
for browser, so the define breaks the dependency on the child-only code when
it's built for the browser.

The Zygote one below is in #if POSIX. Strictly speaking I guess it could have
!CHROME_MULTIPLE_DLL_BROWSER too, but it would be more confusing than anything
because that define only really makes sense on Windows.

[commit-bot: I haz the power, 2017-01-13 23:28:40]

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

[commit-bot: I haz the power, 2017-01-13 23:28:41]

Dry run: This issue passed the CQ dry run.

[Nico, 2017-01-17 13:10:43]

lgtm, thanks for explaining

https://codereview.chromium.org/2627753004/diff/100001/chrome/app/chrome_main...
File chrome/app/chrome_main_delegate.cc (right):

https://codereview.chromium.org/2627753004/diff/100001/chrome/app/chrome_main...
chrome/app/chrome_main_delegate.cc:532: #if
!defined(CHROME_MULTIPLE_DLL_BROWSER)
On 2017/01/13 21:55:08, scottmg wrote:
> On 2017/01/13 21:01:19, Nico wrote:
> > if this is mechanical, why does this have the !defined but the other place
> below
> > doesn't? (feels like a stupid question, sorry)
> 
> You're right, not entirely mechanical I guess. This one runs on all
> platforms(-ish). This code is built twice on Windows, once for child and once
> for browser, so the define breaks the dependency on the child-only code when
> it's built for the browser.
> 
> The Zygote one below is in #if POSIX. Strictly speaking I guess it could have
> !CHROME_MULTIPLE_DLL_BROWSER too, but it would be more confusing than anything
> because that define only really makes sense on Windows.

Maybe add a comment explaining why this is ifdef'd and the other isn't (but up
to you; if you think it's more confusing with a comment, omit it)

[scottmg, 2017-01-18 17:00:04]

On 2017/01/17 13:10:43, Nico wrote:
> lgtm, thanks for explaining
> 
>
https://codereview.chromium.org/2627753004/diff/100001/chrome/app/chrome_main...
> File chrome/app/chrome_main_delegate.cc (right):
> 
>
https://codereview.chromium.org/2627753004/diff/100001/chrome/app/chrome_main...
> chrome/app/chrome_main_delegate.cc:532: #if
> !defined(CHROME_MULTIPLE_DLL_BROWSER)
> On 2017/01/13 21:55:08, scottmg wrote:
> > On 2017/01/13 21:01:19, Nico wrote:
> > > if this is mechanical, why does this have the !defined but the other place
> > below
> > > doesn't? (feels like a stupid question, sorry)
> > 
> > You're right, not entirely mechanical I guess. This one runs on all
> > platforms(-ish). This code is built twice on Windows, once for child and
once
> > for browser, so the define breaks the dependency on the child-only code when
> > it's built for the browser.
> > 
> > The Zygote one below is in #if POSIX. Strictly speaking I guess it could
have
> > !CHROME_MULTIPLE_DLL_BROWSER too, but it would be more confusing than
anything
> > because that define only really makes sense on Windows.
> 
> Maybe add a comment explaining why this is ifdef'd and the other isn't (but up
> to you; if you think it's more confusing with a comment, omit it)

I think I'll leave it, I think it'd be equally confusing to have the zygote code
talking about Windows.

[scottmg, 2017-01-18 17:00:30]

The CQ bit was checked by scottmg@chromium.org

[commit-bot: I haz the power, 2017-01-18 17:01:08]

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-01-18 17:09:08]

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

[commit-bot: I haz the power, 2017-01-18 17:09:09]

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...)

[scottmg, 2017-01-18 18:37:56]

Description was changed from

==========
Move gin and v8 portion of profiling to child from common

This breaks up profiling.cc into separate common and child-only
parts to remove the dependency of the browser DLL on Windows on v8.

R=thakis@chromium.org
BUG=581766
==========

to

==========
Move gin and v8 portion of profiling to child from common

This breaks up profiling.cc into separate common and child-only
parts to remove the dependency of the browser DLL on Windows on v8.

R=thakis@chromium.org
TBR=jochen@chromium.org
BUG=581766
==========

[scottmg, 2017-01-18 18:37:56]

scottmg@chromium.org changed reviewers:
+ jochen@chromium.org

[scottmg, 2017-01-18 19:47:16]

tbr jochen for deps addition to child.

[scottmg, 2017-01-18 19:47:28]

The CQ bit was checked by scottmg@chromium.org

[commit-bot: I haz the power, 2017-01-18 19:47:58]

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-01-18 19:54:10]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1484768848460840,
"parent_rev": "828cd7f49f285857a741f6f255193ca6bbb011cd", "commit_rev":
"608e24db64fd84c9ebaff87bf07d50c3d34059d7"}

[commit-bot: I haz the power, 2017-01-18 19:54:44]

Description was changed from

==========
Move gin and v8 portion of profiling to child from common

This breaks up profiling.cc into separate common and child-only
parts to remove the dependency of the browser DLL on Windows on v8.

R=thakis@chromium.org
TBR=jochen@chromium.org
BUG=581766
==========

to

==========
Move gin and v8 portion of profiling to child from common

This breaks up profiling.cc into separate common and child-only
parts to remove the dependency of the browser DLL on Windows on v8.

R=thakis@chromium.org
TBR=jochen@chromium.org
BUG=581766

Review-Url: https://codereview.chromium.org/2627753004
Cr-Commit-Position: refs/heads/master@{#444443}
Committed:
https://chromium.googlesource.com/chromium/src/+/608e24db64fd84c9ebaff87bf07d...
==========

[commit-bot: I haz the power, 2017-01-18 19:54:45]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/608e24db64fd84c9ebaff87bf07d...