Convert utility process extension ParseUpdate IPC to mojo

Convert utility process extension ParseUpdate IPC to mojo

Add ManifestParser mojo, used to parse an extensions update XML
manifest document, and expose it to the browser via the utility
process policy file. Change the caller to use a utility process
mojo client. Add mojo structure traits to handle the API return
values UpdateManifest::{Results,Result}. Remove the IPC message
file: no longer needed or used.

Covered by tests: ExtensionManagementTest.ExternalUrlUpdate and
ExtensionManagementTest.AutoUpdate and others. Refer to [1] for
a complete list.

[1] https://codereview.chromium.org/2761763002

BUG=691410
Review-Url: https://codereview.chromium.org/2699663003
Cr-Commit-Position: refs/heads/master@{#460280}
Committed: https://chromium.googlesource.com/chromium/src/+/825202ab2ec8a1d8eba2a08f481773e07641dda3

[Noel Gordon, 2017-02-16 01:27:24]

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

[commit-bot: I haz the power, 2017-02-16 01:29:18]

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

[Noel Gordon, 2017-02-16 01:38:44]

Description was changed from

==========
Convert utility process extension ParseUpdate IPC to mojo

BUG=691410

patch from issue 2697463002 at patchset 80001
(http://crrev.com/2697463002#ps80001)
==========

to

==========
Convert utility process extension ParseUpdate IPC to mojo

Depends on http://crrev.com/2697463002

BUG=691410

patch from issue 2697463002 at patchset 80001
(http://crrev.com/2697463002#ps80001)
==========

[Noel Gordon, 2017-02-16 01:43:48]

Description was changed from

==========
Convert utility process extension ParseUpdate IPC to mojo

Depends on http://crrev.com/2697463002

BUG=691410

patch from issue 2697463002 at patchset 80001
(http://crrev.com/2697463002#ps80001)
==========

to

==========
Convert utility process extension ParseUpdate IPC to mojo

Depends on http://crrev.com/2697463002

BUG=691410

patch from issue 2697463002 at patchset ps120001
(https://codereview.chromium.org/2697463002/#ps120001)
==========

[commit-bot: I haz the power, 2017-02-16 01:53:39]

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

[commit-bot: I haz the power, 2017-02-16 01:53:40]

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

[Noel Gordon, 2017-02-16 03:51:40]

On 2017/02/16 01:53:40, commit-bot: I haz the power wrote:
> Dry run: Try jobs failed on following builders:
>   mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

Mojo struct traits: UpdateManifest::Results has an inline copy constructor. 
Clang compilers do not like it, nor does Chromium style.

Fix: define the copy constructor in the .h, and provide an out-of-line impl in
the .cc

[Noel Gordon, 2017-02-16 07:32:26]

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

[commit-bot: I haz the power, 2017-02-16 07:33:31]

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

[Noel Gordon, 2017-02-16 07:34:11]

Description was changed from

==========
Convert utility process extension ParseUpdate IPC to mojo

Depends on http://crrev.com/2697463002

BUG=691410

patch from issue 2697463002 at patchset ps120001
(https://codereview.chromium.org/2697463002/#ps120001)
==========

to

==========
Convert utility process extension ParseUpdate IPC to mojo

Depends on http://crrev.com/2697463002

BUG=691410
==========

[commit-bot: I haz the power, 2017-02-16 07:38:09]

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

[commit-bot: I haz the power, 2017-02-16 07:38:10]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  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...)
  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_...)

[Noel Gordon, 2017-02-16 07:47:50]

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

[commit-bot: I haz the power, 2017-02-16 07:49:05]

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-16 07:52:28]

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

[commit-bot: I haz the power, 2017-02-16 07:52:29]

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

[Noel Gordon, 2017-02-16 08:00:25]

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

[commit-bot: I haz the power, 2017-02-16 08:01: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-02-16 08:04:10]

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

[commit-bot: I haz the power, 2017-02-16 08:04:11]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[Noel Gordon, 2017-02-16 09:43:30]

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

[Noel Gordon, 2017-02-16 09:43:56]

The CQ bit was unchecked by noel@chromium.org

[Noel Gordon, 2017-02-16 09:44:02]

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

[commit-bot: I haz the power, 2017-02-16 09:44:08]

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-16 09:48:35]

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

[commit-bot: I haz the power, 2017-02-16 09:48:36]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  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-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  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_...)

[Noel Gordon, 2017-02-16 14:37:10]

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

[commit-bot: I haz the power, 2017-02-16 14:37:50]

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-16 14:54:39]

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

