Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(174)

Issue 15908002: Differential updates for components. (Closed)

Created:
7 years, 7 months ago by Sorin Jianu
Modified:
7 years, 6 months ago
CC:
chromium-reviews, Aaron Boodman, cbentzel+watch_chromium.org, grt+watch_chromium.org, chromium-apps-reviews_chromium.org, waffles, xhwang, paulgazz
Visibility:
Public.

Description

Differential updates for components. We are adding support for delivering delta updates for Chrome components. Initial platform support for the patcher is Windows only. The update response includes both the full update and, if available, the differential update. The differential update is tried first, then the full update, if needed. BUG=245318 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=207805 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=207917

Patch Set 1 #

Patch Set 2 : #

Patch Set 3 : #

Patch Set 4 : #

Patch Set 5 : #

Total comments: 34

Patch Set 6 : #

Patch Set 7 : #

Patch Set 8 : Remove lzma fixes. #

Patch Set 9 : really removing the lzma files this time. #

Total comments: 78

Patch Set 10 : Addressed most of the feedback so far. #

Total comments: 2

Patch Set 11 : rest of the feedback up to this point. #

Patch Set 12 : lint fixes. #

Patch Set 13 : Final changes. #

Patch Set 14 : Added missing const decl. #

Total comments: 34

Patch Set 15 : #

Patch Set 16 : #

Total comments: 9

Patch Set 17 : #

Total comments: 18

Patch Set 18 : Addressed feedback from grt@ #

Total comments: 4

Patch Set 19 : #

Total comments: 7

Patch Set 20 : Changes to logging in setup_util and getting rid of pings. #

Patch Set 21 : Removed ping command line switches and the urls. #

Patch Set 22 : #

Patch Set 23 : all done. #

Patch Set 24 : Enable diffs flag. #

Total comments: 10

Patch Set 25 : #

Total comments: 4

Patch Set 26 : done, done, done. #

Patch Set 27 : Run patcher unit tests on Windows only. #

