Delete the utility process startup ping.

Delete the utility process startup ping.

It's no longer necessary since IPC::TakePlatformFileForTransit() doesn't
require the child process handle.

BUG=597124
Committed: https://crrev.com/1bd8d3bba0e883748dcbf7c89c335c9864b39f2f
Cr-Commit-Position: refs/heads/master@{#387835}

[Anand Mistry (off Chromium), 2016-04-13 03:48:00]

amistry@chromium.org changed reviewers:
+ scottbyer@chromium.org, shess@chromium.org, tzik@chromium.org

[Anand Mistry (off Chromium), 2016-04-13 03:48:01]

scottbyer@chromium.org: Please review changes in printing

shess@chromium.org: Please review changes in safe_browsing

tzik@chromium.org: Please review changes in fileapi

[Anand Mistry (off Chromium), 2016-04-13 03:48:26]

Description was changed from

==========
Delete the utility process startup ping.

It's no longer necessary since IPC::TakePlatformFileForTransit() doesn't
require the child process handle.

BUG=None
==========

to

==========
Delete the utility process startup ping.

It's no longer necessary since IPC::TakePlatformFileForTransit() doesn't
require the child process handle.

BUG=597124
==========

[Lei Zhang, 2016-04-13 04:38:22]

thestig@chromium.org changed reviewers:
+ thestig@chromium.org

[Lei Zhang, 2016-04-13 04:38:23]

lgtm

https://codereview.chromium.org/1885813002/diff/20001/chrome/browser/media_ga...
File chrome/browser/media_galleries/fileapi/safe_audio_video_checker.h (right):

https://codereview.chromium.org/1885813002/diff/20001/chrome/browser/media_ga...
chrome/browser/media_galleries/fileapi/safe_audio_video_checker.h:37:
PINGED_STATE,
Isn't PINGED_STATE unused now?

[Lei Zhang, 2016-04-13 04:39:36]

https://codereview.chromium.org/1885813002/diff/20001/chrome/common/chrome_ut...
File chrome/common/chrome_utility_messages.h (left):

https://codereview.chromium.org/1885813002/diff/20001/chrome/common/chrome_ut...
chrome/common/chrome_utility_messages.h:170:
IPC_MESSAGE_CONTROL0(ChromeUtilityMsg_StartupPing)
And you need an IPC reviewer.

[Anand Mistry (off Chromium), 2016-04-13 04:41:19]

https://codereview.chromium.org/1885813002/diff/20001/chrome/browser/media_ga...
File chrome/browser/media_galleries/fileapi/safe_audio_video_checker.h (right):

https://codereview.chromium.org/1885813002/diff/20001/chrome/browser/media_ga...
chrome/browser/media_galleries/fileapi/safe_audio_video_checker.h:37:
PINGED_STATE,
On 2016/04/13 04:38:23, Lei Zhang wrote:
> Isn't PINGED_STATE unused now?

Yes, removed.

[Anand Mistry (off Chromium), 2016-04-13 04:42:31]

amistry@chromium.org changed reviewers:
+ jorgelo@chromium.org

[Anand Mistry (off Chromium), 2016-04-13 04:42:31]

jorgelo: For ipc messages

[Jorge Lucangeli Obes, 2016-04-13 04:52:36]

On 2016/04/13 04:42:31, Anand Mistry wrote:
> jorgelo: For ipc messages

chrome_utility_messages.h lgtm

[Anand Mistry (off Chromium), 2016-04-13 05:08:41]

To other reviewers:

Even though this has all needed lgtms, I'll let it sit for a few days in case
anyone has any concerns.

[tzik, 2016-04-13 10:20:31]

lgtm

[Scott Hess - ex-Googler, 2016-04-13 17:44:44]

LGTM for safe_browsing, though I had a question about whether the instance
variable is still necessary.

https://codereview.chromium.org/1885813002/diff/40001/chrome/browser/printing...
File chrome/browser/printing/pwg_raster_converter.cc (right):

https://codereview.chromium.org/1885813002/diff/40001/chrome/browser/printing...
chrome/browser/printing/pwg_raster_converter.cc:222:
utility_process_host_.reset();
I have no idea how this code works, was just reviewing for color WRT
safe_browsing, but AFAICT the only reason utility_process_host_ exists is for
reference in the ping callback.  So maybe it can stop being a member variable?

https://codereview.chromium.org/1885813002/diff/40001/chrome/browser/safe_bro...
File chrome/browser/safe_browsing/sandboxed_dmg_analyzer_mac.cc (right):

https://codereview.chromium.org/1885813002/diff/40001/chrome/browser/safe_bro...
chrome/browser/safe_browsing/sandboxed_dmg_analyzer_mac.cc:71: }
I suspect this may also no longer need to be an instance variable, though since
it's not reset() after, it's less obvious.  I guess it depends on how
utility_process_host_ operates internally.

[Anand Mistry (off Chromium), 2016-04-13 23:42:58]

https://codereview.chromium.org/1885813002/diff/40001/chrome/browser/printing...
File chrome/browser/printing/pwg_raster_converter.cc (right):

https://codereview.chromium.org/1885813002/diff/40001/chrome/browser/printing...
chrome/browser/printing/pwg_raster_converter.cc:222:
utility_process_host_.reset();
On 2016/04/13 17:44:43, Scott Hess wrote:
> I have no idea how this code works, was just reviewing for color WRT
> safe_browsing, but AFAICT the only reason utility_process_host_ exists is for
> reference in the ping callback.  So maybe it can stop being a member variable?

