Commit 3311e34c authored by Marijn Kruisselbrink's avatar Marijn Kruisselbrink Committed by Commit Bot

Revert "Create a ResetShortcutsComponent in CCT."

This reverts commit 1e6b827e.

Reason for revert: Tests are failing on https://ci.chromium.org/p/chromium/builders/ci/Win7%20Tests%20%281%29

For example:

[ RUN      ] ResetShortcutsComponentTest.ResetShortcutChromeTargetPreserveArguments
[3748:3728:1117/162648.254:1947251:ERROR:reset_shortcuts_component.cc(62)] Number of shortcuts to reset: 1
../../chrome/chrome_cleaner/components/reset_shortcuts_component_unittest.cc(144): error: Expected equality of these values:
  found_shortcuts[0].target_path
    Which is: L"E:\\b\\swarm_slave\\w\\itk5vfdk\\5116_696520538\\0\\scoped_dir3748_371919881\\d8473183-ca73-484f-bbdf-d7df1deebbed.tmp"
  fake_chrome_path_.value()
    Which is: L"e:\\b\\swarm_slave\\w\\itk5vfdk\\5116_696520538\\0\\scoped_dir3748_371919881\\d8473183-ca73-484f-bbdf-d7df1deebbed.tmp"
Stack trace:
Backtrace:
	chrome_cleaner::ResetShortcutsComponentTest_ResetShortcutChromeTargetPreserveArguments_Test::TestBody [0x0017FAAD+1389]

[  FAILED  ] ResetShortcutsComponentTest.ResetShortcutChromeTargetPreserveArguments (172 ms)


Original change's description:
> Create a ResetShortcutsComponent in CCT.
>
> The ResetShortcutsComponent will use the
> existing sandboxed shortcut parser to retrieve
> modified shortcuts and then overwrites the
> existing shortcuts with new ones. The
> component will be hooked up to the CCT in a
> future CL.
>
> Bug: 1116017,1148930
> Change-Id: Ia6c8453af267b98884804eed3b18944afb0506ba
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2520419
> Reviewed-by: Albert J. Wong <ajwong@chromium.org>
> Reviewed-by: Will Harris <wfh@chromium.org>
> Reviewed-by: Varun Khaneja <vakh@chromium.org>
> Commit-Queue: Bettina Dea <bdea@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#828453}

TBR=ajwong@chromium.org,veranika@chromium.org,vakh@chromium.org,wfh@chromium.org,bdea@chromium.org

