[Ephemeral profiles] Clean up 'Custom Dictionary.txt' on window close

When the ForceEphemeralProfiles policy is enabled, Chrome tries to
clean up sensitive files when the last window in a profile is closed.
However, the Custom Dictionary.txt file is still populated, and it
can contain sensitive (e.g., a customer's name that was added to
spellcheck).

With this patch, the Custom Dictionary.txt file is properly cleared
when the last window in an ephemeral profile closes.

TESTED=Created a new profile in epehemeral mode, added some dictionary
       words, then closed it. Checked that
       Custom Dictionary.txt{,.backup} didn't contain the words. Also
       tested with Sync turned on, to make sure the words were not
       cleared from synced data.

Bug: 1132950
Change-Id: If50f0f78fc89a7bbdbc1b563856bc36288c53c5b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2441947Reviewed-by: Martin Šrámek <msramek@chromium.org>
Reviewed-by: Guillaume Jenkins <gujen@google.com>
Commit-Queue: Nicolas Ouellet-Payeur <nicolaso@chromium.org>
Cr-Commit-Position: refs/heads/master@{#816673}

chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc

@@ -65,6 +65,8 @@
 #include "chrome/browser/previews/previews_service_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
+#include "chrome/browser/spellchecker/spellcheck_factory.h"
+#include "chrome/browser/spellchecker/spellcheck_service.h"
 #include "chrome/browser/translate/chrome_translate_client.h"
 #include "chrome/browser/ui/find_bar/find_bar_state.h"
 #include "chrome/browser/ui/find_bar/find_bar_state_factory.h"
@@ -736,6 +738,17 @@ void ChromeBrowsingDataRemoverDelegate::RemoveEmbedderData(
     }
   }
 
