Issue 2273113002: Delete old files after an update.

Issue 2273113002: Delete old files after an update. (Closed)

Created:
4 years, 4 months ago by fdoray

Modified:
4 years, 3 months ago

Reviewers:
grt (UTC plus 2)

CC:
chromium-reviews, grt+watch_chromium.org, pennymac+watch_chromium.org, wfh+watch_chromium.org

Base URL:
https://chromium.googlesource.com/chromium/src.git@harvester

Target Ref:
refs/pending/heads/master

Project:
chromium

Visibility:
Public.

More Reviews

Description

Delete old files after an update. This is a variation of r378802 which was reverted because of a Dr Memory error (solved by using GenerateAlternatePEFileVersion instead of GenerateSpecificPEFileVersion). r378802 also caused problems with shortcuts crbug.com/592040 (solved by not letting the deletion of old files cause a rollback). With this CL, the installer deletes all files that belong to old versions of Chrome that are not in use after an update. Eventually, the DeleteOldVersions() will be invoked again to delete old versions that were in use at time of the update. BUG=451546 TEST= -- Not-in-use -- 1. Run mini_installer.exe --chrome --multi-install --verbose-logging --do-not-launch-chrome - The install directory should contain chrome.exe and a matching version directory. - There should be no other executable or version directory in the install directory. 2. Run next_version_mini_installer.exe --multi-install --verbose-logging --do-not-launch-chrome - The install directory should contain chrome.exe and a matching version directory. The version should be higher than at the previous step. - There should be no other executable or version directory in the install directory. -- In-use -- 1. Run mini_installer.exe --chrome --multi-install --verbose-logging --do-not-launch-chrome - The install directory should contain chrome.exe and a matching version directory. - There should be no other executable or version directory in the install directory. 2. Launch Chrome. 3. Run next_version_mini_installer.exe --multi-install --verbose-logging --do-not-launch-chrome. - The install directory should contain chrome.exe and a matching version directory + new_chrome.exe and a matching version directory. - There should be no other executable or version directory in the install directory. Committed: https://crrev.com/32c0f60f45c1a309c7a22544969ccb2e09027ea6 Cr-Commit-Position: refs/heads/master@{#414842}

Patch Set 1 #

Total comments: 20

Patch Set 2 : CR grt #4 #

Created: 4 years, 3 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+570 lines, -465 lines)			Patch
M	chrome/installer/setup/install.cc	View	1	2 chunks	+4 lines, -5 lines	0 comments	Download
M	chrome/installer/util/BUILD.gn	View		2 chunks	+3 lines, -0 lines	0 comments	Download
A	chrome/installer/util/delete_old_versions.h	View	1	1 chunk	+23 lines, -0 lines	0 comments	Download
A	chrome/installer/util/delete_old_versions.cc	View	1	1 chunk	+250 lines, -0 lines	0 comments	Download
A	chrome/installer/util/delete_old_versions_unittest.cc	View	1	1 chunk	+287 lines, -0 lines	0 comments	Download
M	chrome/installer/util/installer_state.h	View		3 chunks	+0 lines, -13 lines	0 comments	Download
M	chrome/installer/util/installer_state.cc	View		2 chunks	+0 lines, -66 lines	0 comments	Download
M	chrome/installer/util/installer_state_unittest.cc	View		5 chunks	+3 lines, -381 lines	0 comments	Download

Dependent Patchsets:

Issue 2283833002 Patch 1

Messages

Total messages: 13 (6 generated)

Expand Messages | Collapse Messages | Show Generated Messages | Hide Generated Messages

fdoray

Description was changed from ========== Delete old files after an update. With this CL, the ...

4 years, 4 months ago (2016-08-24 20:38:01 UTC) #1

grt (UTC plus 2)

looks pretty good. please mention in the CL description that this is a variation of ...

4 years, 3 months ago (2016-08-25 11:24:47 UTC) #4

fdoray

Description was changed from ========== Delete old files after an update. With this CL, the ...

4 years, 3 months ago (2016-08-26 15:13:59 UTC) #5

Description was changed from

==========
Delete old files after an update.

With this CL, the installer deletes all files that belong to old
versions of Chrome that are not in use after an update.

Eventually, the DeleteOldVersions() will be invoked again to delete
old versions that were in use at time of the update.

BUG=451546
TEST=
-- Not-in-use --
1. Run mini_installer.exe --chrome --multi-install --verbose-logging
--do-not-launch-chrome
- The install directory should contain chrome.exe and a matching version
directory.
- There should be no other executable or version directory in the install
directory.
2. Run next_version_mini_installer.exe --multi-install --verbose-logging
--do-not-launch-chrome
- The install directory should contain chrome.exe and a matching version
directory. The version should be higher than at the previous step.
- There should be no other executable or version directory in the install
directory.