Change-Id: I87e55d98da30d41141ca5c09585c02e78aa64022
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1116017
Bug: 1148930
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2545895Reviewed-by: default avatarMarijn Kruisselbrink <mek@chromium.org>
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#828513}
parent 757d190a
...@@ -138,7 +138,6 @@ namespace chrome_browser_net { ...@@ -138,7 +138,6 @@ namespace chrome_browser_net {
class Predictor; class Predictor;
} }
namespace chrome_cleaner { namespace chrome_cleaner {
class ResetShortcutsComponent;
class SystemReportComponent; class SystemReportComponent;
} }
namespace content { namespace content {
...@@ -451,7 +450,6 @@ class BASE_EXPORT ScopedAllowBaseSyncPrimitives { ...@@ -451,7 +450,6 @@ class BASE_EXPORT ScopedAllowBaseSyncPrimitives {
friend class blink::SourceStream; friend class blink::SourceStream;
friend class blink::WorkerThread; friend class blink::WorkerThread;
friend class blink::scheduler::WorkerThread; friend class blink::scheduler::WorkerThread;
friend class chrome_cleaner::ResetShortcutsComponent;
friend class chrome_cleaner::SystemReportComponent; friend class chrome_cleaner::SystemReportComponent;
friend class content::BrowserMainLoop; friend class content::BrowserMainLoop;
friend class content::BrowserProcessSubThread; friend class content::BrowserProcessSubThread;
......
...@@ -14,8 +14,6 @@ source_set("components") { ...@@ -14,8 +14,6 @@ source_set("components") {
"component_unpacker.h", "component_unpacker.h",
"recovery_component.cc", "recovery_component.cc",
"recovery_component.h", "recovery_component.h",
"reset_shortcuts_component.cc",
"reset_shortcuts_component.h",
"system_report_component.cc", "system_report_component.cc",
"system_report_component.h", "system_report_component.h",
"system_restore_point_component.cc", "system_restore_point_component.cc",
...@@ -41,7 +39,6 @@ source_set("components") { ...@@ -41,7 +39,6 @@ source_set("components") {
"//chrome/chrome_cleaner/parsers/parser_utils:command_line_arguments_sanitizer", "//chrome/chrome_cleaner/parsers/parser_utils:command_line_arguments_sanitizer",
"//chrome/chrome_cleaner/parsers/shortcut_parser/broker:sandboxed_shortcut_parser", "//chrome/chrome_cleaner/parsers/shortcut_parser/broker:sandboxed_shortcut_parser",
"//chrome/chrome_cleaner/pup_data:pup_data_base", "//chrome/chrome_cleaner/pup_data:pup_data_base",
"//chrome/common:constants",
"//components/chrome_cleaner/public/constants:constants", "//components/chrome_cleaner/public/constants:constants",
"//components/crx_file", "//components/crx_file",
"//crypto", "//crypto",
...@@ -56,7 +53,6 @@ source_set("unittest_sources") { ...@@ -56,7 +53,6 @@ source_set("unittest_sources") {
sources = [ sources = [
"component_manager_unittest.cc", "component_manager_unittest.cc",
"recovery_component_unittest.cc", "recovery_component_unittest.cc",
"reset_shortcuts_component_unittest.cc",
"system_report_component_unittest.cc", "system_report_component_unittest.cc",
"system_restore_point_component_unittest.cc", "system_restore_point_component_unittest.cc",
] ]
...@@ -70,26 +66,20 @@ source_set("unittest_sources") { ...@@ -70,26 +66,20 @@ source_set("unittest_sources") {
"//chrome/chrome_cleaner/constants:common_strings", "//chrome/chrome_cleaner/constants:common_strings",
"//chrome/chrome_cleaner/constants:uws_id", "//chrome/chrome_cleaner/constants:uws_id",
"//chrome/chrome_cleaner/http:mock_http_agent_factory", "//chrome/chrome_cleaner/http:mock_http_agent_factory",
"//chrome/chrome_cleaner/ipc:mojo_task_runner",
"//chrome/chrome_cleaner/logging:cleaner_logging", "//chrome/chrome_cleaner/logging:cleaner_logging",
"//chrome/chrome_cleaner/logging:common", "//chrome/chrome_cleaner/logging:common",
"//chrome/chrome_cleaner/logging/proto:chrome_cleaner_report_proto", "//chrome/chrome_cleaner/logging/proto:chrome_cleaner_report_proto",
"//chrome/chrome_cleaner/os:cleaner_os", "//chrome/chrome_cleaner/os:cleaner_os",
"//chrome/chrome_cleaner/os:common_os", "//chrome/chrome_cleaner/os:common_os",
"//chrome/chrome_cleaner/parsers/broker:parser_sandbox_broker",
"//chrome/chrome_cleaner/parsers/json_parser", "//chrome/chrome_cleaner/parsers/json_parser",
"//chrome/chrome_cleaner/parsers/parser_utils:command_line_arguments_sanitizer", "//chrome/chrome_cleaner/parsers/parser_utils:command_line_arguments_sanitizer",
"//chrome/chrome_cleaner/parsers/shortcut_parser:sandboxed_lnk_parser_test_util",
"//chrome/chrome_cleaner/parsers/shortcut_parser/broker:fake_shortcut_parser", "//chrome/chrome_cleaner/parsers/shortcut_parser/broker:fake_shortcut_parser",
"//chrome/chrome_cleaner/parsers/target:parser_sandbox_target",
"//chrome/chrome_cleaner/test:test_branding_header", "//chrome/chrome_cleaner/test:test_branding_header",
"//chrome/chrome_cleaner/test:test_component", "//chrome/chrome_cleaner/test:test_component",
"//chrome/chrome_cleaner/test:test_extensions", "//chrome/chrome_cleaner/test:test_extensions",
"//chrome/chrome_cleaner/test:test_pup_data", "//chrome/chrome_cleaner/test:test_pup_data",
"//chrome/chrome_cleaner/test:test_util", "//chrome/chrome_cleaner/test:test_util",
"//components/chrome_cleaner/public/constants", "//components/chrome_cleaner/public/constants",
"//mojo/public/cpp/system:system",
"//sandbox/win:sandbox",
"//testing/gtest", "//testing/gtest",
] ]
} }
include_rules = [ include_rules = [
"+chrome/common/chrome_switches.h",
"+components/crx_file", "+components/crx_file",
"+third_party/zlib/google", "+third_party/zlib/google",
] ]
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "chrome/chrome_cleaner/components/reset_shortcuts_component.h"
#include <windows.h>
#include <stdint.h>
#include <memory>
#include <string>
#include <vector>
#include "base/base_paths.h"
#include "base/bind.h"
#include "base/callback_helpers.h"
#include "base/command_line.h"
#include "base/files/file_enumerator.h"
#include "base/files/file_path.h"
#include "base/files/file_util.h"
#include "base/path_service.h"
#include "base/stl_util.h"
#include "base/strings/stringprintf.h"
#include "base/synchronization/waitable_event.h"
#include "base/task/task_traits.h"
#include "base/threading/scoped_blocking_call.h"
#include "base/threading/thread_restrictions.h"
#include "base/win/shortcut.h"
#include "base/win/win_util.h"
#include "chrome/chrome_cleaner/chrome_utils/chrome_util.h"
#include "chrome/chrome_cleaner/constants/chrome_cleaner_switches.h"
#include "chrome/chrome_cleaner/logging/logging_service_api.h"
#include "chrome/chrome_cleaner/os/file_path_set.h"
#include "chrome/chrome_cleaner/os/task_scheduler.h"
#include "chrome/chrome_cleaner/parsers/parser_utils/command_line_arguments_sanitizer.h"
#include "chrome/chrome_cleaner/parsers/shortcut_parser/broker/sandboxed_shortcut_parser.h"
#include "chrome/common/chrome_switches.h"
#include "components/chrome_cleaner/public/constants/constants.h"
using base::WaitableEvent;
namespace chrome_cleaner {
namespace {
std::vector<base::FilePath> GetPathsToExplore() {
std::vector<int> keys_of_paths_to_explore = {
base::DIR_USER_DESKTOP, base::DIR_COMMON_DESKTOP,
base::DIR_USER_QUICK_LAUNCH, base::DIR_START_MENU,
base::DIR_COMMON_START_MENU, base::DIR_TASKBAR_PINS};
std::vector<base::FilePath> paths_to_explore;
for (int path_key : keys_of_paths_to_explore) {
base::FilePath path;
if (base::PathService::Get(path_key, &path))
paths_to_explore.push_back(path);
}
return paths_to_explore;
}
void ResetShortcuts(std::vector<ShortcutInformation> shortcuts,
const FilePathSet& chrome_exe_paths) {
LOG(ERROR) << "Number of shortcuts to reset: " << shortcuts.size();
for (const ShortcutInformation& shortcut : shortcuts) {
base::FilePath shortcut_path(shortcut.lnk_path);
base::ScopedBlockingCall scoped_blocking_call(
FROM_HERE, base::BlockingType::MAY_BLOCK);
// Clear arguments that are custom-made.
base::win::ShortcutProperties updated_properties;
// Use the first chrome.exe path in the set.
updated_properties.set_target(chrome_exe_paths.ToVector()[0]);
// Additional Chrome profiles may have custom icons so the icon location
// should be preserved.
base::FilePath icon_location(shortcut.icon_location);
// TODO(bdea): Get the correct icon_index from the lnk_parser.
updated_properties.set_icon(icon_location, /*icon_index=*/0);
base::CommandLine current_args(
base::CommandLine::FromString(base::StringPrintf(
L"unused_program %ls", shortcut.command_line_arguments.c_str())));
const char* const kept_switches[] = {
switches::kApp,
switches::kAppId,
switches::kProfileDirectory,
};
base::CommandLine desired_args(base::CommandLine::NO_PROGRAM);
desired_args.CopySwitchesFrom(current_args, kept_switches,
base::size(kept_switches));
updated_properties.set_arguments(desired_args.GetArgumentsString());
bool success = base::win::CreateOrUpdateShortcutLink(
shortcut_path, updated_properties, base::win::SHORTCUT_CREATE_ALWAYS);
if (!success)
LOG(ERROR) << "Reset shortcut failed on: "
<< SanitizePath(shortcut.lnk_path);
}
}
} // namespace
ResetShortcutsComponent::ResetShortcutsComponent(
ShortcutParserAPI* shortcut_parser)
: shortcut_parser_(shortcut_parser) {
shortcut_paths_to_explore_ = GetPathsToExplore();
std::set<base::FilePath> chrome_exe_paths;
ListChromeExePaths(&chrome_exe_paths);
for (const auto& path : chrome_exe_paths) {
chrome_exe_file_path_set_.Insert(path);
}
if (chrome_exe_paths.size() > 1) {
LOG(WARNING) << "More than one chrome.exe candidate for target_path. Path "
"to be used: "
<< SanitizePath(chrome_exe_file_path_set_.ToVector()[0]);
}
}
ResetShortcutsComponent::~ResetShortcutsComponent() {}
void ResetShortcutsComponent::PreScan() {}
void ResetShortcutsComponent::PostScan(const std::vector<UwSId>& found_pups) {
// If no removable UwS was found, we should reset shortcuts now since there
// won't be a post-cleanup called.
if (found_pups.size() == 0 ||
!PUPData::HasFlaggedPUP(found_pups, &PUPData::HasRemovalFlag)) {
base::ScopedAllowBaseSyncPrimitives allow_sync;
FindAndResetShortcuts();
}
}
void ResetShortcutsComponent::PreCleanup() {}
void ResetShortcutsComponent::PostCleanup(ResultCode result_code,
RebooterAPI* rebooter) {
// If the user cancels the cleanup, don't reset shortcutss.
if (result_code == RESULT_CODE_CANCELED)
return;
{
base::ScopedAllowBaseSyncPrimitives allow_sync;
FindAndResetShortcuts();
}
}
void ResetShortcutsComponent::PostValidation(ResultCode result_code) {}
void ResetShortcutsComponent::OnClose(ResultCode result_code) {}
std::vector<ShortcutInformation> ResetShortcutsComponent::GetShortcuts() {
std::vector<ShortcutInformation> shortcuts_found;
if (!shortcut_parser_)
return shortcuts_found;
base::WaitableEvent event(WaitableEvent::ResetPolicy::MANUAL,
WaitableEvent::InitialState::NOT_SIGNALED);
shortcut_parser_->FindAndParseChromeShortcutsInFoldersAsync(
shortcut_paths_to_explore_, chrome_exe_file_path_set_,
base::BindOnce(
[](base::WaitableEvent* event,
std::vector<ShortcutInformation>* shortcuts_found,
std::vector<ShortcutInformation> parsed_shortcuts) {
*shortcuts_found = parsed_shortcuts;
event->Signal();
},
&event, &shortcuts_found));
event.Wait();
return shortcuts_found;
}
void ResetShortcutsComponent::FindAndResetShortcuts() {
// A return here means that lnk shortcut analysis is not enabled on the
// command line.
if (!shortcut_parser_)
return;
std::vector<ShortcutInformation> shortcuts_found = GetShortcuts();
ResetShortcuts(shortcuts_found, chrome_exe_file_path_set_);
}
void ResetShortcutsComponent::SetShortcutPathsToExploreForTesting(
const std::vector<base::FilePath>& fake_shortcut_location_paths_) {
shortcut_paths_to_explore_ = fake_shortcut_location_paths_;
}
void ResetShortcutsComponent::SetChromeExeFilePathSetForTesting(
const FilePathSet& fake_chrome_exe_file_path_set) {
chrome_exe_file_path_set_ = fake_chrome_exe_file_path_set;
}
} // namespace chrome_cleaner
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef CHROME_CHROME_CLEANER_COMPONENTS_RESET_SHORTCUTS_COMPONENT_H_
#define CHROME_CHROME_CLEANER_COMPONENTS_RESET_SHORTCUTS_COMPONENT_H_
#include <vector>
#include "chrome/chrome_cleaner/components/component_api.h"
#include "chrome/chrome_cleaner/parsers/shortcut_parser/broker/sandboxed_shortcut_parser.h"
namespace chrome_cleaner {
// This class manages the resetting of shortcuts.
class ResetShortcutsComponent : public ComponentAPI {
public:
explicit ResetShortcutsComponent(ShortcutParserAPI* shortcut_parser);
~ResetShortcutsComponent() override;
// ComponentAPI methods.
void PreScan() override;
void PostScan(const std::vector<UwSId>& found_pups) override;
void PreCleanup() override;
void PostCleanup(ResultCode result_code, RebooterAPI* rebooter) override;
void PostValidation(ResultCode result_code) override;
void OnClose(ResultCode result_code) override;
// Get the list of shortcuts to reset.
std::vector<ShortcutInformation> GetShortcuts();
// Find and reset shortcuts while preserving the icon location and some
// command line flags.
void FindAndResetShortcuts();
void SetShortcutPathsToExploreForTesting(
const std::vector<base::FilePath>& fake_shortcut_location_paths_);
void SetChromeExeFilePathSetForTesting(
const FilePathSet& fake_chrome_exe_file_path_set);
private:
ShortcutParserAPI* shortcut_parser_;
std::vector<base::FilePath> shortcut_paths_to_explore_;
FilePathSet chrome_exe_file_path_set_;
};
} // namespace chrome_cleaner
#endif // CHROME_CHROME_CLEANER_COMPONENTS_RESET_SHORTCUTS_COMPONENT_H_
...@@ -130,9 +130,7 @@ void SandboxedShortcutParser::OnShortcutsParsingDone( ...@@ -130,9 +130,7 @@ void SandboxedShortcutParser::OnShortcutsParsingDone(
const std::wstring kChromeLnkName = L"Google Chrome.lnk"; const std::wstring kChromeLnkName = L"Google Chrome.lnk";
if (chrome_exe_locations.Contains( if (chrome_exe_locations.Contains(
base::FilePath(parsed_shortcut.icon_location)) || base::FilePath(parsed_shortcut.icon_location)) ||
lnk_path.BaseName().value() == kChromeLnkName || lnk_path.BaseName().value() == kChromeLnkName) {
chrome_exe_locations.Contains(
base::FilePath(parsed_shortcut.target_path))) {
base::AutoLock lock(lock_); base::AutoLock lock(lock_);
found_shortcuts->push_back(parsed_shortcut); found_shortcuts->push_back(parsed_shortcut);
} }
......
...@@ -6,12 +6,10 @@ ...@@ -6,12 +6,10 @@
#include <set> #include <set>
#include "base/base_paths.h"
#include "base/bind.h" #include "base/bind.h"
#include "base/files/file_util.h" #include "base/files/file_util.h"
#include "base/files/scoped_temp_dir.h" #include "base/files/scoped_temp_dir.h"
#include "base/memory/scoped_refptr.h" #include "base/memory/scoped_refptr.h"
#include "base/path_service.h"
#include "base/run_loop.h" #include "base/run_loop.h"
#include "base/test/multiprocess_test.h" #include "base/test/multiprocess_test.h"
#include "base/test/task_environment.h" #include "base/test/task_environment.h"
...@@ -171,44 +169,6 @@ TEST_F(SandboxedShortcutParserTest, ParseMultipleFoldersWithChromeLnk) { ...@@ -171,44 +169,6 @@ TEST_F(SandboxedShortcutParserTest, ParseMultipleFoldersWithChromeLnk) {
} }
} }
TEST_F(SandboxedShortcutParserTest,
ParseShortcutWithChromeTargetDifferentName) {
base::ScopedTempDir temp_dir;
ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
base::win::ShortcutProperties shortcut;
base::FilePath fake_chrome_path =
fake_chrome_exe_file_path_set_.ToVector()[0];
shortcut.set_target(fake_chrome_path);
base::win::ScopedHandle fake_chrome_shortcut_handle =
CreateAndOpenShortcutInTempDir("Google Chrome-New Profile.lnk", shortcut,
&temp_dir);
ASSERT_TRUE(fake_chrome_shortcut_handle.IsValid());
base::RunLoop run_loop;
std::vector<ShortcutInformation> found_shortcuts;
FilePathSet empty_file_path_set;
shortcut_parser_->FindAndParseChromeShortcutsInFoldersAsync(
{
temp_dir.GetPath(),
},
fake_chrome_exe_file_path_set_,
base::BindOnce(
[](std::vector<ShortcutInformation>* found_shortcuts,
base::OnceClosure closure,
std::vector<ShortcutInformation> parsed_shortcuts) {
*found_shortcuts = parsed_shortcuts;
std::move(closure).Run();
},
&found_shortcuts, run_loop.QuitClosure()));
run_loop.Run();
ASSERT_EQ(found_shortcuts.size(), 1ul);
EXPECT_TRUE(PathEqual(fake_chrome_path,
base::FilePath(found_shortcuts[0].target_path)));
}
TEST_F(SandboxedShortcutParserTest, ParseShortcutWithEmptyIconSet) { TEST_F(SandboxedShortcutParserTest, ParseShortcutWithEmptyIconSet) {
base::ScopedTempDir temp_dir; base::ScopedTempDir temp_dir;
ASSERT_TRUE(temp_dir.CreateUniqueTempDir()); ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment