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

Issue 2805027: AU: Changes for deltas on traditional bios machines. (Closed)

Created:
10 years, 6 months ago by adlr
Modified:
10 years, 6 months ago
Reviewers:
Daniel Erat
CC:
chromium-os-reviews_chromium.org
Base URL:
ssh://git@chromiumos-git/update_engine.git
Visibility:
Public.

Description

AU: Changes for deltas on traditional bios machines. BUG=None TEST=Attached unittests/tested on image - Fix uninitialized variable err in action processor unittest - Let Omaha dictate if an update is a delta or full update - Bug fix in delta generator for differently-sized images - More logging when applying delta updates - Fix infinite loop in http fetcher unittest - log each HTTP connection to know when a dropped connection is reestablished. - Detect when speed goes below a threshold and reestablish HTTP connection (currently < 10bytes/sec for 90 contiguous seconds). - Fix stack overflow in libcurl http fetcher. - optimize out a lot of needless CPU usage in libcurl http fetcher (turns out adding a glib main loop source uses a lot of CPU). - subprocess: pass PATH, log stdout/stderr - postinstall runner: support for ext3 and ext4 target filesystems.

Patch Set 1 #

Total comments: 4

Patch Set 2 : fixes for review and merging in petkov's recent changes #

Unified diffs Side-by-side diffs Delta from patch set Stats (+162 lines, -69 lines) Patch
M action_processor_unittest.cc View 1 chunk +5 lines, -1 line 0 comments Download
M delta_diff_generator.cc View 2 chunks +2 lines, -1 line 0 comments Download
M delta_performer.cc View 1 chunk +2 lines, -0 lines 0 comments Download
M delta_performer_unittest.cc View 1 1 chunk +2 lines, -2 lines 0 comments Download
M http_fetcher_unittest.cc View 4 chunks +32 lines, -2 lines 0 comments Download
M libcurl_http_fetcher.h View 1 2 chunks +3 lines, -2 lines 0 comments Download
M libcurl_http_fetcher.cc View 1 8 chunks +47 lines, -38 lines 0 comments Download
M omaha_request_action.h View 1 chunk +1 line, -0 lines 0 comments Download
M omaha_request_action.cc View 1 chunk +2 lines, -0 lines 0 comments Download
M omaha_request_action_unittest.cc View 3 chunks +3 lines, -0 lines 0 comments Download
M omaha_response_handler_action.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M omaha_response_handler_action_unittest.cc View 1 3 chunks +4 lines, -1 line 0 comments Download
M postinstall_runner_action.cc View 2 chunks +10 lines, -7 lines 0 comments Download
M subprocess.cc View 5 chunks +48 lines, -14 lines 0 comments Download

Messages

Total messages: 2 (0 generated)
adlr
It's a lot of little changes. Thanks,
10 years, 6 months ago (2010-06-23 03:55:57 UTC) #1
Daniel Erat
10 years, 6 months ago (2010-06-23 19:52:23 UTC) #2
LGTM

http://codereview.chromium.org/2805027/diff/1/7
File libcurl_http_fetcher.cc (right):

http://codereview.chromium.org/2805027/diff/1/7#newcode204
libcurl_http_fetcher.cc:204: // g_source_destroy(timeout_source_);
delete this?

http://codereview.chromium.org/2805027/diff/1/7#newcode233
libcurl_http_fetcher.cc:233: bool LibcurlHttpFetcher::TimeoutCallback() {
nit: this and other glib callbacks should probably have type gboolean and return
TRUE/FALSE instead of using C++ types

http://codereview.chromium.org/2805027/diff/1/12
File subprocess.cc (right):

http://codereview.chromium.org/2805027/diff/1/12#newcode39
subprocess.cc:39: // Note: Caller responsible for free()ing the returned value!
clarify: "for free()ing the returned value and all of the strings that it points
to"

http://codereview.chromium.org/2805027/diff/1/12#newcode126
subprocess.cc:126: char* child_stdout;
mind initializing these to NULL here?

Powered by Google App Engine
This is Rietveld 408576698