-- In-use --
1. Run mini_installer.exe --chrome --multi-install --verbose-logging
--do-not-launch-chrome
- The install directory should contain chrome.exe and a matching version
directory.
- There should be no other executable or version directory in the install
directory.
2. Launch Chrome.
3. Run next_version_mini_installer.exe --multi-install --verbose-logging
--do-not-launch-chrome.
- The install directory should contain chrome.exe and a matching version
directory + new_chrome.exe and a matching version directory.
- There should be no other executable or version directory in the install
directory.
==========

to

==========
Delete old files after an update.

This is a variation of r378802 which was reverted because of
a Dr Memory error (solved by using GenerateAlternatePEFileVersion
instead of GenerateSpecificPEFileVersion). r378802 also caused
problems with shortcuts crbug.com/592040 (solved by not letting
the deletion of old files cause a rollback).

With this CL, the installer deletes all files that belong to old
versions of Chrome that are not in use after an update.

Eventually, the DeleteOldVersions() will be invoked again to delete
old versions that were in use at time of the update.

BUG=451546
TEST=
-- Not-in-use --
1. Run mini_installer.exe --chrome --multi-install --verbose-logging
--do-not-launch-chrome
- The install directory should contain chrome.exe and a matching version
directory.
- There should be no other executable or version directory in the install
directory.
2. Run next_version_mini_installer.exe --multi-install --verbose-logging
--do-not-launch-chrome
- The install directory should contain chrome.exe and a matching version
directory. The version should be higher than at the previous step.
- There should be no other executable or version directory in the install
directory.

-- In-use --
1. Run mini_installer.exe --chrome --multi-install --verbose-logging
--do-not-launch-chrome
- The install directory should contain chrome.exe and a matching version
directory.
- There should be no other executable or version directory in the install
directory.
2. Launch Chrome.
3. Run next_version_mini_installer.exe --multi-install --verbose-logging
--do-not-launch-chrome.
- The install directory should contain chrome.exe and a matching version
directory + new_chrome.exe and a matching version directory.
- There should be no other executable or version directory in the install
directory.
==========

fdoray

PTAnL https://codereview.chromium.org/2273113002/diff/1/chrome/installer/util/delete_old_versions.cc File chrome/installer/util/delete_old_versions.cc (right): https://codereview.chromium.org/2273113002/diff/1/chrome/installer/util/delete_old_versions.cc#newcode12 chrome/installer/util/delete_old_versions.cc:12: #include "base/bind.h" On 2016/08/25 11:24:47, grt (UTC plus ...

4 years, 3 months ago (2016-08-26 15:16:15 UTC) #6

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/2273113002/20001

4 years, 3 months ago (2016-08-26 20:12:09 UTC) #9

commit-bot: I haz the power

Description was changed from ========== Delete old files after an update. This is a variation ...

4 years, 3 months ago (2016-08-26 22:44:45 UTC) #10

Message was sent while issue was closed.

Description was changed from

==========
Delete old files after an update.

This is a variation of r378802 which was reverted because of
a Dr Memory error (solved by using GenerateAlternatePEFileVersion
instead of GenerateSpecificPEFileVersion). r378802 also caused
problems with shortcuts crbug.com/592040 (solved by not letting
the deletion of old files cause a rollback).

With this CL, the installer deletes all files that belong to old
versions of Chrome that are not in use after an update.

Eventually, the DeleteOldVersions() will be invoked again to delete
old versions that were in use at time of the update.

BUG=451546
TEST=
-- Not-in-use --
1. Run mini_installer.exe --chrome --multi-install --verbose-logging
--do-not-launch-chrome
- The install directory should contain chrome.exe and a matching version
directory.
- There should be no other executable or version directory in the install
directory.
2. Run next_version_mini_installer.exe --multi-install --verbose-logging
--do-not-launch-chrome
- The install directory should contain chrome.exe and a matching version
directory. The version should be higher than at the previous step.
- There should be no other executable or version directory in the install
directory.

-- In-use --
1. Run mini_installer.exe --chrome --multi-install --verbose-logging
--do-not-launch-chrome
- The install directory should contain chrome.exe and a matching version
directory.
- There should be no other executable or version directory in the install
directory.
2. Launch Chrome.
3. Run next_version_mini_installer.exe --multi-install --verbose-logging
--do-not-launch-chrome.
- The install directory should contain chrome.exe and a matching version
directory + new_chrome.exe and a matching version directory.
- There should be no other executable or version directory in the install
directory.
==========

to

==========
Delete old files after an update.

This is a variation of r378802 which was reverted because of
a Dr Memory error (solved by using GenerateAlternatePEFileVersion
instead of GenerateSpecificPEFileVersion). r378802 also caused
problems with shortcuts crbug.com/592040 (solved by not letting
the deletion of old files cause a rollback).

With this CL, the installer deletes all files that belong to old
versions of Chrome that are not in use after an update.

Eventually, the DeleteOldVersions() will be invoked again to delete
old versions that were in use at time of the update.