[commit-bot: I haz the power, 2017-02-16 14:54:40]

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

[Noel Gordon, 2017-02-16 15:47:27]

Patchset #4 (id:60001) has been deleted

[Noel Gordon, 2017-02-16 15:56:59]

Patchset #4 (id:80001) has been deleted

[Noel Gordon, 2017-02-16 15:59:53]

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

[commit-bot: I haz the power, 2017-02-16 16:00:25]

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-16 16:03:41]

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

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

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

[Noel Gordon, 2017-02-16 21:36:28]

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

[commit-bot: I haz the power, 2017-02-16 21:37:41]

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-16 21:43:10]

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

[commit-bot: I haz the power, 2017-02-16 21:43:11]

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

[Noel Gordon, 2017-02-17 00:31:05]

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

[commit-bot: I haz the power, 2017-02-17 00:33:24]

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-17 00:39:10]

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

[commit-bot: I haz the power, 2017-02-17 00:39:12]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  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...)
  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_...)

[Noel Gordon, 2017-02-17 00:40:06]

The CQ bit was unchecked by noel@chromium.org

[Noel Gordon, 2017-02-17 01:02:03]

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

[commit-bot: I haz the power, 2017-02-17 01:04:25]

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

[Noel Gordon, 2017-02-17 01:26:49]

Patchset #5 (id:120001) has been deleted

[Noel Gordon, 2017-02-17 01:45:09]

Patchset #5 (id:140001) has been deleted

[Noel Gordon, 2017-02-17 05:26:16]

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

[commit-bot: I haz the power, 2017-02-17 05:26:41]

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-17 06:52:22]

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

[commit-bot: I haz the power, 2017-02-17 06:52:23]

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

[Noel Gordon, 2017-02-17 07:06:37]

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

[commit-bot: I haz the power, 2017-02-17 07:07:08]

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-17 08:19:49]

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

[commit-bot: I haz the power, 2017-02-17 08:19:50]

Dry run: This issue passed the CQ dry run.

[Noel Gordon, 2017-02-19 14:55:02]

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

[commit-bot: I haz the power, 2017-02-19 14:55:06]

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 15:11:10]

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

[commit-bot: I haz the power, 2017-02-19 15:11:11]

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

[Noel Gordon, 2017-02-19 22:22:18]

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

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

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 22:26:33]

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

[commit-bot: I haz the power, 2017-02-19 22:26:34]

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

[Noel Gordon, 2017-02-23 15:23:04]

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

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

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-23 16:29:00]

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

[commit-bot: I haz the power, 2017-02-23 16:29:01]

Dry run: This issue passed the CQ dry run.

[Noel Gordon, 2017-03-06 13:33:34]

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

[commit-bot: I haz the power, 2017-03-06 13:33:45]

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-06 14:39:00]

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

[commit-bot: I haz the power, 2017-03-06 14:39:01]

Dry run: This issue passed the CQ dry run.

[Noel Gordon, 2017-03-19 22:46:10]

Patchset #3 (id:40001) has been deleted

[Noel Gordon, 2017-03-19 23:02:29]

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

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

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-20 00:43:24]

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

[commit-bot: I haz the power, 2017-03-20 00:43:26]

Dry run: This issue passed the CQ dry run.

[Noel Gordon, 2017-03-20 02:46:35]

noel@chromium.org changed reviewers:
+ mbarbella@chromium.org, sammc@chromium.org, tibell@chromium.org

[Noel Gordon, 2017-03-20 02:46:36]

+ sammc, tibell for mojo, +martin for IPC security

[Sam McNally, 2017-03-20 04:22:06]

https://codereview.chromium.org/2699663003/diff/240001/extensions/common/upda...
File extensions/common/update_manifest.h (right):

https://codereview.chromium.org/2699663003/diff/240001/extensions/common/upda...
extensions/common/update_manifest.h:67: Results(const Results& other);
Why?

https://codereview.chromium.org/2699663003/diff/240001/extensions/utility/uti...
File extensions/utility/utility_handler.h (right):