Done.

https://codereview.chromium.org/1885813002/diff/40001/chrome/browser/safe_bro...
File chrome/browser/safe_browsing/sandboxed_dmg_analyzer_mac.cc (right):

https://codereview.chromium.org/1885813002/diff/40001/chrome/browser/safe_bro...
chrome/browser/safe_browsing/sandboxed_dmg_analyzer_mac.cc:71: }
On 2016/04/13 17:44:43, Scott Hess wrote:
> I suspect this may also no longer need to be an instance variable, though
since
> it's not reset() after, it's less obvious.  I guess it depends on how
> utility_process_host_ operates internally.

It's unneeded (and reset does nothing since it's a weakptr). Removed.

[Scott Hess - ex-Googler, 2016-04-13 23:50:41]

LGTM, with the change suggested.

Given that I have no idea what any of this means, I sure hope the result still
works :-).

https://codereview.chromium.org/1885813002/diff/60001/chrome/browser/printing...
File chrome/browser/printing/pwg_raster_converter.cc (right):

https://codereview.chromium.org/1885813002/diff/60001/chrome/browser/printing...
chrome/browser/printing/pwg_raster_converter.cc:213:
content::UtilityProcessHost* utility_process_host_ =
Now it should be utility_process_host.

Sorry :-).

https://codereview.chromium.org/1885813002/diff/60001/chrome/browser/safe_bro...
File chrome/browser/safe_browsing/sandboxed_dmg_analyzer_mac.cc (right):

https://codereview.chromium.org/1885813002/diff/60001/chrome/browser/safe_bro...
chrome/browser/safe_browsing/sandboxed_dmg_analyzer_mac.cc:61:
content::UtilityProcessHost* utility_process_host_ =
Also here.

[Anand Mistry (off Chromium), 2016-04-14 00:00:07]

https://codereview.chromium.org/1885813002/diff/60001/chrome/browser/printing...
File chrome/browser/printing/pwg_raster_converter.cc (right):

https://codereview.chromium.org/1885813002/diff/60001/chrome/browser/printing...
chrome/browser/printing/pwg_raster_converter.cc:213:
content::UtilityProcessHost* utility_process_host_ =
On 2016/04/13 23:50:41, Scott Hess wrote:
> Now it should be utility_process_host.
> 
> Sorry :-).

Oops.

https://codereview.chromium.org/1885813002/diff/60001/chrome/browser/safe_bro...
File chrome/browser/safe_browsing/sandboxed_dmg_analyzer_mac.cc (right):

https://codereview.chromium.org/1885813002/diff/60001/chrome/browser/safe_bro...
chrome/browser/safe_browsing/sandboxed_dmg_analyzer_mac.cc:61:
content::UtilityProcessHost* utility_process_host_ =
On 2016/04/13 23:50:41, Scott Hess wrote:
> Also here.

Done.

[Scott Hess - ex-Googler, 2016-04-14 19:06:52]

LGTM for your changes, in case you are awaiting a final LGTM.  LGTM!

[Anand Mistry (off Chromium), 2016-04-15 03:47:31]

On 2016/04/14 19:06:52, Scott Hess wrote:
> LGTM for your changes, in case you are awaiting a final LGTM.  LGTM!

Nah. Just waiting to see if Scott Byer wants to comment. There's nothing urgent
about this cl. We can all just chill.

[Scott Byer, 2016-04-15 16:16:48]

lgtm

[Anand Mistry (off Chromium), 2016-04-17 23:27:24]

The CQ bit was checked by amistry@chromium.org

[Anand Mistry (off Chromium), 2016-04-17 23:27:25]

The patchset sent to the CQ was uploaded after l-g-t-m from
thestig@chromium.org, jorgelo@chromium.org, tzik@chromium.org,
scottbyer@chromium.org, shess@chromium.org
Link to the patchset: https://codereview.chromium.org/1885813002/#ps100001
(title: "Rebase")

[commit-bot: I haz the power, 2016-04-17 23:27:34]

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

[commit-bot: I haz the power, 2016-04-18 00:12:10]

Description was changed from

==========
Delete the utility process startup ping.

It's no longer necessary since IPC::TakePlatformFileForTransit() doesn't
require the child process handle.

BUG=597124
==========

to

==========
Delete the utility process startup ping.

It's no longer necessary since IPC::TakePlatformFileForTransit() doesn't
require the child process handle.

BUG=597124
==========

[commit-bot: I haz the power, 2016-04-18 00:12:12]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-04-18 00:13:47]

Description was changed from

==========
Delete the utility process startup ping.

It's no longer necessary since IPC::TakePlatformFileForTransit() doesn't
require the child process handle.

BUG=597124
==========

to

==========
Delete the utility process startup ping.

It's no longer necessary since IPC::TakePlatformFileForTransit() doesn't
require the child process handle.

BUG=597124

Committed: https://crrev.com/1bd8d3bba0e883748dcbf7c89c335c9864b39f2f
Cr-Commit-Position: refs/heads/master@{#387835}
==========

[commit-bot: I haz the power, 2016-04-18 00:13:49]

Patchset 6 (id:??) landed as
https://crrev.com/1bd8d3bba0e883748dcbf7c89c335c9864b39f2f
Cr-Commit-Position: refs/heads/master@{#387835}