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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 5429002: Support for checking multiple keyfiles when removing an installation package.... (Closed)

Created:
10 years ago by tommi (sloooow) - chröme
Modified:
9 years, 7 months ago
Reviewers:
robertshield
CC:
chromium-reviews, Paweł Hajdan Jr.
Visibility:
Public.

Description

Support for checking multiple keyfiles when removing an installation package. BUG=61609 TEST=No change right now (multi-install related). Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=67968

Patch Set 1 #

Total comments: 12

Patch Set 2 : '' #

Patch Set 3 : '' #

Patch Set 4 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+163 lines, -63 lines) Patch
M chrome/installer/setup/install.cc View 1 2 3 3 chunks +2 lines, -4 lines 0 comments Download
M chrome/installer/setup/setup_main.cc View 1 2 3 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/installer/util/browser_distribution.h View 1 2 3 2 chunks +13 lines, -1 line 0 comments Download
M chrome/installer/util/browser_distribution.cc View 1 2 3 2 chunks +5 lines, -1 line 0 comments Download
M chrome/installer/util/chrome_frame_distribution.h View 1 2 3 2 chunks +2 lines, -1 line 0 comments Download
M chrome/installer/util/chrome_frame_distribution.cc View 1 2 3 2 chunks +6 lines, -2 lines 0 comments Download
M chrome/installer/util/delete_tree_work_item.h View 1 2 3 2 chunks +14 lines, -10 lines 0 comments Download
M chrome/installer/util/delete_tree_work_item.cc View 1 2 3 4 chunks +81 lines, -19 lines 0 comments Download
M chrome/installer/util/delete_tree_work_item_unittest.cc View 1 2 3 3 chunks +6 lines, -6 lines 0 comments Download
M chrome/installer/util/package.cc View 1 2 3 1 chunk +11 lines, -9 lines 0 comments Download
M chrome/installer/util/work_item.h View 1 2 3 2 chunks +3 lines, -1 line 0 comments Download
M chrome/installer/util/work_item.cc View 1 2 3 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/installer/util/work_item_list.h View 1 2 3 2 chunks +6 lines, -2 lines 0 comments Download
M chrome/installer/util/work_item_list.cc View 1 2 3 2 chunks +10 lines, -3 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
tommi (sloooow) - chröme
10 years ago (2010-12-01 21:18:53 UTC) #1
robertshield
LGTM, just nits. http://codereview.chromium.org/5429002/diff/1/chrome/installer/setup/install.cc File chrome/installer/setup/install.cc (left): http://codereview.chromium.org/5429002/diff/1/chrome/installer/setup/install.cc#oldcode627 chrome/installer/setup/install.cc:627: // TODO(tommi): See if we can't ...
10 years ago (2010-12-01 21:40:54 UTC) #2
tommi (sloooow) - chröme
10 years ago (2010-12-01 22:07:15 UTC) #3 
http://codereview.chromium.org/5429002/diff/1/chrome/installer/setup/install.cc
File chrome/installer/setup/install.cc (left):

http://codereview.chromium.org/5429002/diff/1/chrome/installer/setup/install....
chrome/installer/setup/install.cc:627: // TODO(tommi): See if we can't get rid
of this parameter.
On 2010/12/01 21:40:54, robertshield wrote:
> Gave up? :p

yeah, well, this todo was added when I thought I could easily get rid of it :)

http://codereview.chromium.org/5429002/diff/1/chrome/installer/util/browser_d...
File chrome/installer/util/browser_distribution.h (right):

http://codereview.chromium.org/5429002/diff/1/chrome/installer/util/browser_d...
chrome/installer/util/browser_distribution.h:16: #include <vector>
On 2010/12/01 21:40:54, robertshield wrote:
> why include vector?

Removed.  I started out supporting a vector of key files but then simplified.

http://codereview.chromium.org/5429002/diff/1/chrome/installer/util/browser_d...
chrome/installer/util/browser_distribution.h:116: // only a name that's relative
to the installation folder.
On 2010/12/01 21:40:54, robertshield wrote:
> xnay on the stallationinnay olderfay.

Done.

http://codereview.chromium.org/5429002/diff/1/chrome/installer/util/delete_tr...
File chrome/installer/util/delete_tree_work_item.cc (right):

http://codereview.chromium.org/5429002/diff/1/chrome/installer/util/delete_tr...
chrome/installer/util/delete_tree_work_item.cc:14:
std::vector<FilePath>::const_iterator it = key_paths.begin();
On 2010/12/01 21:40:54, robertshield wrote:
> nitty nit: KeyFileList::const_iterator it(key_paths_.begin());
> here and below

Done.

http://codereview.chromium.org/5429002/diff/1/chrome/installer/util/delete_tr...
chrome/installer/util/delete_tree_work_item.cc:43: // with only the
FILE_FLAG_SHARE_DELETE flag.  Once we know we have all
On 2010/12/01 21:40:54, robertshield wrote:
> *FILE_SHARE_DELETE

Done.

http://codereview.chromium.org/5429002/diff/1/chrome/installer/util/delete_tr...
File chrome/installer/util/delete_tree_work_item.h (right):

http://codereview.chromium.org/5429002/diff/1/chrome/installer/util/delete_tr...
chrome/installer/util/delete_tree_work_item.h:49: // started working.  If key
files are specified, deletion will be performed
On 2010/12/01 21:40:54, robertshield wrote:
> "the WorkItem has started working" -> "Do() has been called"?

Done.

Powered by Google App Engine
This is Rietveld 408576698