https://codereview.chromium.org/2699663003/diff/240001/extensions/utility/uti...
extensions/utility/utility_handler.h:16: static void UtilityThreadStarted();
These should be functions.

[Noel Gordon, 2017-03-20 05:53:57]

https://codereview.chromium.org/2699663003/diff/240001/extensions/common/upda...
File extensions/common/update_manifest.h (right):

https://codereview.chromium.org/2699663003/diff/240001/extensions/common/upda...
extensions/common/update_manifest.h:67: Results(const Results& other);
On 2017/03/20 04:22:06, Sam McNally wrote:
> Why?

So the Results in the utility process can be passed directly into the mojo
callback.

https://codereview.chromium.org/2699663003/diff/240001/extensions/utility/uti...
File extensions/utility/utility_handler.h (right):

https://codereview.chromium.org/2699663003/diff/240001/extensions/utility/uti...
extensions/utility/utility_handler.h:16: static void UtilityThreadStarted();
On 2017/03/20 04:22:06, Sam McNally wrote:
> These should be functions.

They are functions, so perhaps you're asking about something else.

[Sam McNally, 2017-03-20 05:56:26]

https://codereview.chromium.org/2699663003/diff/240001/extensions/common/upda...
File extensions/common/update_manifest.h (right):

https://codereview.chromium.org/2699663003/diff/240001/extensions/common/upda...
extensions/common/update_manifest.h:67: Results(const Results& other);
On 2017/03/20 05:53:57, noel gordon wrote:
> On 2017/03/20 04:22:06, Sam McNally wrote:
> > Why?
> 
> So the Results in the utility process can be passed directly into the mojo
> callback.

Add an assignment operator too.

https://codereview.chromium.org/2699663003/diff/240001/extensions/utility/uti...
File extensions/utility/utility_handler.h (right):

https://codereview.chromium.org/2699663003/diff/240001/extensions/utility/uti...
extensions/utility/utility_handler.h:16: static void UtilityThreadStarted();
On 2017/03/20 05:53:57, noel gordon wrote:
> On 2017/03/20 04:22:06, Sam McNally wrote:
> > These should be functions.
> 
> They are functions, so perhaps you're asking about something else.

https://google.github.io/styleguide/cppguide.html#Nonmember,_Static_Member,_a...

[Noel Gordon, 2017-03-20 11:32:53]

https://codereview.chromium.org/2699663003/diff/240001/extensions/common/upda...
File extensions/common/update_manifest.h (right):

https://codereview.chromium.org/2699663003/diff/240001/extensions/common/upda...
extensions/common/update_manifest.h:67: Results(const Results& other);
On 2017/03/20 05:56:26, Sam McNally wrote:
> On 2017/03/20 05:53:57, noel gordon wrote:
> > On 2017/03/20 04:22:06, Sam McNally wrote:
> > > Why?
> > 
> > So the Results in the utility process can be passed directly into the mojo
> > callback.
> 
> Add an assignment operator too.

Ok, added Results operator=().

https://codereview.chromium.org/2699663003/diff/240001/extensions/utility/uti...
File extensions/utility/utility_handler.h (right):

https://codereview.chromium.org/2699663003/diff/240001/extensions/utility/uti...
extensions/utility/utility_handler.h:16: static void UtilityThreadStarted();
On 2017/03/20 05:56:26, Sam McNally wrote:
> On 2017/03/20 05:53:57, noel gordon wrote:
> > On 2017/03/20 04:22:06, Sam McNally wrote:
> > > These should be functions.
> > 
> > They are functions, so perhaps you're asking about something else.
> 
>
https://google.github.io/styleguide/cppguide.html#Nonmember,_Static_Member,_a...

Done, moved these helpers into namespace utility_handler.

[Noel Gordon, 2017-03-20 11:34:13]

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

[commit-bot: I haz the power, 2017-03-20 11:34:26]

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

[Noel Gordon, 2017-03-20 12:02:42]

Description was changed from

==========
Convert utility process extension ParseUpdate IPC to mojo

Depends on http://crrev.com/2697463002

BUG=691410
==========

to

==========
Convert utility process extension ParseUpdate IPC to mojo

BUG=691410
==========

[commit-bot: I haz the power, 2017-03-20 12:34:13]

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