Patch Set 28 : Sync to LKGR revision 207804. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+2334 lines, -377 lines) Patch
A chrome/browser/component_updater/component_patcher.h View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +86 lines, -0 lines 0 comments Download
A chrome/browser/component_updater/component_patcher.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +81 lines, -0 lines 0 comments Download
A chrome/browser/component_updater/component_patcher_operation.h View 1 2 3 4 5 6 7 8 9 10 1 chunk +158 lines, -0 lines 0 comments Download
A chrome/browser/component_updater/component_patcher_operation.cc View 1 2 3 4 5 6 7 8 9 10 1 chunk +222 lines, -0 lines 0 comments Download
A chrome/browser/component_updater/component_patcher_win.h View 1 2 3 4 5 1 chunk +24 lines, -0 lines 0 comments Download
A chrome/browser/component_updater/component_patcher_win.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +98 lines, -0 lines 0 comments Download
M chrome/browser/component_updater/component_unpacker.h View 1 2 3 4 5 6 7 8 9 3 chunks +18 lines, -2 lines 0 comments Download
M chrome/browser/component_updater/component_unpacker.cc View 1 2 3 4 5 6 7 8 9 7 chunks +70 lines, -18 lines 0 comments Download
M chrome/browser/component_updater/component_updater_configurator.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 4 chunks +43 lines, -12 lines 0 comments Download
M chrome/browser/component_updater/component_updater_service.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 4 chunks +19 lines, -4 lines 0 comments Download
M chrome/browser/component_updater/component_updater_service.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 23 chunks +166 lines, -45 lines 0 comments Download
M chrome/browser/component_updater/pepper_flash_component_installer.cc View 1 2 3 4 5 2 chunks +8 lines, -0 lines 0 comments Download
M chrome/browser/component_updater/pnacl/pnacl_component_installer.h View 1 2 3 4 5 2 chunks +6 lines, -0 lines 0 comments Download
M chrome/browser/component_updater/pnacl/pnacl_component_installer.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 1 chunk +5 lines, -0 lines 0 comments Download
M chrome/browser/component_updater/pnacl/pnacl_profile_observer.h View 1 2 3 4 5 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/component_updater/recovery_component_installer.cc View 1 2 3 4 5 2 chunks +8 lines, -0 lines 0 comments Download
M chrome/browser/component_updater/swiftshader_component_installer.cc View 1 2 3 4 5 2 chunks +8 lines, -0 lines 0 comments Download
M chrome/browser/component_updater/test/DEPS View 1 2 3 4 5 1 chunk +1 line, -0 lines 0 comments Download
A chrome/browser/component_updater/test/component_patcher_mock.h View 1 2 3 4 5 6 7 8 9 1 chunk +28 lines, -0 lines 0 comments Download
A chrome/browser/component_updater/test/component_patcher_unittest.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 1 chunk +42 lines, -0 lines 0 comments Download
A chrome/browser/component_updater/test/component_patcher_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 1 chunk +118 lines, -0 lines 0 comments Download
A chrome/browser/component_updater/test/component_patcher_unittest_win.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 1 chunk +83 lines, -0 lines 0 comments Download
A chrome/browser/component_updater/test/component_updater_service_unittest.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 1 chunk +134 lines, -0 lines 0 comments Download
M chrome/browser/component_updater/test/component_updater_service_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 12 chunks +322 lines, -238 lines 0 comments Download
A chrome/browser/component_updater/test/component_updater_service_unittest_win.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 1 chunk +101 lines, -0 lines 0 comments Download
A chrome/browser/component_updater/test/test_installer.h View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +73 lines, -0 lines 0 comments Download
A chrome/browser/component_updater/test/test_installer.cc View 1 2 3 4 5 6 7 8 9 1 chunk +81 lines, -0 lines 0 comments Download
M chrome/browser/component_updater/widevine_cdm_component_installer.cc View 1 2 3 4 5 2 chunks +8 lines, -0 lines 0 comments Download
M chrome/browser/net/crl_set_fetcher.h View 1 2 3 4 5 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/browser/net/crl_set_fetcher.cc View 1 2 3 4 5 1 chunk +5 lines, -0 lines 0 comments Download
M chrome/chrome_browser.gypi View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 1 chunk +6 lines, -0 lines 0 comments Download
M chrome/chrome_tests_unit.gypi View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 2 chunks +6 lines, -0 lines 0 comments Download
M chrome/common/chrome_switches.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 1 chunk +1 line, -1 line 0 comments Download
M chrome/common/chrome_switches.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 1 chunk +1 line, -1 line 0 comments Download
M chrome/common/extensions/update_manifest.h View 1 2 3 4 5 6 7 8 9 10 3 chunks +23 lines, -8 lines 0 comments Download
M chrome/common/extensions/update_manifest.cc View 1 2 3 4 5 3 chunks +21 lines, -2 lines 0 comments Download
M chrome/common/omaha_query_params/omaha_query_params.h View 1 2 3 4 5 6 7 8 9 1 chunk +4 lines, -0 lines 0 comments Download
M chrome/common/omaha_query_params/omaha_query_params.cc View 1 2 3 4 5 3 chunks +19 lines, -16 lines 0 comments Download
M chrome/installer/setup/setup_main.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 4 chunks +22 lines, -2 lines 0 comments Download
M chrome/installer/setup/setup_util.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 2 chunks +23 lines, -4 lines 0 comments Download
M chrome/installer/setup/setup_util.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 4 chunks +64 lines, -6 lines 0 comments Download
M chrome/installer/util/util_constants.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 4 chunks +19 lines, -1 line 0 comments Download
M chrome/installer/util/util_constants.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 2 chunks +12 lines, -0 lines 0 comments Download
A chrome/test/data/components/updatecheck_diff_reply_1.xml View 1 2 3 4 5 1 chunk +6 lines, -0 lines 0 comments Download
A chrome/test/data/components/updatecheck_diff_reply_2.xml View 1 2 3 4 5 1 chunk +6 lines, -0 lines 0 comments Download
A + chrome/test/data/components/updatecheck_diff_reply_3.xml View 1 2 3 4 5 1 chunk +2 lines, -2 lines 0 comments Download
A + chrome/test/data/components/updatecheck_reply_noupdate.xml View 1 2 3 4 5 1 chunk +2 lines, -2 lines 0 comments Download
M courgette/disassembler_elf_32.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 3 chunks +7 lines, -7 lines 0 comments Download
M courgette/disassembler_elf_32_x86.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 1 chunk +2 lines, -3 lines 0 comments Download
M courgette/third_party/bsdiff.h View 1 2 3 4 5 3 chunks +9 lines, -1 line 0 comments Download
M courgette/third_party/bsdiff_apply.cc View 1 2 3 4 5 3 chunks +41 lines, -0 lines 0 comments Download
M extensions/common/crx_file.h View 1 2 3 4 5 1 chunk +4 lines, -0 lines 0 comments Download
M extensions/common/crx_file.cc View 1 2 3 4 5 3 chunks +15 lines, -2 lines 0 comments Download

Messages