BUG=451546
TEST=
-- Not-in-use --
1. Run mini_installer.exe --chrome --multi-install --verbose-logging
--do-not-launch-chrome
- The install directory should contain chrome.exe and a matching version
directory.
- There should be no other executable or version directory in the install
directory.
2. Run next_version_mini_installer.exe --multi-install --verbose-logging
--do-not-launch-chrome
- The install directory should contain chrome.exe and a matching version
directory. The version should be higher than at the previous step.
- There should be no other executable or version directory in the install
directory.

-- In-use --
1. Run mini_installer.exe --chrome --multi-install --verbose-logging
--do-not-launch-chrome
- The install directory should contain chrome.exe and a matching version
directory.
- There should be no other executable or version directory in the install
directory.
2. Launch Chrome.
3. Run next_version_mini_installer.exe --multi-install --verbose-logging
--do-not-launch-chrome.
- The install directory should contain chrome.exe and a matching version
directory + new_chrome.exe and a matching version directory.
- There should be no other executable or version directory in the install
directory.
==========

commit-bot: I haz the power

Description was changed from ========== Delete old files after an update. This is a variation ...

4 years, 3 months ago (2016-08-26 22:48:14 UTC) #12

Message was sent while issue was closed.

Description was changed from

==========
Delete old files after an update.

This is a variation of r378802 which was reverted because of
a Dr Memory error (solved by using GenerateAlternatePEFileVersion
instead of GenerateSpecificPEFileVersion). r378802 also caused
problems with shortcuts crbug.com/592040 (solved by not letting
the deletion of old files cause a rollback).

With this CL, the installer deletes all files that belong to old
versions of Chrome that are not in use after an update.

Eventually, the DeleteOldVersions() will be invoked again to delete
old versions that were in use at time of the update.

BUG=451546
TEST=
-- Not-in-use --
1. Run mini_installer.exe --chrome --multi-install --verbose-logging
--do-not-launch-chrome
- The install directory should contain chrome.exe and a matching version
directory.
- There should be no other executable or version directory in the install
directory.
2. Run next_version_mini_installer.exe --multi-install --verbose-logging
--do-not-launch-chrome
- The install directory should contain chrome.exe and a matching version
directory. The version should be higher than at the previous step.
- There should be no other executable or version directory in the install
directory.

-- In-use --
1. Run mini_installer.exe --chrome --multi-install --verbose-logging
--do-not-launch-chrome
- The install directory should contain chrome.exe and a matching version
directory.
- There should be no other executable or version directory in the install
directory.
2. Launch Chrome.
3. Run next_version_mini_installer.exe --multi-install --verbose-logging
--do-not-launch-chrome.
- The install directory should contain chrome.exe and a matching version
directory + new_chrome.exe and a matching version directory.
- There should be no other executable or version directory in the install
directory.
==========

to

==========
Delete old files after an update.

This is a variation of r378802 which was reverted because of
a Dr Memory error (solved by using GenerateAlternatePEFileVersion
instead of GenerateSpecificPEFileVersion). r378802 also caused
problems with shortcuts crbug.com/592040 (solved by not letting
the deletion of old files cause a rollback).

With this CL, the installer deletes all files that belong to old
versions of Chrome that are not in use after an update.

Eventually, the DeleteOldVersions() will be invoked again to delete
old versions that were in use at time of the update.

BUG=451546
TEST=
-- Not-in-use --
1. Run mini_installer.exe --chrome --multi-install --verbose-logging
--do-not-launch-chrome
- The install directory should contain chrome.exe and a matching version
directory.
- There should be no other executable or version directory in the install
directory.
2. Run next_version_mini_installer.exe --multi-install --verbose-logging
--do-not-launch-chrome
- The install directory should contain chrome.exe and a matching version
directory. The version should be higher than at the previous step.
- There should be no other executable or version directory in the install
directory.

-- In-use --
1. Run mini_installer.exe --chrome --multi-install --verbose-logging
--do-not-launch-chrome
- The install directory should contain chrome.exe and a matching version
directory.
- There should be no other executable or version directory in the install
directory.
2. Launch Chrome.
3. Run next_version_mini_installer.exe --multi-install --verbose-logging
--do-not-launch-chrome.
- The install directory should contain chrome.exe and a matching version
directory + new_chrome.exe and a matching version directory.
- There should be no other executable or version directory in the install
directory.

Committed: https://crrev.com/32c0f60f45c1a309c7a22544969ccb2e09027ea6
Cr-Commit-Position: refs/heads/master@{#414842}
==========

commit-bot: I haz the power

4 years, 3 months ago (2016-08-26 22:48:15 UTC) #13

Message was sent while issue was closed.

Patchset 2 (id:??) landed as
https://crrev.com/32c0f60f45c1a309c7a22544969ccb2e09027ea6
Cr-Commit-Position: refs/heads/master@{#414842}

Expand Messages | Collapse Messages | Show Generated Messages | Hide Generated Messages