Chromium Code Reviews
Help | Chromium Project | Sign in
(29)

Issue 2697463002: Convert utility process extension Unpacker IPC to mojo (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 month, 1 week ago by noel gordon
Modified:
1 week, 4 days ago
CC:
Aaron Boodman, abarth-chromium, Ben Goodger (Google), 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, viettrungluu+watch_chromium.org, yzshen+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Convert utility process extension Unpacker IPC to mojo Add ExtensionUnpacker mojo, used to (unzip and) unpack a chrome extension, and expose it to the browser via the utility process policy file. Make callers use a utility process mojo client. Add [Native] IPC enum param traits via a mojo typemap to handle extensions::Manifest::Location. Set the wire limit for the enum value to one less than Manifest::NUM_LOCATIONS (for compat with the pre-existing custom traits removed in this patch since this patch auto-generates them: manifest_location_param_traits.cc). In the extensions/utility/utility_handler.cc, retain the CHECKs on the Manifest::Location enum limits while bug 692069 is still being investigated. Move DecodeImages declarations from the extension messages-file into its own file (extension_utility_types.h) to decouple those declarations from this messages-file. A follow-up patch deletes the messages-file so DecodeImages needs a new home. Update all BUILD.gn for using clients to directly depend on the //extensions/common:common target (and hence, its :mojo) to fix post-compile step failures (warning about under-specified BUILD files via "targets is_dirty" errors, see review comment #169). Add TestContentBrowserClient to the extension test harness, and also an overlay file in test/data (needed to expose the mojo in the extension test harness to the unit tests listed below). Covered by existing unit tests: {sandboxed_unpacker, zipfile_installer}_unittest.cc BUG=691410 Review-Url: https://codereview.chromium.org/2697463002 Cr-Commit-Position: refs/heads/master@{#457374} Committed: https://chromium.googlesource.com/chromium/src/+/c8702c4cdecf0859c34a5ae564c89463ac727967

Patch Set 1 #

Patch Set 2 : Support manifest_overlay.json in extensions unittests. #

Patch Set 3 : Add TestContentUtilityClient::ExposeInterfacesToBrowser. #

Patch Set 4 : Add extensions::unpacker::TakeParsedManifest(). #

Total comments: 10

Patch Set 5 : Use MakeUnique when creating the utility mojo client. #

Total comments: 33

Patch Set 6 : Address review comments. #

Patch Set 7 : #include sequenced_worker_pool. #

Patch Set 8 : Add it again (from a different machine). #

Total comments: 6

Patch Set 9 : Address review comments. #

Patch Set 10 : Sync to ToT. #

Total comments: 12

Patch Set 11 : Take #2, Add full mojo enum traits. #

Patch Set 12 : Sync, review comments, remove utility_process_mojo_client.h change. #

Total comments: 5

Patch Set 13 : Comment-only change in response to review comments. #

Total comments: 15

Patch Set 14 : Take #3, use [Native] enum param traits. #

Total comments: 2

Patch Set 15 : Take #3, use [Native] enum param traits, try build fix. #

Patch Set 16 : Take #4, declare the IPC enum traits in the message file, try build fix. #

Total comments: 2

Patch Set 17 : Declare [Native] enum param traits in extensions_messages.h file, delete custom param traits. #

Patch Set 18 : Post compile step fails: targets is_dirty: try make extension unittests depend on :mojo (failed). #

Patch Set 19 : Make mojo build the manifest location enum param traits (works) ... or ... #

Patch Set 20 : Update using BUILD files to depend on //extensions/common:common (works). #

Patch Set 21 : Move DecodedImages into namespace extensions. #

Patch Set 22 : Set the IPC enum traits limit to extensions::Manifest::NUM_LOCATIONS - 1. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+627 lines, -505 lines) Patch
M chrome/browser/chrome_content_utility_manifest_overlay.json View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/extensions/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/extensions/zipfile_installer.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +31 lines, -24 lines 0 comments Download
M chrome/browser/extensions/zipfile_installer.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +74 lines, -87 lines 0 comments Download
M chrome/test/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 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 11 12 13 14 15 16 17 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/utility/extensions/extensions_handler.cc View 1 2 3 4 5 6 7 8 1 chunk +1 line, -2 lines 0 comments Download
M extensions/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +2 lines, -0 lines 0 comments Download
M extensions/DEPS View 1 2 3 4 5 6 7 8 9 10 1 chunk +1 line, -0 lines 0 comments Download
M extensions/browser/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 1 chunk +1 line, -1 line 0 comments Download
M extensions/browser/extensions_test.h View 1 2 chunks +2 lines, -0 lines 0 comments Download
M extensions/browser/extensions_test.cc View 1 2 chunks +4 lines, -1 line 0 comments Download
M extensions/browser/sandboxed_unpacker.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 7 chunks +60 lines, -93 lines 0 comments Download
M extensions/browser/sandboxed_unpacker.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 18 chunks +101 lines, -125 lines 0 comments Download
M extensions/browser/sandboxed_unpacker_unittest.cc View 1 2 3 4 5 6 7 8 2 chunks +2 lines, -1 line 0 comments Download
M extensions/common/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 3 chunks +9 lines, -0 lines 0 comments Download
M extensions/common/OWNERS View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +9 lines, -0 lines 0 comments Download
M extensions/common/extension_messages.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 2 chunks +1 line, -23 lines 0 comments Download
A extensions/common/extension_unpacker.mojom View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +36 lines, -0 lines 0 comments Download
M extensions/common/extension_utility_messages.h View 16 3 chunks +0 lines, -49 lines 0 comments Download
A extensions/common/extension_utility_types.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 1 chunk +25 lines, -0 lines 0 comments Download
A extensions/common/manifest_location.typemap View 1 2 3 4 5 6 7 8 9 10 11 12 13 16 17 18 19 1 chunk +12 lines, -0 lines 0 comments Download
A extensions/common/manifest_location_param_traits.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 1 chunk +16 lines, -0 lines 0 comments Download
A extensions/common/manifest_location_param_traits.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 1 chunk +32 lines, -0 lines 0 comments Download
A extensions/common/typemaps.gni View 1 2 3 4 5 6 7 8 9 10 1 chunk +5 lines, -0 lines 0 comments Download
A extensions/test/data/extensions_test_content_utility_manifest_overlay.json View 1 1 chunk +12 lines, -0 lines 0 comments Download
A extensions/test/test_content_browser_client.h View 1 1 chunk +31 lines, -0 lines 0 comments Download
A extensions/test/test_content_browser_client.cc View 1 2 3 4 5 1 chunk +37 lines, -0 lines 0 comments Download
M extensions/test/test_content_utility_client.h View 1 2 1 chunk +3 lines, -1 line 0 comments Download
M extensions/test/test_content_utility_client.cc View 1 2 1 chunk +7 lines, -5 lines 0 comments Download
M extensions/utility/unpacker.h View 1 2 3 1 chunk +4 lines, -0 lines 0 comments Download
M extensions/utility/unpacker.cc View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -1 line 0 comments Download
M extensions/utility/utility_handler.h View 2 chunks +8 lines, -10 lines 0 comments Download
M extensions/utility/utility_handler.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 16 5 chunks +94 lines, -82 lines 0 comments Download
M mojo/public/tools/bindings/chromium_bindings_configuration.gni View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +1 line, -0 lines 0 comments Download
Trybot results:  linux_chromium_chromeos_ozone_rel_ng   chromium_presubmit   linux_chromium_chromeos_ozone_rel_ng   ios-simulator   linux_chromium_chromeos_ozone_rel_ng   mac_chromium_rel_ng   ios-simulator   ios-device   ios-device-xcode-clang   win_clang   linux_chromium_rel_ng   linux_chromium_tsan_rel_ng   mac_chromium_compile_dbg_ng   win_chromium_compile_dbg_ng   ios-simulator-xcode-clang   win_chromium_rel_ng   win_chromium_x64_rel_ng   android_arm64_dbg_recipe   chromeos_daisy_chromium_compile_only_ng   chromeos_amd64-generic_chromium_compile_only_ng   linux_chromium_asan_rel_ng   cast_shell_linux   android_cronet   android_n5x_swarming_rel   linux_chromium_chromeos_ozone_rel_ng   linux_chromium_compile_dbg_ng   android_clang_dbg_recipe   cast_shell_android   android_compile_dbg   linux_chromium_chromeos_rel_ng   linux_android_rel_ng   chromium_presubmit   chromium_presubmit   chromium_presubmit   linux_chromium_chromeos_rel_ng   linux_chromium_chromeos_rel_ng   chromium_presubmit   linux_chromium_chromeos_rel_ng   chromium_presubmit   linux_chromium_chromeos_rel_ng   linux_chromium_chromeos_rel_ng   chromeos_amd64-generic_chromium_compile_only_ng   win_chromium_compile_dbg_ng   ios-simulator-xcode-clang   linux_chromium_asan_rel_ng   ios-device   linux_chromium_rel_ng   chromium_presubmit   chromeos_daisy_chromium_compile_only_ng   win_chromium_x64_rel_ng   linux_chromium_chromeos_rel_ng   cast_shell_linux   win_chromium_rel_ng   mac_chromium_rel_ng   linux_chromium_tsan_rel_ng   linux_android_rel_ng   win_clang   mac_chromium_compile_dbg_ng   ios-device-xcode-clang   linux_chromium_chromeos_ozone_rel_ng   linux_chromium_compile_dbg_ng   ios-simulator   cast_shell_android   android_arm64_dbg_recipe   android_n5x_swarming_rel   android_cronet   android_clang_dbg_recipe   android_compile_dbg 
Commit queue not available (can’t edit this change).

