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

Issue 2163803003: [chrome_elf] Removing blacklist finch for dynamic dll changes. (Closed)

Created:
4 years, 5 months ago by penny
Modified:
4 years, 4 months ago
Reviewers:
robertshield
CC:
chromium-reviews, grt+watch_chromium.org, caitkp+watch_chromium.org, proberge
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[chrome_elf] Removing blacklist finch for dynamic dll changes. Part of chrome_elf cleanup. This was never used in the wild, and we won't need it in the foreseeable future. Leaving beacon and emergency disable finch switch for safety. Tests: 1) chrome_elf_unittests, chrome_elf_util_unittest.cc: BlacklistTest* 2) unit_tests, chrome_elf_init_unittest_win.cc: ChromeBlacklistTrialTest* R=robertshield@chromium.org CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng BUG=631771 Committed: https://crrev.com/c4da848392dabb7b49e554a15ad47d1c9f0dac78 Cr-Commit-Position: refs/heads/master@{#408269}

Patch Set 1 #

Patch Set 2 : Missed reference in uninstaller. #

Patch Set 3 : clang fix #

Patch Set 4 : Another clang fix. #

Total comments: 2

Patch Set 5 : Review fixes. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+14 lines, -231 lines) Patch
M chrome/browser/safe_browsing/incident_reporting/environment_data_collection_win.cc View 2 chunks +0 lines, -13 lines 0 comments Download
M chrome/browser/safe_browsing/incident_reporting/environment_data_collection_win_unittest.cc View 1 2 3 2 chunks +0 lines, -49 lines 0 comments Download
M chrome/browser/win/chrome_elf_init.h View 1 chunk +0 lines, -3 lines 0 comments Download
M chrome/browser/win/chrome_elf_init.cc View 2 chunks +0 lines, -37 lines 0 comments Download
M chrome/browser/win/chrome_elf_init_unittest.cc View 1 2 2 chunks +0 lines, -43 lines 0 comments Download
M chrome/installer/setup/uninstall.cc View 1 2 3 4 1 chunk +10 lines, -1 line 0 comments Download
M chrome_elf/blacklist/blacklist.h View 1 chunk +0 lines, -4 lines 0 comments Download
M chrome_elf/blacklist/blacklist.cc View 1 chunk +0 lines, -18 lines 0 comments Download
M chrome_elf/blacklist/test/blacklist_test.cc View 5 chunks +4 lines, -47 lines 0 comments Download
M chrome_elf/blacklist/test/blacklist_test_main_dll.def View 1 chunk +0 lines, -1 line 0 comments Download
M chrome_elf/chrome_elf_constants.h View 1 chunk +0 lines, -10 lines 0 comments Download
M chrome_elf/chrome_elf_constants.cc View 2 chunks +0 lines, -5 lines 0 comments Download

Messages

Total messages: 28 (22 generated)
penny
Hello Robert, Here's a stab at removing the blacklist dynamic changes/finch. Please review for sanity! ...
4 years, 5 months ago (2016-07-20 22:35:01 UTC) #11
robertshield
lgtm https://codereview.chromium.org/2163803003/diff/60001/chrome/installer/setup/uninstall.cc File chrome/installer/setup/uninstall.cc (left): https://codereview.chromium.org/2163803003/diff/60001/chrome/installer/setup/uninstall.cc#oldcode791 chrome/installer/setup/uninstall.cc:791: 0); // wow64_access I reckon we should leave ...
4 years, 5 months ago (2016-07-22 02:37:52 UTC) #14
penny
Thanks for your time Robert. Left the cleanup in there, with a comment. https://codereview.chromium.org/2163803003/diff/60001/chrome/installer/setup/uninstall.cc File ...
4 years, 4 months ago (2016-07-27 22:55:52 UTC) #19
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/2163803003/100001
4 years, 4 months ago (2016-07-27 23:20:44 UTC) #24
commit-bot: I haz the power
Committed patchset #5 (id:100001)
4 years, 4 months ago (2016-07-27 23:24:54 UTC) #26
commit-bot: I haz the power
4 years, 4 months ago (2016-07-27 23:27:13 UTC) #28
Message was sent while issue was closed.
Patchset 5 (id:??) landed as
https://crrev.com/c4da848392dabb7b49e554a15ad47d1c9f0dac78
Cr-Commit-Position: refs/heads/master@{#408269}

Powered by Google App Engine
This is Rietveld 408576698