[BrowserSwitcher] Only write/delete cache.dat in Chrome builds

In Chromium builds, cache.dat and sitelistcache.dat will neither be written by
nor cleaned up by Legacy Browser Support.

Bug: 1054573
Change-Id: I9f0a782dc0011465dfb192cbe381892b97fc4c3e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2074920
Commit-Queue: Nicolas Ouellet-Payeur <nicolaso@chromium.org>
Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org>
Auto-Submit: Nicolas Ouellet-Payeur <nicolaso@chromium.org>
Cr-Commit-Position: refs/heads/master@{#745111}

chrome/browser/browser_switcher/browser_switcher_service_browsertest.cc

@@ -107,8 +107,21 @@ class BrowserSwitcherServiceTest : public InProcessBrowserTest {
     BrowserSwitcherService::SetRefreshDelayForTesting(action_timeout() * 3 / 2);
 #if defined(OS_WIN)
     ASSERT_TRUE(fake_appdata_dir_.CreateUniqueTempDir());
-    base::PathService::Override(base::DIR_LOCAL_APP_DATA,
-                                fake_appdata_dir_.GetPath());
+#endif
+  }
+
+  void SetUpOnMainThread() override {
+#if defined(OS_WIN)
+    BrowserSwitcherServiceFactory::GetInstance()->SetTestingFactory(
+        browser()->profile(),
+        base::BindRepeating(
+            [](base::FilePath cache_dir, content::BrowserContext* context) {
+              auto* instance = new BrowserSwitcherServiceWin(
+                  Profile::FromBrowserContext(context), cache_dir);
+              instance->Init();
+              return std::unique_ptr<KeyedService>(instance);
+            },
+            cache_dir()));
 #endif
   }
 

chrome/browser/browser_switcher/browser_switcher_service_win.cc

@@ -39,16 +39,6 @@ const wchar_t kIeSiteListValue[] = L"SiteList";
 
 const int kCurrentFileVersion = 1;
 
-// Returns "AppData\Local\Google\BrowserSwitcher".
-base::FilePath GetCacheDir() {
-  base::FilePath path;
-  if (!base::PathService::Get(base::DIR_LOCAL_APP_DATA, &path))
-    return path;
-  path = path.AppendASCII("Google");
-  path = path.AppendASCII("BrowserSwitcher");
-  return path;
-}
-
 // Creates a RuleSet that is the concatenation of all 3 sources.
 RuleSet GetRules(const BrowserSwitcherPrefs& prefs,
                  const BrowserSwitcherSitelist* sitelist) {
@@ -94,12 +84,8 @@ std::string SerializeCacheFile(const BrowserSwitcherPrefs& prefs,
   return buffer.str();
 }
 
-void SaveDataToFile(const std::string& data, base::StringPiece file_name) {
-  base::FilePath dir = GetCacheDir();
-
-  if (dir.empty())
-    return;
-
+void SaveDataToFile(const std::string& data, base::FilePath path) {
+  base::FilePath dir = path.DirName();
   // Ensure the directory exists.
   bool success = base::CreateDirectory(dir);
   UMA_HISTOGRAM_BOOLEAN("BrowserSwitcher.CacheFile.MkDirSuccess", success);
@@ -119,21 +105,12 @@ void SaveDataToFile(const std::string& data, base::StringPiece file_name) {
 
   base::WriteFile(tmp_path, data.c_str(), data.size());
 
-  base::FilePath dest_path = dir.AppendASCII(file_name);
-  success = base::Move(tmp_path, dest_path);
+  success = base::Move(tmp_path, path);
   UMA_HISTOGRAM_BOOLEAN("BrowserSwitcher.CacheFile.MoveSuccess", success);
 }
 
-// Delete the file at "AppData\Local\Google\BrowserSwitcher\<file_name>".
-void DoRemoveFileFromCacheDir(std::string file_name) {
-  base::FilePath dir = GetCacheDir();
-
-  if (dir.empty())
-    return;
-
-  // Ignore errors while deleting.
-  base::FilePath dest_path = dir.AppendASCII(file_name);
-  base::DeleteFile(dest_path, false);
+void RemoveFile(base::FilePath path) {
+  base::DeleteFile(path, false);
 }
 
 // URL to fetch the IEEM sitelist from. Only used for testing.
@@ -152,8 +129,11 @@ bool IsLBSExtensionEnabled(Profile* profile) {
 
 }  // namespace
 
-BrowserSwitcherServiceWin::BrowserSwitcherServiceWin(Profile* profile)
+BrowserSwitcherServiceWin::BrowserSwitcherServiceWin(
+    Profile* profile,
+    base::FilePath cache_dir_for_testing)
     : BrowserSwitcherService(profile),
+      cache_dir_for_testing_(std::move(cache_dir_for_testing)),
       sequenced_task_runner_(base::ThreadPool::CreateSequencedTaskRunner(
           {base::MayBlock(), base::TaskPriority::BEST_EFFORT,
            base::TaskShutdownBehavior::BLOCK_SHUTDOWN})) {
@@ -196,6 +176,21 @@ void BrowserSwitcherServiceWin::LoadRulesFromPrefs() {
   SavePrefsToFile();
 }
 
+base::FilePath BrowserSwitcherServiceWin::GetCacheDir() {
+  if (!cache_dir_for_testing_.empty())
+    return cache_dir_for_testing_;
+#if defined(GOOGLE_CHROME_BUILD)
+  base::FilePath path;
+  if (!base::PathService::Get(base::DIR_LOCAL_APP_DATA, &path))
+    return path;
+  path = path.AppendASCII("Google");
+  path = path.AppendASCII("BrowserSwitcher");
+  return path;
+#else
+  return base::FilePath();
+#endif
+}
+
 void BrowserSwitcherServiceWin::OnAllRulesetsParsed() {
   BrowserSwitcherService::OnAllRulesetsParsed();
 
@@ -239,21 +234,32 @@ void BrowserSwitcherServiceWin::OnIeemSitelistParsed(ParsedXml xml) {
 
 void BrowserSwitcherServiceWin::SavePrefsToFile() {
   DCHECK(prefs().IsEnabled());
+  base::FilePath path = GetCacheDir();
+  if (path.empty())
+    return;
+  path = path.AppendASCII("cache.dat");
   sequenced_task_runner_->PostTask(
       FROM_HERE,
       base::BindOnce(&SaveDataToFile, SerializeCacheFile(prefs(), sitelist()),
-                     "cache.dat"));
+                     std::move(path)));
 }
 
 void BrowserSwitcherServiceWin::DeletePrefsFile() {
+  base::FilePath path = GetCacheDir();
+  if (path.empty())
+    return;
+  path = path.AppendASCII("cache.dat");
   sequenced_task_runner_->PostTask(
-      FROM_HERE, base::BindOnce(&DoRemoveFileFromCacheDir, "cache.dat"));
+      FROM_HERE, base::BindOnce(&RemoveFile, std::move(path)));
 }
 
 void BrowserSwitcherServiceWin::DeleteSitelistCacheFile() {
+  base::FilePath path = GetCacheDir();
+  if (path.empty())
+    return;
+  path = path.AppendASCII("sitelistcache.dat");
   sequenced_task_runner_->PostTask(
-      FROM_HERE,
-      base::BindOnce(&DoRemoveFileFromCacheDir, "sitelistcache.dat"));
+      FROM_HERE, base::BindOnce(&RemoveFile, std::move(path)));
 }
 
 void BrowserSwitcherServiceWin::UpdateAllCacheFiles() {

chrome/browser/browser_switcher/browser_switcher_service_win.h

@@ -8,6 +8,7 @@
 #include <memory>
 #include <string>
 
+#include "base/files/file_path.h"
 #include "base/optional.h"
 #include "chrome/browser/browser_switcher/browser_switcher_service.h"
 
@@ -16,7 +17,9 @@ namespace browser_switcher {
 // Windows-specific extension of BrowserSwitcherService.
 class BrowserSwitcherServiceWin : public BrowserSwitcherService {
  public:
-  explicit BrowserSwitcherServiceWin(Profile* profile);
+  explicit BrowserSwitcherServiceWin(
+      Profile* profile,
+      base::FilePath cache_dir_for_testing = base::FilePath());
   ~BrowserSwitcherServiceWin() override;
 
   static void SetIeemSitelistUrlForTesting(const std::string& url);
@@ -35,6 +38,9 @@ class BrowserSwitcherServiceWin : public BrowserSwitcherService {
       const std::vector<std::string>& changed_prefs) override;
 
  private:
+  // Returns "AppData\Local\Google\BrowserSwitcher", in official builds.
+  base::FilePath GetCacheDir();
+
   // Returns the URL to fetch to get Internet Explorer's Enterprise Mode
   // sitelist, based on policy. Returns an empty (invalid) URL if IE's SiteList
   // policy is unset, or if |use_ie_sitelist| is false.
@@ -57,6 +63,8 @@ class BrowserSwitcherServiceWin : public BrowserSwitcherService {
   // BrowserSwitcher is enabled or disabled.
   void UpdateAllCacheFiles();
 
+  base::FilePath cache_dir_for_testing_;
+
   scoped_refptr<base::SequencedTaskRunner> sequenced_task_runner_;
 
   base::WeakPtrFactory<BrowserSwitcherServiceWin> weak_ptr_factory_{this};