Messages

Total messages: 217 (162 generated)
noel gordon
PTAL. A DEPS change here to allow utilty_handler to use service_manager (so utility_handler can expose ...
1 month, 1 week ago (2017-02-14 02:53:45 UTC) #18
Sam McNally
https://codereview.chromium.org/2697463002/diff/80001/chrome/browser/extensions/zipfile_installer.cc File chrome/browser/extensions/zipfile_installer.cc (right): https://codereview.chromium.org/2697463002/diff/80001/chrome/browser/extensions/zipfile_installer.cc#newcode60 chrome/browser/extensions/zipfile_installer.cc:60: if (!base::CreateTemporaryDirInDir(dir_temp, dir_name, &temp_dir_)) { This class would be ...
1 month, 1 week ago (2017-02-14 05:41:52 UTC) #25
noel gordon
https://codereview.chromium.org/2697463002/diff/80001/chrome/browser/extensions/zipfile_installer.cc File chrome/browser/extensions/zipfile_installer.cc (right): https://codereview.chromium.org/2697463002/diff/80001/chrome/browser/extensions/zipfile_installer.cc#newcode60 chrome/browser/extensions/zipfile_installer.cc:60: if (!base::CreateTemporaryDirInDir(dir_temp, dir_name, &temp_dir_)) { On 2017/02/14 05:41:52, Sam ...
1 month, 1 week ago (2017-02-14 12:35:37 UTC) #28
Devlin
FYI, +catmullings@ is also looking into converting extension messages to mojo (she has an in-progress ...
1 month, 1 week ago (2017-02-14 17:24:59 UTC) #31
noel gordon
On 2017/02/14 17:24:59, Devlin wrote: > FYI, +catmullings@ is also looking into converting extension messages ...
1 month, 1 week ago (2017-02-15 13:35:13 UTC) #32
noel gordon
https://codereview.chromium.org/2697463002/diff/100001/chrome/browser/extensions/zipfile_installer.cc File chrome/browser/extensions/zipfile_installer.cc (right): https://codereview.chromium.org/2697463002/diff/100001/chrome/browser/extensions/zipfile_installer.cc#newcode61 chrome/browser/extensions/zipfile_installer.cc:61: base::FilePath temp_dir; On 2017/02/14 17:24:59, Devlin wrote: > dir_temp ...
1 month, 1 week ago (2017-02-15 17:28:09 UTC) #34
noel gordon
On 2017/02/15 13:35:13, noel gordon wrote: > On 2017/02/14 17:24:59, Devlin wrote: > > FYI, ...
1 month, 1 week ago (2017-02-16 01:39:53 UTC) #38
Devlin
https://codereview.chromium.org/2697463002/diff/100001/chrome/browser/extensions/zipfile_installer.h File chrome/browser/extensions/zipfile_installer.h (right): https://codereview.chromium.org/2697463002/diff/100001/chrome/browser/extensions/zipfile_installer.h#newcode16 chrome/browser/extensions/zipfile_installer.h:16: #include "extensions/common/extension_unpacker.mojom.h" On 2017/02/15 17:28:08, noel gordon wrote: > ...
1 month, 1 week ago (2017-02-17 15:53:56 UTC) #59
noel gordon
https://codereview.chromium.org/2697463002/diff/100001/chrome/browser/extensions/zipfile_installer.h File chrome/browser/extensions/zipfile_installer.h (right): https://codereview.chromium.org/2697463002/diff/100001/chrome/browser/extensions/zipfile_installer.h#newcode16 chrome/browser/extensions/zipfile_installer.h:16: #include "extensions/common/extension_unpacker.mojom.h" On 2017/02/17 15:53:56, Devlin wrote: > On ...
4 weeks ago (2017-02-27 11:47:51 UTC) #78
Devlin
https://codereview.chromium.org/2697463002/diff/100001/extensions/common/extension_unpacker.mojom File extensions/common/extension_unpacker.mojom (right): https://codereview.chromium.org/2697463002/diff/100001/extensions/common/extension_unpacker.mojom#newcode25 extensions/common/extension_unpacker.mojom:25: int32 location, On 2017/02/27 11:47:50, noel gordon wrote: > ...
3 weeks, 6 days ago (2017-02-27 19:30:15 UTC) #79
Devlin
(other than that, this lg)
3 weeks, 6 days ago (2017-02-27 23:45:17 UTC) #80
dcheng
https://codereview.chromium.org/2697463002/diff/240001/chrome/browser/extensions/zipfile_installer.cc File chrome/browser/extensions/zipfile_installer.cc (right): https://codereview.chromium.org/2697463002/diff/240001/chrome/browser/extensions/zipfile_installer.cc#newcode77 chrome/browser/extensions/zipfile_installer.cc:77: DCHECK_CURRENTLY_ON(content::BrowserThread::UI); Does this need to be on the UI ...
3 weeks, 6 days ago (2017-02-28 06:12:49 UTC) #81
jochen
(I'm waiting for the other reviewers to finish their reviews first)
3 weeks ago (2017-03-06 12:48:11 UTC) #86
noel gordon
https://codereview.chromium.org/2697463002/diff/240001/chrome/browser/extensions/zipfile_installer.cc File chrome/browser/extensions/zipfile_installer.cc (right): https://codereview.chromium.org/2697463002/diff/240001/chrome/browser/extensions/zipfile_installer.cc#newcode77 chrome/browser/extensions/zipfile_installer.cc:77: DCHECK_CURRENTLY_ON(content::BrowserThread::UI); On 2017/02/28 06:12:49, dcheng wrote: > Does this ...
3 weeks ago (2017-03-06 13:12:04 UTC) #89
dcheng
https://codereview.chromium.org/2697463002/diff/240001/extensions/browser/sandboxed_unpacker.cc File extensions/browser/sandboxed_unpacker.cc (right): https://codereview.chromium.org/2697463002/diff/240001/extensions/browser/sandboxed_unpacker.cc#newcode672 extensions/browser/sandboxed_unpacker.cc:672: std::unique_ptr<base::DictionaryValue> original_manifest, On 2017/03/06 13:12:03, noel gordon wrote: > ...
2 weeks, 6 days ago (2017-03-07 09:45:14 UTC) #92
noel gordon
https://codereview.chromium.org/2697463002/diff/240001/extensions/browser/sandboxed_unpacker.cc File extensions/browser/sandboxed_unpacker.cc (right): https://codereview.chromium.org/2697463002/diff/240001/extensions/browser/sandboxed_unpacker.cc#newcode672 extensions/browser/sandboxed_unpacker.cc:672: std::unique_ptr<base::DictionaryValue> original_manifest, On 2017/03/07 09:45:13, dcheng wrote: > On ...
2 weeks, 5 days ago (2017-03-08 13:44:26 UTC) #94
Devlin
lgtm % nits https://codereview.chromium.org/2697463002/diff/280001/extensions/utility/utility_handler.cc File extensions/utility/utility_handler.cc (right): https://codereview.chromium.org/2697463002/diff/280001/extensions/utility/utility_handler.cc#newcode59 extensions/utility/utility_handler.cc:59: CHECK_NE(location, Manifest::INVALID_LOCATION); On 2017/03/08 13:44:26, noel ...
2 weeks, 5 days ago (2017-03-08 16:22:24 UTC) #95
noel gordon
https://codereview.chromium.org/2697463002/diff/280001/extensions/utility/utility_handler.cc File extensions/utility/utility_handler.cc (right): https://codereview.chromium.org/2697463002/diff/280001/extensions/utility/utility_handler.cc#newcode59 extensions/utility/utility_handler.cc:59: CHECK_NE(location, Manifest::INVALID_LOCATION); On 2017/03/08 16:22:24, Devlin wrote: > On ...
2 weeks, 4 days ago (2017-03-08 17:56:10 UTC) #96
Devlin
https://codereview.chromium.org/2697463002/diff/300001/extensions/common/manifest_location.mojom File extensions/common/manifest_location.mojom (right): https://codereview.chromium.org/2697463002/diff/300001/extensions/common/manifest_location.mojom#newcode11 extensions/common/manifest_location.mojom:11: enum ManifestLocation { On 2017/03/08 17:56:09, noel gordon wrote: ...
2 weeks, 4 days ago (2017-03-08 18:05:17 UTC) #97
jochen
lgtm
2 weeks, 4 days ago (2017-03-08 18:53:01 UTC) #98
dcheng
lgtm https://codereview.chromium.org/2697463002/diff/280001/extensions/utility/utility_handler.cc File extensions/utility/utility_handler.cc (right): https://codereview.chromium.org/2697463002/diff/280001/extensions/utility/utility_handler.cc#newcode59 extensions/utility/utility_handler.cc:59: CHECK_NE(location, Manifest::INVALID_LOCATION); On 2017/03/08 17:56:09, noel gordon wrote: ...
2 weeks, 4 days ago (2017-03-09 01:03:32 UTC) #99
Sam McNally
lgtm https://codereview.chromium.org/2697463002/diff/300001/extensions/browser/sandboxed_unpacker.cc File extensions/browser/sandboxed_unpacker.cc (right): https://codereview.chromium.org/2697463002/diff/300001/extensions/browser/sandboxed_unpacker.cc#newcode671 extensions/browser/sandboxed_unpacker.cc:671: const std::unique_ptr<base::DictionaryValue>& original_manifest, This is weird. Either take ...
2 weeks, 4 days ago (2017-03-09 08:50:18 UTC) #100
noel gordon
PTAL https://codereview.chromium.org/2697463002/diff/300001/extensions/browser/sandboxed_unpacker.cc File extensions/browser/sandboxed_unpacker.cc (right): https://codereview.chromium.org/2697463002/diff/300001/extensions/browser/sandboxed_unpacker.cc#newcode671 extensions/browser/sandboxed_unpacker.cc:671: const std::unique_ptr<base::DictionaryValue>& original_manifest, On 2017/03/09 08:50:18, Sam McNally ...
2 weeks, 4 days ago (2017-03-09 15:40:11 UTC) #102
noel gordon
https://codereview.chromium.org/2697463002/diff/300001/extensions/common/extension_utility_types.h File extensions/common/extension_utility_types.h (right): https://codereview.chromium.org/2697463002/diff/300001/extensions/common/extension_utility_types.h#newcode13 extensions/common/extension_utility_types.h:13: #include "ui/gfx/ipc/skia/gfx_skia_param_traits.h" On 2017/03/08 17:56:09, noel gordon wrote: > ...
2 weeks, 4 days ago (2017-03-09 15:48:44 UTC) #107
noel gordon
Some build funk on the bots, not sure what's up, compiles fine locally for me ...
2 weeks, 4 days ago (2017-03-09 15:52:02 UTC) #108
Devlin
My mojo knowledge is limited, so sammc can comment better than I, but the manifest_location_param_traits.cc ...
2 weeks, 4 days ago (2017-03-09 15:52:12 UTC) #109
noel gordon
On 2017/03/09 15:52:12, Devlin wrote: > My mojo knowledge is limited, so sammc can comment ...
2 weeks, 3 days ago (2017-03-09 16:55:35 UTC) #114
noel gordon
Build worked everywhere except win, dunno way. Plan B: put the ipc enum in the ...
2 weeks, 3 days ago (2017-03-09 19:12:23 UTC) #124
noel gordon
^^^ Should work, but does not in windows. /me puts on x-ray goggles ◕‿◕ A) ...
2 weeks, 3 days ago (2017-03-10 02:38:28 UTC) #127
dcheng
On 2017/03/10 02:38:28, noel gordon wrote: > ^^^ Should work, but does not in windows. ...
2 weeks, 3 days ago (2017-03-10 02:46:43 UTC) #128
dcheng
On 2017/03/10 02:46:43, dcheng wrote: > On 2017/03/10 02:38:28, noel gordon wrote: > > ^^^ ...
2 weeks, 3 days ago (2017-03-10 02:47:14 UTC) #129
dcheng
https://codereview.chromium.org/2697463002/diff/420001/extensions/common/extension_utility_types.h File extensions/common/extension_utility_types.h (right): https://codereview.chromium.org/2697463002/diff/420001/extensions/common/extension_utility_types.h#newcode15 extensions/common/extension_utility_types.h:15: using DecodedImages = std::vector<std::tuple<SkBitmap, base::FilePath>>; Nit: this should be ...
2 weeks, 3 days ago (2017-03-10 02:48:47 UTC) #130
dcheng
https://codereview.chromium.org/2697463002/diff/420001/extensions/common/extension_utility_messages.h File extensions/common/extension_utility_messages.h (right): https://codereview.chromium.org/2697463002/diff/420001/extensions/common/extension_utility_messages.h#newcode19 extensions/common/extension_utility_messages.h:19: extensions::Manifest::NUM_LOCATIONS) Also, this should be NUM_LOCATIONS - 1, since ...
2 weeks, 3 days ago (2017-03-10 02:51:05 UTC) #131
tibell
lgtm
2 weeks, 3 days ago (2017-03-10 05:16:51 UTC) #132
noel gordon
On 2017/03/10 02:47:14, dcheng wrote: > On 2017/03/10 02:46:43, dcheng wrote: > > On 2017/03/10 ...
2 weeks ago (2017-03-13 12:50:12 UTC) #166
noel gordon
On 2017/03/10 02:48:47, dcheng wrote: > https://codereview.chromium.org/2697463002/diff/420001/extensions/common/extension_utility_types.h > File extensions/common/extension_utility_types.h (right): > > https://codereview.chromium.org/2697463002/diff/420001/extensions/common/extension_utility_types.h#newcode15 > ...
2 weeks ago (2017-03-13 12:51:55 UTC) #167
noel gordon
On 2017/03/10 02:51:05, dcheng wrote: > https://codereview.chromium.org/2697463002/diff/420001/extensions/common/extension_utility_messages.h > File extensions/common/extension_utility_messages.h (right): > > https://codereview.chromium.org/2697463002/diff/420001/extensions/common/extension_utility_messages.h#newcode19 > ...
2 weeks ago (2017-03-13 12:55:38 UTC) #168
noel gordon
The pre-exiting ParamTraits<extensions::Manifest::Location> defined in extension_messages.cc caused the multiple definitions error that the windows compilers ...
2 weeks ago (2017-03-13 13:14:21 UTC) #169
noel gordon
So why all these is_dirty targets? Turns out that the BUILD.on files that use the ...
2 weeks ago (2017-03-13 13:24:36 UTC) #170
noel gordon
On 2017/03/13 12:55:38, noel gordon wrote: > On 2017/03/10 02:51:05, dcheng wrote: > > > ...
2 weeks ago (2017-03-13 13:32:10 UTC) #171
noel gordon
On 2017/03/13 13:32:10, noel gordon wrote: > So Manifest::NUM_LOCATIONS was considered invalid, so I think ...
2 weeks ago (2017-03-13 13:40:23 UTC) #174
noel gordon
On 2017/03/13 13:40:23, noel gordon wrote: > On 2017/03/13 13:32:10, noel gordon wrote: > > ...
2 weeks ago (2017-03-13 13:50:26 UTC) #175
noel gordon
On 2017/03/09 01:03:32, dcheng wrote: > lgtm > > https://codereview.chromium.org/2697463002/diff/280001/extensions/utility/utility_handler.cc > File extensions/utility/utility_handler.cc (right): > ...
1 week, 6 days ago (2017-03-14 03:13:47 UTC) #187
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/2697463002/580001
1 week, 6 days ago (2017-03-14 03:22:10 UTC) #194
commit-bot: I haz the power
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_presubmit/builds/384613)
1 week, 6 days ago (2017-03-14 03:36:28 UTC) #196
noel gordon
+ben for approval to add the typemap file and DEPS.
1 week, 6 days ago (2017-03-14 03:53:49 UTC) #198
noel gordon
-ben, +rockot
1 week, 5 days ago (2017-03-15 04:04:53 UTC) #200
Ken Rockot
LGTM with uber-nitty terminology nit, at least in the CL description and some code comment(s): ...
1 week, 4 days ago (2017-03-15 17:46:34 UTC) #203
noel gordon
Removed "service" from the CL description. Agree the term is overloaded. Indeed, searching the CL ...
1 week, 4 days ago (2017-03-16 03:42:27 UTC) #205
Ken Rockot
Yep, I want to get rid of the "utility process mojo client" - it's not ...
1 week, 4 days ago (2017-03-16 03:43:56 UTC) #206
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/2697463002/580001
1 week, 4 days ago (2017-03-16 03:44:38 UTC) #208
commit-bot: I haz the power
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_chromeos_ozone_rel_ng/builds/341290)
1 week, 4 days ago (2017-03-16 05:13:16 UTC) #210
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/2697463002/580001
1 week, 4 days ago (2017-03-16 07:54:06 UTC) #212
commit-bot: I haz the power
Committed patchset #22 (id:580001) as https://chromium.googlesource.com/chromium/src/+/c8702c4cdecf0859c34a5ae564c89463ac727967
1 week, 4 days ago (2017-03-16 08:52:11 UTC) #215
noel gordon
1 week, 4 days ago (2017-03-16 09:11:49 UTC) #217
On 2017/03/16 03:43:56, Ken Rockot wrote:
> Yep, I want to get rid of the "utility process mojo client" - it's not
> really an abstraction we want hanging around. Clients shouldn't even think
> of the term "utility process", they should just be connecting to services.
> Soon...

Nod.  Can discuss in upcoming s13n meeting.
Sign in to reply to this message.

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