Total messages: 73 (0 generated)
cpu_(ooo_6.6-7.5)
First off I haven't seen anything horrible yet so we are off to a good ...
7 years, 6 months ago (2013-05-29 20:06:44 UTC) #1
cpu_(ooo_6.6-7.5)
https://codereview.chromium.org/15908002/diff/64007/chrome/browser/component_updater/component_updater_service.h File chrome/browser/component_updater/component_updater_service.h (right): https://codereview.chromium.org/15908002/diff/64007/chrome/browser/component_updater/component_updater_service.h#newcode21 chrome/browser/component_updater/component_updater_service.h:21: class FilePath; note filepath is fwd declared here. https://codereview.chromium.org/15908002/diff/64007/chrome/browser/component_updater/component_updater_service.h#newcode74 ...
7 years, 6 months ago (2013-05-29 20:13:58 UTC) #2
waffles
https://codereview.chromium.org/15908002/diff/64007/chrome/browser/chrome_browser_main.cc File chrome/browser/chrome_browser_main.cc (right): https://codereview.chromium.org/15908002/diff/64007/chrome/browser/chrome_browser_main.cc#newcode403 chrome/browser/chrome_browser_main.cc:403: // file IO to know your existing component version. ...
7 years, 6 months ago (2013-06-13 20:55:04 UTC) #3
Sorin Jianu
erikwright@: Erik, please take a look at chrome\browser\net\ changes for clr_set_fetcher.* asargent@: Anthony, please review ...
7 years, 6 months ago (2013-06-14 22:26:55 UTC) #4
cpu_(ooo_6.6-7.5)
https://codereview.chromium.org/15908002/diff/101001/extensions/common/crx_file.cc File extensions/common/crx_file.cc (right): https://codereview.chromium.org/15908002/diff/101001/extensions/common/crx_file.cc#newcode27 extensions/common/crx_file.cc:27: const char kCrxDiffFileHeaderMagic[] = "CrOD"; in what ways is ...
7 years, 6 months ago (2013-06-14 23:05:49 UTC) #5
cpu_(ooo_6.6-7.5)
can we change each component (nacl, cdm, swiftshader) in a different CL? we can review ...
7 years, 6 months ago (2013-06-14 23:08:35 UTC) #6
cpu_(ooo_6.6-7.5)
https://codereview.chromium.org/15908002/diff/101001/chrome/browser/component_updater/component_patcher.cc File chrome/browser/component_updater/component_patcher.cc (right): https://codereview.chromium.org/15908002/diff/101001/chrome/browser/component_updater/component_patcher.cc#newcode79 chrome/browser/component_updater/component_patcher.cc:79: return ComponentUnpacker::kDeltaOperationFailure; return kDeltaOperationUnsuported ? https://codereview.chromium.org/15908002/diff/101001/chrome/browser/component_updater/component_patcher_win.cc File chrome/browser/component_updater/component_patcher_win.cc (right): ...
7 years, 6 months ago (2013-06-14 23:25:28 UTC) #7
cpu_(ooo_6.6-7.5)
ignore my comment of splitting components into CLs for all the components that just return ...
7 years, 6 months ago (2013-06-14 23:28:18 UTC) #8
waffles
https://codereview.chromium.org/15908002/diff/101001/chrome/browser/component_updater/component_patcher.cc File chrome/browser/component_updater/component_patcher.cc (right): https://codereview.chromium.org/15908002/diff/101001/chrome/browser/component_updater/component_patcher.cc#newcode79 chrome/browser/component_updater/component_patcher.cc:79: return ComponentUnpacker::kDeltaOperationFailure; On 2013/06/14 23:25:28, cpu wrote: > return ...
7 years, 6 months ago (2013-06-15 00:01:37 UTC) #9
Sorin Jianu
We will also hook up the installers for diff aware components (pNacl) in a different ...
7 years, 6 months ago (2013-06-15 00:10:44 UTC) #10
sra1
lgtm lgtm for courgette/* https://codereview.chromium.org/15908002/diff/101001/chrome/browser/component_updater/component_patcher.h File chrome/browser/component_updater/component_patcher.h (right): https://codereview.chromium.org/15908002/diff/101001/chrome/browser/component_updater/component_patcher.h#newcode22 chrome/browser/component_updater/component_patcher.h:22: kPatchTypeBsdiff, One thing to consider: ...
7 years, 6 months ago (2013-06-15 00:56:16 UTC) #11
cpu_(ooo_6.6-7.5)
>> CRXs should do something sane when you click-and-drag the file intro ah makes sense ...
7 years, 6 months ago (2013-06-15 02:40:36 UTC) #12
robertshield
chrome/installer comments, doing a full pass now https://codereview.chromium.org/15908002/diff/101001/chrome/installer/setup/setup_util.cc File chrome/installer/setup/setup_util.cc (right): https://codereview.chromium.org/15908002/diff/101001/chrome/installer/setup/setup_util.cc#newcode38 chrome/installer/setup/setup_util.cc:38: const int ...
7 years, 6 months ago (2013-06-17 16:39:27 UTC) #13
erikwright (departed)
agl@, can you review chrome/browser/net/crl_set_fetcher.{cc,h}?
7 years, 6 months ago (2013-06-17 16:59:58 UTC) #14
agl
LGTM for CRLSet changes.
7 years, 6 months ago (2013-06-17 17:01:33 UTC) #15
erikwright (departed)
https://codereview.chromium.org/15908002/diff/101001/chrome/browser/component_updater/component_patcher.h File chrome/browser/component_updater/component_patcher.h (right): https://codereview.chromium.org/15908002/diff/101001/chrome/browser/component_updater/component_patcher.h#newcode19 chrome/browser/component_updater/component_patcher.h:19: enum PatchType { This enum in the global namespace ...
7 years, 6 months ago (2013-06-17 17:24:28 UTC) #16
robertshield
Some more comments. https://codereview.chromium.org/15908002/diff/101001/chrome/browser/component_updater/component_patcher.cc File chrome/browser/component_updater/component_patcher.cc (right): https://codereview.chromium.org/15908002/diff/101001/chrome/browser/component_updater/component_patcher.cc#newcode58 chrome/browser/component_updater/component_patcher.cc:58: std::string error; error isn't used, pass ...
7 years, 6 months ago (2013-06-17 17:46:59 UTC) #17
cpu_(ooo_6.6-7.5)
https://codereview.chromium.org/15908002/diff/101001/chrome/browser/component_updater/component_patcher.cc File chrome/browser/component_updater/component_patcher.cc (right): https://codereview.chromium.org/15908002/diff/101001/chrome/browser/component_updater/component_patcher.cc#newcode91 chrome/browser/component_updater/component_patcher.cc:91: int* error) { After trying to like the int* ...
7 years, 6 months ago (2013-06-17 20:27:00 UTC) #18
waffles
https://codereview.chromium.org/15908002/diff/101001/chrome/browser/component_updater/test/component_patcher_mock.h File chrome/browser/component_updater/test/component_patcher_mock.h (right): https://codereview.chromium.org/15908002/diff/101001/chrome/browser/component_updater/test/component_patcher_mock.h#newcode24 chrome/browser/component_updater/test/component_patcher_mock.h:24: DISALLOW_COPY_AND_ASSIGN(MockComponentPatcher); On 2013/06/17 17:24:28, erikwright wrote: > IWYU: include ...
7 years, 6 months ago (2013-06-17 22:17:48 UTC) #19
Sorin Jianu
https://codereview.chromium.org/15908002/diff/101001/chrome/browser/component_updater/component_patcher.cc File chrome/browser/component_updater/component_patcher.cc (right): https://codereview.chromium.org/15908002/diff/101001/chrome/browser/component_updater/component_patcher.cc#newcode58 chrome/browser/component_updater/component_patcher.cc:58: std::string error; On 2013/06/17 17:47:00, robertshield wrote: > error ...
7 years, 6 months ago (2013-06-17 22:21:09 UTC) #20
waffles
Changes will be in patchset 11. https://codereview.chromium.org/15908002/diff/101001/chrome/browser/component_updater/component_patcher.h File chrome/browser/component_updater/component_patcher.h (right): https://codereview.chromium.org/15908002/diff/101001/chrome/browser/component_updater/component_patcher.h#newcode22 chrome/browser/component_updater/component_patcher.h:22: kPatchTypeBsdiff, On 2013/06/15 ...
7 years, 6 months ago (2013-06-17 23:51:07 UTC) #21
asargent_no_longer_on_chrome
lgtm nit: The CL description would be easier to read if line wrapped at 70-75 ...
7 years, 6 months ago (2013-06-18 00:08:00 UTC) #22
waffles
https://codereview.chromium.org/15908002/diff/132001/chrome/common/extensions/update_manifest.cc File chrome/common/extensions/update_manifest.cc (right): https://codereview.chromium.org/15908002/diff/132001/chrome/common/extensions/update_manifest.cc#newcode201 chrome/common/extensions/update_manifest.cc:201: result->package_fingerprint = GetAttribute(updatecheck, "fp"); On 2013/06/18 00:08:01, Antony Sargent ...
7 years, 6 months ago (2013-06-18 00:36:25 UTC) #23
Sorin Jianu
We addressed all the feedback so far, PTAL. Thank you! https://codereview.chromium.org/15908002/diff/101001/chrome/browser/component_updater/component_patcher.cc File chrome/browser/component_updater/component_patcher.cc (right): https://codereview.chromium.org/15908002/diff/101001/chrome/browser/component_updater/component_patcher.cc#newcode84 ...
7 years, 6 months ago (2013-06-18 03:05:13 UTC) #24
robertshield
lgtm https://codereview.chromium.org/15908002/diff/101001/chrome/browser/component_updater/component_patcher.cc File chrome/browser/component_updater/component_patcher.cc (right): https://codereview.chromium.org/15908002/diff/101001/chrome/browser/component_updater/component_patcher.cc#newcode64 chrome/browser/component_updater/component_patcher.cc:64: return static_cast<base::ListValue*>(root.release()); On 2013/06/17 22:21:09, sorinj wrote: > ...
7 years, 6 months ago (2013-06-18 13:27:07 UTC) #25
erikwright (departed)
LGTM.
7 years, 6 months ago (2013-06-18 14:02:46 UTC) #26
Sorin Jianu
Thank you Robert! All done! https://codereview.chromium.org/15908002/diff/101001/chrome/browser/component_updater/component_patcher.cc File chrome/browser/component_updater/component_patcher.cc (right): https://codereview.chromium.org/15908002/diff/101001/chrome/browser/component_updater/component_patcher.cc#newcode64 chrome/browser/component_updater/component_patcher.cc:64: return static_cast<base::ListValue*>(root.release()); On 2013/06/18 ...
7 years, 6 months ago (2013-06-18 17:00:11 UTC) #27
cpu_(ooo_6.6-7.5)
sorry I couldn't get to this yesterday, here is enough to get you going. https://codereview.chromium.org/15908002/diff/194002/chrome/browser/component_updater/component_updater_service.cc ...
7 years, 6 months ago (2013-06-18 17:02:05 UTC) #28
Sorin Jianu
Thank you Carlos! PTAL. https://codereview.chromium.org/15908002/diff/194002/chrome/browser/component_updater/component_updater_service.cc File chrome/browser/component_updater/component_updater_service.cc (left): https://codereview.chromium.org/15908002/diff/194002/chrome/browser/component_updater/component_updater_service.cc#oldcode205 chrome/browser/component_updater/component_updater_service.cc:205: CrxComponent component; On 2013/06/18 17:02:06, ...
7 years, 6 months ago (2013-06-18 20:46:38 UTC) #29
waffles
https://codereview.chromium.org/15908002/diff/194002/chrome/browser/component_updater/component_updater_service.cc File chrome/browser/component_updater/component_updater_service.cc (left): https://codereview.chromium.org/15908002/diff/194002/chrome/browser/component_updater/component_updater_service.cc#oldcode453 chrome/browser/component_updater/component_updater_service.cc:453: // |to| statatus and returns how many have been ...
7 years, 6 months ago (2013-06-18 20:50:40 UTC) #30
cpu_(ooo_6.6-7.5)
https://codereview.chromium.org/15908002/diff/194002/chrome/browser/component_updater/component_updater_service.cc File chrome/browser/component_updater/component_updater_service.cc (right): https://codereview.chromium.org/15908002/diff/194002/chrome/browser/component_updater/component_updater_service.cc#newcode985 chrome/browser/component_updater/component_updater_service.cc:985: crx->status = CrxUpdateItem::kNoUpdate; On 2013/06/18 20:46:39, sorinj wrote: > ...
7 years, 6 months ago (2013-06-18 22:02:35 UTC) #31
cpu_(ooo_6.6-7.5)
https://codereview.chromium.org/15908002/diff/194002/chrome/browser/component_updater/component_updater_service.h File chrome/browser/component_updater/component_updater_service.h (right): https://codereview.chromium.org/15908002/diff/194002/chrome/browser/component_updater/component_updater_service.h#newcode55 chrome/browser/component_updater/component_updater_service.h:55: // Only |name| is optional. |pk_hash| is the SHA256 ...
7 years, 6 months ago (2013-06-18 22:07:33 UTC) #32
cpu_(ooo_6.6-7.5)
https://codereview.chromium.org/15908002/diff/194002/chrome/browser/component_updater/component_updater_service.cc File chrome/browser/component_updater/component_updater_service.cc (right): https://codereview.chromium.org/15908002/diff/194002/chrome/browser/component_updater/component_updater_service.cc#newcode986 chrome/browser/component_updater/component_updater_service.cc:986: config_->OnEvent(Configurator::kNetworkError, CrxIdtoUMAId(context->id)); On 2013/06/18 20:46:39, sorinj wrote: > On ...
7 years, 6 months ago (2013-06-18 22:18:24 UTC) #33
cpu_(ooo_6.6-7.5)
https://codereview.chromium.org/15908002/diff/214003/chrome/browser/component_updater/component_updater_service.cc File chrome/browser/component_updater/component_updater_service.cc (right): https://codereview.chromium.org/15908002/diff/214003/chrome/browser/component_updater/component_updater_service.cc#newcode964 chrome/browser/component_updater/component_updater_service.cc:964: base::Unretained(this))); this postask is probably wrong. The design assumes ...
7 years, 6 months ago (2013-06-18 22:32:36 UTC) #34
cpu_(ooo_6.6-7.5)
https://codereview.chromium.org/15908002/diff/214003/chrome/browser/component_updater/component_updater_configurator.cc File chrome/browser/component_updater/component_updater_configurator.cc (right): https://codereview.chromium.org/15908002/diff/214003/chrome/browser/component_updater/component_updater_configurator.cc#newcode97 chrome/browser/component_updater/component_updater_configurator.cc:97: pings_enabled_(false), do we want pings disabled by default? Is ...
7 years, 6 months ago (2013-06-18 22:45:52 UTC) #35
waffles
https://codereview.chromium.org/15908002/diff/214003/chrome/browser/component_updater/component_updater_configurator.cc File chrome/browser/component_updater/component_updater_configurator.cc (right): https://codereview.chromium.org/15908002/diff/214003/chrome/browser/component_updater/component_updater_configurator.cc#newcode97 chrome/browser/component_updater/component_updater_configurator.cc:97: pings_enabled_(false), On 2013/06/18 22:45:52, cpu wrote: > do we ...
7 years, 6 months ago (2013-06-19 00:02:10 UTC) #36
Sorin Jianu
What is still pending is the ping decoupling work. The other items were done or ...
7 years, 6 months ago (2013-06-19 00:29:10 UTC) #37
grt (UTC plus 2)
drive-by chrome/installer comments https://codereview.chromium.org/15908002/diff/226001/chrome/installer/setup/setup_util.cc File chrome/installer/setup/setup_util.cc (right): https://codereview.chromium.org/15908002/diff/226001/chrome/installer/setup/setup_util.cc#newcode96 chrome/installer/setup/setup_util.cc:96: if (src.empty() || patch.empty() || dest.empty()) ...
7 years, 6 months ago (2013-06-19 14:45:36 UTC) #38
Sorin Jianu
Thank you Greg. All done. https://codereview.chromium.org/15908002/diff/226001/chrome/installer/setup/setup_util.cc File chrome/installer/setup/setup_util.cc (right): https://codereview.chromium.org/15908002/diff/226001/chrome/installer/setup/setup_util.cc#newcode96 chrome/installer/setup/setup_util.cc:96: if (src.empty() || patch.empty() ...
7 years, 6 months ago (2013-06-19 17:14:44 UTC) #39
grt (UTC plus 2)
https://codereview.chromium.org/15908002/diff/226001/chrome/installer/setup/setup_util.cc File chrome/installer/setup/setup_util.cc (right): https://codereview.chromium.org/15908002/diff/226001/chrome/installer/setup/setup_util.cc#newcode172 chrome/installer/setup/setup_util.cc:172: VLOG(1) << "Failed to apply patch " << patch.value() ...
7 years, 6 months ago (2013-06-19 20:17:38 UTC) #40
Sorin Jianu
Thank you Greg. Apparently, I can't comment on my own code. Fixed everything. Please take ...
7 years, 6 months ago (2013-06-19 21:05:53 UTC) #41
cpu_(ooo_6.6-7.5)
On 2013/06/19 00:02:10, waffles wrote: > https://codereview.chromium.org/15908002/diff/214003/chrome/browser/component_updater/component_updater_configurator.cc > File chrome/browser/component_updater/component_updater_configurator.cc (right): > > https://codereview.chromium.org/15908002/diff/214003/chrome/browser/component_updater/component_updater_configurator.cc#newcode97 > ...
7 years, 6 months ago (2013-06-20 00:13:26 UTC) #42
cpu_(ooo_6.6-7.5)
The CL is looking very good, I think we are in the final stretch. Do ...
7 years, 6 months ago (2013-06-20 00:19:35 UTC) #43
cpu_(ooo_6.6-7.5)
https://codereview.chromium.org/15908002/diff/244001/chrome/browser/component_updater/component_patcher_win.cc File chrome/browser/component_updater/component_patcher_win.cc (right): https://codereview.chromium.org/15908002/diff/244001/chrome/browser/component_updater/component_patcher_win.cc#newcode89 chrome/browser/component_updater/component_patcher_win.cc:89: int exit_code = 0; I think I made a ...
7 years, 6 months ago (2013-06-20 00:27:47 UTC) #44
waffles
https://codereview.chromium.org/15908002/diff/214003/chrome/browser/component_updater/component_updater_configurator.cc File chrome/browser/component_updater/component_updater_configurator.cc (right): https://codereview.chromium.org/15908002/diff/214003/chrome/browser/component_updater/component_updater_configurator.cc#newcode107 chrome/browser/component_updater/component_updater_configurator.cc:107: pings_enabled_ = cmdline->HasSwitch(switches::kEnableComponentUpdatePings); On 2013/06/19 00:02:11, waffles wrote: > ...
7 years, 6 months ago (2013-06-20 04:53:32 UTC) #45
grt (UTC plus 2)
lgtm w/ final tweaks below. thanks. https://codereview.chromium.org/15908002/diff/244001/chrome/installer/setup/setup_util.cc File chrome/installer/setup/setup_util.cc (right): https://codereview.chromium.org/15908002/diff/244001/chrome/installer/setup/setup_util.cc#newcode93 chrome/installer/setup/setup_util.cc:93: LOG(WARNING) << "Applying ...
7 years, 6 months ago (2013-06-20 14:04:14 UTC) #46
grt (UTC plus 2)
lgtm
7 years, 6 months ago (2013-06-20 14:44:33 UTC) #47
Sorin Jianu
https://codereview.chromium.org/15908002/diff/244001/chrome/installer/setup/setup_util.cc File chrome/installer/setup/setup_util.cc (right): https://codereview.chromium.org/15908002/diff/244001/chrome/installer/setup/setup_util.cc#newcode93 chrome/installer/setup/setup_util.cc:93: LOG(WARNING) << "Applying Courgette patch " << patch.value() << ...
7 years, 6 months ago (2013-06-20 17:40:14 UTC) #48
Sorin Jianu
On 2013/06/20 17:40:14, sorinj wrote: > https://codereview.chromium.org/15908002/diff/244001/chrome/installer/setup/setup_util.cc > File chrome/installer/setup/setup_util.cc (right): > > https://codereview.chromium.org/15908002/diff/244001/chrome/installer/setup/setup_util.cc#newcode93 > ...
7 years, 6 months ago (2013-06-20 19:39:22 UTC) #49
sra1
Courgette files still lgtm
7 years, 6 months ago (2013-06-20 20:11:37 UTC) #50
cpu_(ooo_6.6-7.5)
https://codereview.chromium.org/15908002/diff/276001/chrome/browser/component_updater/component_patcher_win.cc File chrome/browser/component_updater/component_patcher_win.cc (right): https://codereview.chromium.org/15908002/diff/276001/chrome/browser/component_updater/component_patcher_win.cc#newcode81 chrome/browser/component_updater/component_patcher_win.cc:81: so no job object? https://codereview.chromium.org/15908002/diff/276001/chrome/browser/component_updater/component_updater_service.cc File chrome/browser/component_updater/component_updater_service.cc (right): https://codereview.chromium.org/15908002/diff/276001/chrome/browser/component_updater/component_updater_service.cc#newcode266 ...
7 years, 6 months ago (2013-06-20 20:40:33 UTC) #51
Sorin Jianu
Fixed. PTAL. Thank you! https://codereview.chromium.org/15908002/diff/276001/chrome/browser/component_updater/component_patcher_win.cc File chrome/browser/component_updater/component_patcher_win.cc (right): https://codereview.chromium.org/15908002/diff/276001/chrome/browser/component_updater/component_patcher_win.cc#newcode81 chrome/browser/component_updater/component_patcher_win.cc:81: On 2013/06/20 20:40:34, cpu wrote: ...
7 years, 6 months ago (2013-06-20 21:34:54 UTC) #52
cpu_(ooo_6.6-7.5)
https://codereview.chromium.org/15908002/diff/285001/chrome/browser/component_updater/component_updater_service.cc File chrome/browser/component_updater/component_updater_service.cc (right): https://codereview.chromium.org/15908002/diff/285001/chrome/browser/component_updater/component_updater_service.cc#newcode235 chrome/browser/component_updater/component_updater_service.cc:235: bool is_update_available; how is different from status = CrxUpdateItem::kCanUpdate ...
7 years, 6 months ago (2013-06-20 22:30:53 UTC) #53
cpu_(ooo_6.6-7.5)
https://codereview.chromium.org/15908002/diff/276001/chrome/browser/component_updater/component_patcher_win.cc File chrome/browser/component_updater/component_patcher_win.cc (right): https://codereview.chromium.org/15908002/diff/276001/chrome/browser/component_updater/component_patcher_win.cc#newcode81 chrome/browser/component_updater/component_patcher_win.cc:81: On 2013/06/20 21:34:55, sorinj wrote: > On 2013/06/20 20:40:34, ...
7 years, 6 months ago (2013-06-20 22:32:11 UTC) #54
cpu_(ooo_6.6-7.5)
https://codereview.chromium.org/15908002/diff/285001/chrome/browser/component_updater/test/url_request_post_interceptor.cc File chrome/browser/component_updater/test/url_request_post_interceptor.cc (right): https://codereview.chromium.org/15908002/diff/285001/chrome/browser/component_updater/test/url_request_post_interceptor.cc#newcode52 chrome/browser/component_updater/test/url_request_post_interceptor.cc:52: pingMockJob and related code, I think is not needed ...
7 years, 6 months ago (2013-06-20 22:34:24 UTC) #55
cpu_(ooo_6.6-7.5)
That is all I have! please address the comment above. lgtm I started a set ...
7 years, 6 months ago (2013-06-20 22:39:22 UTC) #56
waffles
lgtm Not sure when I got added as a reviewer; but it LGTM.
7 years, 6 months ago (2013-06-20 22:55:28 UTC) #57
Sorin Jianu
Thank you! All changes post LGTM uploaded. https://codereview.chromium.org/15908002/diff/276001/chrome/browser/component_updater/component_patcher_win.cc File chrome/browser/component_updater/component_patcher_win.cc (right): https://codereview.chromium.org/15908002/diff/276001/chrome/browser/component_updater/component_patcher_win.cc#newcode81 chrome/browser/component_updater/component_patcher_win.cc:81: On 2013/06/20 ...
7 years, 6 months ago (2013-06-20 23:51:58 UTC) #58
cpu_(ooo_6.6-7.5)
set CQ bit, I looked at trybots and I am not sure they are running ...
7 years, 6 months ago (2013-06-21 00:06:34 UTC) #59
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sorin@chromium.org/15908002/288001
7 years, 6 months ago (2013-06-21 00:06:53 UTC) #60
commit-bot: I haz the power
Retried try job too often on chromium_presubmit for step(s) presubmit http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_presubmit&number=11102
7 years, 6 months ago (2013-06-21 00:20:30 UTC) #61
sra1
lgtm
7 years, 6 months ago (2013-06-21 00:33:36 UTC) #62
sra%chromium.org
lgtm
7 years, 6 months ago (2013-06-21 00:49:22 UTC) #63
dgarrett
The new wrapper in Courgette looks handy. LGTM
7 years, 6 months ago (2013-06-21 00:53:29 UTC) #64
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sorin@chromium.org/15908002/288001
7 years, 6 months ago (2013-06-21 00:54:46 UTC) #65
dgarrett
7 years, 6 months ago (2013-06-21 00:54:48 UTC) #66
tommi (sloooow) - chröme
Rubberstamp lgtm
7 years, 6 months ago (2013-06-21 02:32:47 UTC) #67
commit-bot: I haz the power
Change committed as 207805
7 years, 6 months ago (2013-06-21 14:20:17 UTC) #68
Sorin Jianu
Carlos, PTAL. The patcher unit tests only run on Windows, to avoid a crash that ...
7 years, 6 months ago (2013-06-21 17:11:45 UTC) #69
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sorin@chromium.org/15908002/272004
7 years, 6 months ago (2013-06-21 17:25:03 UTC) #70
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sorin@chromium.org/15908002/152003
7 years, 6 months ago (2013-06-21 17:44:33 UTC) #71
cpu_(ooo_6.6-7.5)
lgtm
7 years, 6 months ago (2013-06-21 17:46:44 UTC) #72
commit-bot: I haz the power
7 years, 6 months ago (2013-06-21 20:41:39 UTC) #73
Message was sent while issue was closed.
Change committed as 207917

Powered by Google App Engine
This is Rietveld 408576698