[commit-bot: I haz the power, 2017-03-20 12:34:14]

Dry run: This issue passed the CQ dry run.

[tibell, 2017-03-20 22:55:36]

lgtm

https://codereview.chromium.org/2699663003/diff/260001/extensions/browser/upd...
File extensions/browser/updater/safe_manifest_parser.h (right):

https://codereview.chromium.org/2699663003/diff/260001/extensions/browser/upd...
extensions/browser/updater/safe_manifest_parser.h:16: // |callback| with the
results. Runs on the UI thread.
Perhaps add: The results will be null on failure.

[Noel Gordon, 2017-03-20 23:20:57]

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

[Noel Gordon, 2017-03-20 23:21:08]

https://codereview.chromium.org/2699663003/diff/260001/extensions/browser/upd...
File extensions/browser/updater/safe_manifest_parser.h (right):

https://codereview.chromium.org/2699663003/diff/260001/extensions/browser/upd...
extensions/browser/updater/safe_manifest_parser.h:16: // |callback| with the
results. Runs on the UI thread.
On 2017/03/20 22:55:36, tibell wrote:
> Perhaps add: The results will be null on failure.

Done.

[commit-bot: I haz the power, 2017-03-20 23:21:46]

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

[Martin Barbella, 2017-03-20 23:51:01]

security lgtm

[Sam McNally, 2017-03-20 23:54:57]

lgtm

[commit-bot: I haz the power, 2017-03-21 00:55:23]

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

[commit-bot: I haz the power, 2017-03-21 00:55:24]

Dry run: This issue passed the CQ dry run.

[Noel Gordon, 2017-03-21 01:57:28]

noel@chromium.org changed reviewers:
+ rdevlin.cronin@chromium.org

[Noel Gordon, 2017-03-21 01:57:29]

+devlin for extensions.

[Noel Gordon, 2017-03-21 23:09:28]

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

[commit-bot: I haz the power, 2017-03-21 23:10:02]

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-21 23:19:07]

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

[commit-bot: I haz the power, 2017-03-21 23:19:09]

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

[Devlin, 2017-03-21 23:51:17]

lgtm

https://codereview.chromium.org/2699663003/diff/300001/extensions/browser/upd...
File extensions/browser/updater/safe_manifest_parser.cc (right):

https://codereview.chromium.org/2699663003/diff/300001/extensions/browser/upd...
extensions/browser/updater/safe_manifest_parser.cc:22: using ResultCallback =
base::Callback<void(const UpdateManifest::Results*)>;
Can we move this into the .h to use it in the function declaration as well?

https://codereview.chromium.org/2699663003/diff/300001/extensions/common/mani...
File extensions/common/manifest_parser.typemap (right):

https://codereview.chromium.org/2699663003/diff/300001/extensions/common/mani...
extensions/common/manifest_parser.typemap:16:
"extensions.mojom.UpdateManifestResults=::UpdateManifest::Results",
I wonder if it would make sense to just use the mojo types instead of
UpdateManifest::Results?  It would save on conversion/copies, and they're just
data structs... maybe add a TODO somewhere? (In UpdateManifest?)

https://codereview.chromium.org/2699663003/diff/300001/extensions/utility/uti...
File extensions/utility/utility_handler.h (right):

https://codereview.chromium.org/2699663003/diff/300001/extensions/utility/uti...
extensions/utility/utility_handler.h:14: namespace utility_handler {
nit: \n

https://codereview.chromium.org/2699663003/diff/300001/extensions/utility/uti...
extensions/utility/utility_handler.h:18: bool running_elevated);
nit: \n

[Noel Gordon, 2017-03-21 23:56:29]

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

[commit-bot: I haz the power, 2017-03-21 23:56:45]

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

[Noel Gordon, 2017-03-22 01:34:51]

https://codereview.chromium.org/2699663003/diff/300001/extensions/browser/upd...
File extensions/browser/updater/safe_manifest_parser.cc (right):

https://codereview.chromium.org/2699663003/diff/300001/extensions/browser/upd...
extensions/browser/updater/safe_manifest_parser.cc:22: using ResultCallback =
base::Callback<void(const UpdateManifest::Results*)>;
On 2017/03/21 23:51:16, Devlin wrote:
> Can we move this into the .h to use it in the function declaration as well?