+  //////////////////////////////////////////////////////////////////////////////
+  // DATA_TYPE_LOCAL_CUSTOM_DICTIONARY
+  if (remove_mask & DATA_TYPE_LOCAL_CUSTOM_DICTIONARY) {
+    auto* spellcheck = SpellcheckServiceFactory::GetForContext(profile_);
+    if (spellcheck) {
+      auto* dict = spellcheck->GetCustomDictionary();
+      if (dict)
+        dict->Clear();
+    }
+  }
+
   //////////////////////////////////////////////////////////////////////////////
   // DATA_TYPE_DURABLE_PERMISSION
   if (remove_mask & DATA_TYPE_DURABLE_PERMISSION) {

chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.h

@@ -77,6 +77,7 @@ class ChromeBrowsingDataRemoverDelegate
     DATA_TYPE_BOOKMARKS = DATA_TYPE_EMBEDDER_BEGIN << 10,
     DATA_TYPE_ISOLATED_ORIGINS = DATA_TYPE_EMBEDDER_BEGIN << 11,
     DATA_TYPE_ACCOUNT_PASSWORDS = DATA_TYPE_EMBEDDER_BEGIN << 12,
+    DATA_TYPE_LOCAL_CUSTOM_DICTIONARY = DATA_TYPE_EMBEDDER_BEGIN << 13,
 
     // Group datatypes.
 
@@ -122,7 +123,8 @@ class ChromeBrowsingDataRemoverDelegate
                      DATA_TYPE_HISTORY |           //
                      DATA_TYPE_PASSWORDS |         //
                      DATA_TYPE_CONTENT_SETTINGS |  //
-                     DATA_TYPE_BOOKMARKS,
+                     DATA_TYPE_BOOKMARKS |         //
+                     DATA_TYPE_LOCAL_CUSTOM_DICTIONARY,
 
     // Includes all available remove options. Meant to be used when the Profile
     // is scheduled to be deleted, and all possible data should be wiped from

chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc

@@ -52,12 +52,17 @@
 #include "chrome/browser/permissions/adaptive_quiet_notification_permission_ui_enabler.h"
 #include "chrome/browser/permissions/permission_decision_auto_blocker_factory.h"
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
+#include "chrome/browser/spellchecker/spellcheck_custom_dictionary.h"
+#include "chrome/browser/spellchecker/spellcheck_factory.h"
+#include "chrome/browser/spellchecker/spellcheck_service.h"
 #include "chrome/browser/ssl/stateful_ssl_host_state_delegate_factory.h"
 #include "chrome/browser/storage/durable_storage_permission_context.h"
 #include "chrome/browser/subresource_filter/subresource_filter_profile_context_factory.h"
 #include "chrome/browser/sync/profile_sync_service_factory.h"
 #include "chrome/browser/translate/chrome_translate_client.h"
+#include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_features.h"
+#include "chrome/common/chrome_paths.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/testing_profile.h"
@@ -1153,6 +1158,12 @@ class ChromeBrowsingDataRemoverDelegateTest : public testing::Test {
     profile_builder.AddTestingFactory(
         FaviconServiceFactory::GetInstance(),
         FaviconServiceFactory::GetDefaultFactory());
+    profile_builder.AddTestingFactory(
+        SpellcheckServiceFactory::GetInstance(),
+        base::BindRepeating([](content::BrowserContext* profile) {
+          return std::unique_ptr<KeyedService>(
+              new SpellcheckService(static_cast<Profile*>(profile)));
+        }));
 
     profile_ = profile_builder.Build();
 
@@ -3197,6 +3208,39 @@ TEST_F(ChromeBrowsingDataRemoverDelegateTest, WipeCrashData) {
 }
 #endif
 
+TEST_F(ChromeBrowsingDataRemoverDelegateTest, WipeCustomDictionaryData) {
+  base::FilePath dict_path =
+      GetProfile()->GetPath().Append(chrome::kCustomDictionaryFileName);
+  base::FilePath backup_path = dict_path.AddExtensionASCII("backup");
+
+  auto* spellcheck = SpellcheckServiceFactory::GetForContext(GetProfile());
+  ASSERT_NE(nullptr, spellcheck);
+  auto* dict = spellcheck->GetCustomDictionary();
+  ASSERT_NE(nullptr, dict);
+
+  auto change1 = std::make_unique<SpellcheckCustomDictionary::Change>();
+  change1->AddWord("wug");
+  dict->UpdateDictionaryFile(std::move(change1), dict_path);
+
+  auto change2 = std::make_unique<SpellcheckCustomDictionary::Change>();
+  change2->AddWord("spowing");
+  dict->UpdateDictionaryFile(std::move(change2), dict_path);
+
+  EXPECT_TRUE(base::PathExists(dict_path));
+  EXPECT_TRUE(base::PathExists(backup_path));
+
+  BlockUntilBrowsingDataRemoved(
+      base::Time(), base::Time::Max(),
+      ChromeBrowsingDataRemoverDelegate::DATA_TYPE_LOCAL_CUSTOM_DICTIONARY,
+      false);
+
+  std::string contents;
+  base::ReadFileToString(dict_path, &contents);
+  EXPECT_EQ(std::string::npos, contents.find("wug"));
+  EXPECT_EQ(std::string::npos, contents.find("spowing"));
+  EXPECT_FALSE(base::PathExists(backup_path));
+}
+
 TEST_F(ChromeBrowsingDataRemoverDelegateTest,
        WipeNotificationPermissionPromptOutcomesData) {
   PrefService* prefs = GetProfile()->GetPrefs();

chrome/browser/spellchecker/spellcheck_custom_dictionary.cc

@@ -161,7 +161,14 @@ void SaveDictionaryFileReliably(const base::FilePath& path,
   {
     base::ScopedBlockingCall scoped_blocking_call(
         FROM_HERE, base::BlockingType::MAY_BLOCK);
-    base::CopyFile(path, path.AddExtension(BACKUP_EXTENSION));
+    base::FilePath backup_path = path.AddExtension(BACKUP_EXTENSION);
+    if (!custom_words.empty()) {
+      base::CopyFile(path, backup_path);
+    } else {
+      // The wordlist was just cleared, clean up the .backup file for privacy
+      // reasons.
+      base::DeleteFile(backup_path);
+    }
     base::ImportantFileWriter::WriteFileAtomically(path, content.str());
   }
 }
@@ -211,6 +218,10 @@ void SpellcheckCustomDictionary::Change::RemoveWord(const std::string& word) {
   to_remove_.insert(word);
 }
 
+void SpellcheckCustomDictionary::Change::Clear() {
+  clear_ = true;
+}
+
 int SpellcheckCustomDictionary::Change::Sanitize(
     const std::set<std::string>& words) {
   int result = VALID_CHANGE;
@@ -265,6 +276,14 @@ bool SpellcheckCustomDictionary::HasWord(const std::string& word) const {
   return base::Contains(words_, word);
 }
 
+void SpellcheckCustomDictionary::Clear() {
+  std::unique_ptr<Change> dictionary_change(new Change);
+  dictionary_change->Clear();
+  Apply(*dictionary_change);
+  Notify(*dictionary_change);
+  Save(std::move(dictionary_change));
+}
+
 void SpellcheckCustomDictionary::AddObserver(Observer* observer) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DCHECK(observer);
@@ -424,6 +443,10 @@ void SpellcheckCustomDictionary::UpdateDictionaryFile(
 
   std::unique_ptr<LoadFileResult> result = LoadDictionaryFileReliably(path);
 
+  // Clear.
+  if (dictionary_change->clear())
+    result->words.clear();
+
   // Add words.
   result->words.insert(dictionary_change->to_add().begin(),
                        dictionary_change->to_add().end());
@@ -460,6 +483,8 @@ void SpellcheckCustomDictionary::OnLoaded(
 
 void SpellcheckCustomDictionary::Apply(const Change& dictionary_change) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  if (dictionary_change.clear())
+    words_.clear();
   if (!dictionary_change.to_add().empty()) {
     words_.insert(dictionary_change.to_add().begin(),
                   dictionary_change.to_add().end());

chrome/browser/spellchecker/spellcheck_custom_dictionary.h

@@ -57,6 +57,10 @@ class SpellcheckCustomDictionary : public SpellcheckDictionary,
     // Removes |word| in this change.
     void RemoveWord(const std::string& word);
 
+    // Clear the whole dictionary before doing other operations. When saved,
+    // also deletes the backup file.
+    void Clear();
+
     // Prepares this change to be applied to |words| by removing duplicate and
     // invalid words from words to be added and removing missing words from
     // words to be removed. Returns a bitmap of |ChangeSanitationResult| values.
@@ -70,8 +74,13 @@ class SpellcheckCustomDictionary : public SpellcheckDictionary,
       return to_remove_;
     }
 
+    // Returns true if the dictionary should be cleared first.
+    bool clear() const { return clear_; }
+
     // Returns true if there are no changes to be made. Otherwise returns false.
-    bool empty() const { return to_add_.empty() && to_remove_.empty(); }
+    bool empty() const {
+      return !clear_ && to_add_.empty() && to_remove_.empty();
+    }
 
    private:
     // The words to be added.
@@ -80,6 +89,9 @@ class SpellcheckCustomDictionary : public SpellcheckDictionary,
     // The words to be removed.
     std::set<std::string> to_remove_;
 
+    // Whether to clear everything before adding words.
+    bool clear_ = false;
+
     DISALLOW_COPY_AND_ASSIGN(Change);
   };
 
@@ -130,6 +142,9 @@ class SpellcheckCustomDictionary : public SpellcheckDictionary,
   // Returns true if the dictionary contains |word|. Otherwise returns false.
   bool HasWord(const std::string& word) const;
 
+  // Removes all words in the dictionary, and schedules a write to disk.
+  void Clear();
+
   // Adds |observer| to be notified of dictionary events and changes.
   void AddObserver(Observer* observer);
 
@@ -162,6 +177,9 @@ class SpellcheckCustomDictionary : public SpellcheckDictionary,
   friend class DictionarySyncIntegrationTestHelper;
   friend class SpellcheckCustomDictionaryTest;
 
+  FRIEND_TEST_ALL_PREFIXES(ChromeBrowsingDataRemoverDelegateTest,
+                           WipeCustomDictionaryData);
+
   // Returns the list of words in the custom spellcheck dictionary at |path|.
   // Validates that the custom dictionary file does not have duplicates and
   // contains only valid words. Must be called on the FILE thread.