Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(460)

Issue 2699663003: Convert utility process extension ParseUpdate IPC to mojo (Closed)

Created:
1 year, 2 months ago by Noel Gordon
Modified:
1 year ago
CC:
Aaron Boodman, abarth-chromium, catmullings, chromium-apps-reviews_chromium.org, chromium-reviews, darin (slow to review), darin-cc_chromium.org, extensions-reviews_chromium.org, jam, qsr+mojo_chromium.org, Devlin, viettrungluu+watch_chromium.org, yzshen+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

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

Patch Set 1 #

Patch Set 2 : Mojo Traits: Add an out-of-line UpdateManifest::Results copy constructor. #

Patch Set 3 : Rename updater to parser. #

Patch Set 4 : Workaround bot patch apply issue. #ifdef 0 the code to be git rm-ed. #

Patch Set 5 #

Patch Set 6 : Sync to ToT, see if git apply issue and try jobs are happy. #

Patch Set 7 : Sync up with dependent patch set. #

Patch Set 8 : Dependent patch http://crrev.com/2697463002 landed: sync up. #

Total comments: 8

Patch Set 9 : Add Results operator=, move static helpers into a namespace. #

Total comments: 2

Patch Set 10 : Add comment about Results* being null on failure. #

Patch Set 11 : Another change landed in this area, sync up to ToT. #

Total comments: 8

Patch Set 12 : Extensions review comments. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+294 lines, -258 lines) Patch
M chrome/browser/chrome_content_utility_manifest_overlay.json View 1 2 3 4 5 6 7 1 chunk +1 line, -0 lines 0 comments Download
M chrome/utility/chrome_content_utility_client.cc View 1 2 3 4 5 6 7 8 9 10 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/utility/extensions/extensions_handler.h View 1 2 3 4 5 6 7 4 chunks +2 lines, -5 lines 0 comments Download
M chrome/utility/extensions/extensions_handler.cc View 1 2 3 4 5 6 7 8 9 10 3 chunks +2 lines, -10 lines 0 comments Download
M extensions/browser/updater/extension_downloader.cc View 1 1 chunk +5 lines, -7 lines 0 comments Download
M extensions/browser/updater/safe_manifest_parser.h View 1 2 3 4 5 6 7 8 9 1 chunk +7 lines, -37 lines 0 comments Download
M extensions/browser/updater/safe_manifest_parser.cc View 1 2 3 4 5 6 7 1 chunk +28 lines, -55 lines 0 comments Download
M extensions/common/BUILD.gn View 1 2 3 4 5 6 7 2 chunks +2 lines, -0 lines 0 comments Download
M extensions/common/extension_message_generator.h View 1 chunk +0 lines, -1 line 0 comments Download
D extensions/common/extension_utility_messages.h View 1 4 5 6 7 1 chunk +0 lines, -48 lines 0 comments Download
A extensions/common/manifest_parser.mojom View 1 2 1 chunk +36 lines, -0 lines 0 comments Download
A extensions/common/manifest_parser.typemap View 1 2 1 chunk +18 lines, -0 lines 0 comments Download
A extensions/common/manifest_parser_struct_traits.h View 1 2 1 chunk +81 lines, -0 lines 0 comments Download
A extensions/common/manifest_parser_struct_traits.cc View 1 2 1 chunk +47 lines, -0 lines 0 comments Download
M extensions/common/typemaps.gni View 1 2 3 4 5 6 1 chunk +4 lines, -1 line 0 comments Download
M extensions/common/update_manifest.h View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +6 lines, -0 lines 0 comments Download
M extensions/common/update_manifest.cc View 1 2 3 4 5 6 7 8 1 chunk +10 lines, -8 lines 0 comments Download
M extensions/shell/utility/shell_content_utility_client.h View 1 chunk +0 lines, -4 lines 0 comments Download
M extensions/shell/utility/shell_content_utility_client.cc View 1 2 3 4 5 6 7 8 1 chunk +3 lines, -9 lines 0 comments Download
M extensions/test/data/extensions_test_content_utility_manifest_overlay.json View 1 2 3 4 5 1 chunk +2 lines, -1 line 0 comments Download
M extensions/test/test_content_utility_client.h View 1 1 chunk +0 lines, -4 lines 0 comments Download
M extensions/test/test_content_utility_client.cc View 1 2 3 4 5 6 7 8 1 chunk +2 lines, -6 lines 0 comments Download
M extensions/utility/utility_handler.h View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +5 lines, -25 lines 0 comments Download
M extensions/utility/utility_handler.cc View 1 2 3 4 5 6 7 8 9 10 2 chunks +31 lines, -35 lines 0 comments Download

Messages