This is a convenience using (alias), local to the .cc of course.  If it was
moved into the .h, we'd expose its type ResultCallback in the extensions
namespace.  Feeling was that that type was maybe too general to be exposed at
extensions namespace level.

https://codereview.chromium.org/2699663003/diff/300001/extensions/common/mani...
File extensions/common/manifest_parser.typemap (right):

https://codereview.chromium.org/2699663003/diff/300001/extensions/common/mani...
extensions/common/manifest_parser.typemap:16:
"extensions.mojom.UpdateManifestResults=::UpdateManifest::Results",
On 2017/03/21 23:51:16, Devlin wrote:
> I wonder if it would make sense to just use the mojo types instead of
> UpdateManifest::Results?

You might recall the recent enum Manifest::location change we did.  We did think
about re-writing callers in terms of the mojo enum type (didn't mention it as
option in review), but there were many call-sites in extensions that would'a
needed updating to do that (too many for that particular patch set).

But "just use the mojo types instead" is always good thing to consider, given
...

> It would save on conversion/copies, and they're just
> data structs... 

and, furthermore, saves you having to decide upon using [Native] traits, or else
building mojo struct traits, etc, etc.  So indeed, lots of benefits and,
assuming callers / call-sites can be easily updated to use the mojo type(s),
it's a good way to go ...

> maybe add a TODO somewhere? (In UpdateManifest?)

... or consider: TODO done.

https://codereview.chromium.org/2699663003/diff/300001/extensions/utility/uti...
File extensions/utility/utility_handler.h (right):

https://codereview.chromium.org/2699663003/diff/300001/extensions/utility/uti...
extensions/utility/utility_handler.h:14: namespace utility_handler {
On 2017/03/21 23:51:17, Devlin wrote:
> nit: \n

Done.

https://codereview.chromium.org/2699663003/diff/300001/extensions/utility/uti...
extensions/utility/utility_handler.h:18: bool running_elevated);
On 2017/03/21 23:51:17, Devlin wrote:
> nit: \n

Done.

[Noel Gordon, 2017-03-22 01:36:06]

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

[commit-bot: I haz the power, 2017-03-22 01:37:30]

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-22 03:39:39]

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

[commit-bot: I haz the power, 2017-03-22 03:39:40]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_TIMED_OUT, build hasn't
started yet, builder probably lacks capacity)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_TIMED_OUT,
build hasn't started yet, builder probably lacks capacity)

[Noel Gordon, 2017-03-22 04:04:57]

+ jochen for OWNERS

What's test cover this change, a shed-load it seems [1].

[1] https://codereview.chromium.org/2761763002

[Noel Gordon, 2017-03-22 04:28:43]

On 2017/03/22 04:04:57, noel gordon wrote:
> + jochen for OWNERS
> 
> What's test cover this change, a shed-load it seems [1].
> 
> [1] https://codereview.chromium.org/2761763002

"What tests cover this change?" /even.

[Noel Gordon, 2017-03-22 04:45:59]

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

[commit-bot: I haz the power, 2017-03-22 04:46:40]

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-22 06:33:11]

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

[commit-bot: I haz the power, 2017-03-22 06:33:12]

Dry run: This issue passed the CQ dry run.

[Noel Gordon, 2017-03-23 15:43:41]

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

[jochen (gone - plz use gerrit), 2017-03-27 12:36:10]

lgtm

[Noel Gordon, 2017-03-28 23:59:50]

Description was changed from

==========
Convert utility process extension ParseUpdate IPC to mojo

BUG=691410
==========

to

==========
Convert utility process extension ParseUpdate IPC to mojo

Add ManifestParser mojo, used to parse an extensions update XML
manifest document, and expose it to the browser via the utility
process policy file. Change the caller to use a utility process
mojo client. Add mojo structure traits to handle the API return
values UpdateManifest::{Results,Result}. Remove the IPC message
file (it's no longer needed or used).

Covered by tests: ExtensionManagementTest.ExternalUrlUpdate and
ExtensionManagementTest.AutoUpdate and others. Refer to [1] for
a complete list.

[1] https://codereview.chromium.org/2761763002

BUG=691410
==========

[Noel Gordon, 2017-03-29 00:00:20]

The CQ bit was checked by noel@chromium.org

[Noel Gordon, 2017-03-29 00:00:20]

The patchset sent to the CQ was uploaded after l-g-t-m from tibell@chromium.org,
sammc@chromium.org, mbarbella@chromium.org, rdevlin.cronin@chromium.org
Link to the patchset: https://codereview.chromium.org/2699663003/#ps320001
(title: "Extensions review comments.")

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

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-29 01:43:08]

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

