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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 months, 1 week ago by noel gordon
Modified:
2 months 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 (US Holiday May 29), 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
Trybot results:  linux_android_rel_ng   linux_chromium_rel_ng   win_chromium_x64_rel_ng   ios-simulator   linux_chromium_chromeos_ozone_rel_ng   linux_chromium_asan_rel_ng   win_chromium_rel_ng   chromium_presubmit   linux_chromium_tsan_rel_ng   chromium_presubmit   linux_android_rel_ng   win_chromium_rel_ng   win_clang   win_clang   win_chromium_x64_rel_ng   mac_chromium_rel_ng   win_chromium_compile_dbg_ng   win_chromium_rel_ng   mac_chromium_compile_dbg_ng   ios-simulator-xcode-clang   ios-simulator   ios-device-xcode-clang   ios-device   linux_chromium_rel_ng   linux_chromium_compile_dbg_ng   linux_chromium_tsan_rel_ng   linux_chromium_chromeos_rel_ng   linux_chromium_chromeos_ozone_rel_ng   linux_chromium_asan_rel_ng   chromium_presubmit   chromeos_daisy_chromium_compile_only_ng   chromeos_amd64-generic_chromium_compile_only_ng   cast_shell_linux   cast_shell_android   linux_android_rel_ng   android_n5x_swarming_rel   android_cronet   android_clang_dbg_recipe   android_compile_dbg   android_arm64_dbg_recipe   chromium_presubmit   ios-simulator-xcode-clang   ios-simulator   linux_android_rel_ng   mac_chromium_compile_dbg_ng   linux_chromium_compile_dbg_ng   cast_shell_linux   linux_chromium_tsan_rel_ng   linux_chromium_rel_ng   ios-device-xcode-clang   linux_chromium_asan_rel_ng   win_chromium_x64_rel_ng   android_n5x_swarming_rel   ios-simulator-xcode-clang   linux_chromium_chromeos_rel_ng   ios-simulator   win_chromium_rel_ng   mac_chromium_rel_ng   chromeos_amd64-generic_chromium_compile_only_ng   android_cronet   android_arm64_dbg_recipe   ios-device   android_clang_dbg_recipe   chromeos_daisy_chromium_compile_only_ng   chromium_presubmit   cast_shell_android   linux_chromium_chromeos_ozone_rel_ng   android_compile_dbg   win_chromium_compile_dbg_ng   win_clang 
Commit queue not available (can’t edit this change).

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 ...
3 months, 1 week ago (2017-02-16 03:51:40 UTC) #7
noel gordon
+ sammc, tibell for mojo, +martin for IPC security
2 months, 1 week 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 ...
2 months, 1 week 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: ...
2 months, 1 week 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: ...
2 months, 1 week 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: ...
2 months, 1 week 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 ...
2 months, 1 week 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 ...
2 months, 1 week ago (2017-03-20 23:21:08 UTC) #92
Martin Barbella
security lgtm
2 months, 1 week ago (2017-03-20 23:51:01 UTC) #94
Sam McNally
lgtm
2 months, 1 week ago (2017-03-20 23:54:57 UTC) #95
noel gordon
+devlin for extensions.
2 months, 1 week ago (2017-03-21 01:57:29 UTC) #99
Devlin (US Holiday May 29)
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 ...
2 months, 1 week 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 ...
2 months, 1 week 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] ...
2 months, 1 week 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 ...
2 months, 1 week ago (2017-03-22 04:28:43 UTC) #113
noel gordon
2 months, 1 week ago (2017-03-23 15:43:42 UTC) #119
jochen
lgtm
2 months 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
2 months 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)
2 months 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
2 months 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 ...
2 months 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
2 months ago (2017-03-29 02:57:09 UTC) #133
commit-bot: I haz the power
2 months 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...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 650457f06