Total messages: 136 (113 generated)
Noel Gordon
On 2017/02/16 01:53:40, commit-bot: I haz the power wrote: > Dry run: Try jobs failed ...
1 year, 2 months ago (2017-02-16 03:51:40 UTC) #7
Noel Gordon
+ sammc, tibell for mojo, +martin for IPC security
1 year, 1 month ago (2017-03-20 02:46:36 UTC) #80
Sam McNally
https://codereview.chromium.org/2699663003/diff/240001/extensions/common/update_manifest.h File extensions/common/update_manifest.h (right): https://codereview.chromium.org/2699663003/diff/240001/extensions/common/update_manifest.h#newcode67 extensions/common/update_manifest.h:67: Results(const Results& other); Why? https://codereview.chromium.org/2699663003/diff/240001/extensions/utility/utility_handler.h File extensions/utility/utility_handler.h (right): https://codereview.chromium.org/2699663003/diff/240001/extensions/utility/utility_handler.h#newcode16 ...
1 year, 1 month ago (2017-03-20 04:22:06 UTC) #81
Noel Gordon
https://codereview.chromium.org/2699663003/diff/240001/extensions/common/update_manifest.h File extensions/common/update_manifest.h (right): https://codereview.chromium.org/2699663003/diff/240001/extensions/common/update_manifest.h#newcode67 extensions/common/update_manifest.h:67: Results(const Results& other); On 2017/03/20 04:22:06, Sam McNally wrote: ...
1 year, 1 month ago (2017-03-20 05:53:57 UTC) #82
Sam McNally
https://codereview.chromium.org/2699663003/diff/240001/extensions/common/update_manifest.h File extensions/common/update_manifest.h (right): https://codereview.chromium.org/2699663003/diff/240001/extensions/common/update_manifest.h#newcode67 extensions/common/update_manifest.h:67: Results(const Results& other); On 2017/03/20 05:53:57, noel gordon wrote: ...
1 year, 1 month ago (2017-03-20 05:56:26 UTC) #83
Noel Gordon
https://codereview.chromium.org/2699663003/diff/240001/extensions/common/update_manifest.h File extensions/common/update_manifest.h (right): https://codereview.chromium.org/2699663003/diff/240001/extensions/common/update_manifest.h#newcode67 extensions/common/update_manifest.h:67: Results(const Results& other); On 2017/03/20 05:56:26, Sam McNally wrote: ...
1 year, 1 month ago (2017-03-20 11:32:53 UTC) #84
tibell
lgtm https://codereview.chromium.org/2699663003/diff/260001/extensions/browser/updater/safe_manifest_parser.h File extensions/browser/updater/safe_manifest_parser.h (right): https://codereview.chromium.org/2699663003/diff/260001/extensions/browser/updater/safe_manifest_parser.h#newcode16 extensions/browser/updater/safe_manifest_parser.h:16: // |callback| with the results. Runs on the ...
1 year, 1 month ago (2017-03-20 22:55:36 UTC) #90
Noel Gordon
https://codereview.chromium.org/2699663003/diff/260001/extensions/browser/updater/safe_manifest_parser.h File extensions/browser/updater/safe_manifest_parser.h (right): https://codereview.chromium.org/2699663003/diff/260001/extensions/browser/updater/safe_manifest_parser.h#newcode16 extensions/browser/updater/safe_manifest_parser.h:16: // |callback| with the results. Runs on the UI ...
1 year, 1 month ago (2017-03-20 23:21:08 UTC) #92
Martin Barbella
security lgtm
1 year, 1 month ago (2017-03-20 23:51:01 UTC) #94
Sam McNally
lgtm
1 year, 1 month ago (2017-03-20 23:54:57 UTC) #95
Noel Gordon
+devlin for extensions.
1 year, 1 month ago (2017-03-21 01:57:29 UTC) #99
Devlin
lgtm https://codereview.chromium.org/2699663003/diff/300001/extensions/browser/updater/safe_manifest_parser.cc File extensions/browser/updater/safe_manifest_parser.cc (right): https://codereview.chromium.org/2699663003/diff/300001/extensions/browser/updater/safe_manifest_parser.cc#newcode22 extensions/browser/updater/safe_manifest_parser.cc:22: using ResultCallback = base::Callback<void(const UpdateManifest::Results*)>; Can we move ...
1 year, 1 month ago (2017-03-21 23:51:17 UTC) #104
Noel Gordon
https://codereview.chromium.org/2699663003/diff/300001/extensions/browser/updater/safe_manifest_parser.cc File extensions/browser/updater/safe_manifest_parser.cc (right): https://codereview.chromium.org/2699663003/diff/300001/extensions/browser/updater/safe_manifest_parser.cc#newcode22 extensions/browser/updater/safe_manifest_parser.cc:22: using ResultCallback = base::Callback<void(const UpdateManifest::Results*)>; On 2017/03/21 23:51:16, Devlin ...
1 year, 1 month ago (2017-03-22 01:34:51 UTC) #107
Noel Gordon
+ jochen for OWNERS What's test cover this change, a shed-load it seems [1]. [1] ...
1 year, 1 month ago (2017-03-22 04:04:57 UTC) #112
Noel Gordon
On 2017/03/22 04:04:57, noel gordon wrote: > + jochen for OWNERS > > What's test ...
1 year, 1 month ago (2017-03-22 04:28:43 UTC) #113
Noel Gordon
1 year ago (2017-03-23 15:43:42 UTC) #119
jochen (gone - plz use gerrit)
lgtm
1 year ago (2017-03-27 12:36:10 UTC) #120
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2699663003/320001
1 year ago (2017-03-29 00:01:05 UTC) #124
commit-bot: I haz the power
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_android_rel_ng/builds/259811)
1 year ago (2017-03-29 01:43:10 UTC) #126
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2699663003/320001
1 year ago (2017-03-29 01:48:25 UTC) #128
commit-bot: I haz the power
Try jobs failed on following builders: ios-simulator on master.tryserver.chromium.mac (JOB_TIMED_OUT, build hasn't started yet, builder ...
1 year ago (2017-03-29 02:05:24 UTC) #131
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2699663003/320001
1 year ago (2017-03-29 02:57:09 UTC) #133
commit-bot: I haz the power
1 year ago (2017-03-29 04:38:10 UTC) #136
Message was sent while issue was closed.
Committed patchset #12 (id:320001) as
https://chromium.googlesource.com/chromium/src/+/825202ab2ec8a1d8eba2a08f4817...

Powered by Google App Engine
This is Rietveld 408576698