[commit-bot: I haz the power, 2017-03-29 01:43:10]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Noel Gordon, 2017-03-29 01:47:40]

The CQ bit was checked by noel@chromium.org

[commit-bot: I haz the power, 2017-03-29 01:48:25]

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

[Noel Gordon, 2017-03-29 01:54:01]

Description was changed from

==========
Convert utility process extension ParseUpdate IPC to mojo

Add ManifestParser mojo, used to parse an extensions update XML
manifest document, and expose it to the browser via the utility
process policy file. Change the caller to use a utility process
mojo client. Add mojo structure traits to handle the API return
values UpdateManifest::{Results,Result}. Remove the IPC message
file (it's no longer needed or used).

Covered by tests: ExtensionManagementTest.ExternalUrlUpdate and
ExtensionManagementTest.AutoUpdate and others. Refer to [1] for
a complete list.

[1] https://codereview.chromium.org/2761763002

BUG=691410
==========

to

==========
Convert utility process extension ParseUpdate IPC to mojo

Add ManifestParser mojo, used to parse an extensions update XML
manifest document, and expose it to the browser via the utility
process policy file. Change the caller to use a utility process
mojo client. Add mojo structure traits to handle the API return
values UpdateManifest::{Results,Result}. Remove the IPC message
file: no longer needed or used.

Covered by tests: ExtensionManagementTest.ExternalUrlUpdate and
ExtensionManagementTest.AutoUpdate and others. Refer to [1] for
a complete list.

[1] https://codereview.chromium.org/2761763002

BUG=691410
==========

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

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

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

Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_TIMED_OUT, build hasn't
started yet, builder probably lacks capacity)

[Noel Gordon, 2017-03-29 02:56:21]

The CQ bit was checked by noel@chromium.org

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

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-29 04:37:24]

CQ is committing da patch.

Bot data: {"patchset_id": 320001, "attempt_start_ts": 1490756181901580,
"parent_rev": "ce3ccb9d8093a1eb542098f2e68fd51c90fa989d", "commit_rev":
"825202ab2ec8a1d8eba2a08f481773e07641dda3"}

[commit-bot: I haz the power, 2017-03-29 04:38:08]

Description was changed from

==========
Convert utility process extension ParseUpdate IPC to mojo

Add ManifestParser mojo, used to parse an extensions update XML
manifest document, and expose it to the browser via the utility
process policy file. Change the caller to use a utility process
mojo client. Add mojo structure traits to handle the API return
values UpdateManifest::{Results,Result}. Remove the IPC message
file: no longer needed or used.

Covered by tests: ExtensionManagementTest.ExternalUrlUpdate and
ExtensionManagementTest.AutoUpdate and others. Refer to [1] for
a complete list.

[1] https://codereview.chromium.org/2761763002

BUG=691410
==========

to

==========
Convert utility process extension ParseUpdate IPC to mojo

Add ManifestParser mojo, used to parse an extensions update XML
manifest document, and expose it to the browser via the utility
process policy file. Change the caller to use a utility process
mojo client. Add mojo structure traits to handle the API return
values UpdateManifest::{Results,Result}. Remove the IPC message
file: no longer needed or used.

Covered by tests: ExtensionManagementTest.ExternalUrlUpdate and
ExtensionManagementTest.AutoUpdate and others. Refer to [1] for
a complete list.

[1] https://codereview.chromium.org/2761763002

BUG=691410

Review-Url: https://codereview.chromium.org/2699663003
Cr-Commit-Position: refs/heads/master@{#460280}
Committed:
https://chromium.googlesource.com/chromium/src/+/825202ab2ec8a1d8eba2a08f4817...
==========

[commit-bot: I haz the power, 2017-03-29 04:38:10]

Committed patchset #12 (id:320001) as
https://chromium.googlesource.com/chromium/src/+/825202ab2ec8a1d8eba2a08f4817...