Properly handle the Module SBDatabase when it's corrupted

chrome/browser/safe_browsing/safe_browsing_database.cc

 // Copyright (c) 2012 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/browser/safe_browsing/safe_browsing_database.h"
 
 #include <stddef.h>
 #include <stdint.h>
 #include <algorithm>
 #include <iterator>
@@ skipping 823 matching lines @@
     return false;
 
   // Reset objects in memory.
   scoped_ptr<WriteTransaction> txn = state_manager_.BeginWriteTransaction();
   txn->clear_prefix_gethash_cache();
   txn->SwapPrefixSet(PrefixSetId::BROWSE, nullptr);
   txn->SwapPrefixSet(PrefixSetId::UNWANTED_SOFTWARE, nullptr);
   txn->clear_ip_blacklist();
   txn->WhitelistEverything(SBWhitelistId::CSD);
   txn->WhitelistEverything(SBWhitelistId::DOWNLOAD);
+  txn->WhitelistEverything(SBWhitelistId::MODULE);
   return true;
 }
 
 bool SafeBrowsingDatabaseNew::ContainsBrowseUrl(
     const GURL& url,
     std::vector<SBPrefix>* prefix_hits,
     std::vector<SBFullHashResult>* cache_hits) {
   return PrefixSetContainsUrl(url, PrefixSetId::BROWSE, prefix_hits,
                               cache_hits);
 }
@@ skipping 869 matching lines @@
   const bool r10 = base::DeleteFile(
       UnwantedSoftwareDBFilename(db_state_manager_.filename_base()), false);
   if (!r10)
     RecordFailure(FAILURE_UNWANTED_SOFTWARE_PREFIX_SET_DELETE);
 
   const bool r11 = base::DeleteFile(
       ResourceBlacklistDBFilename(db_state_manager_.filename_base()), false);
   if (!r11)
     RecordFailure(FAILURE_RESOURCE_BLACKLIST_DELETE);
 
-  return r1 && r2 && r3 && r4 && r5 && r6 && r7 && r8 && r9 && r10 && r11;
+  const bool r12 =
+      module_whitelist_store_.get() ? module_whitelist_store_->Delete() : true;

[Nathan Parker, 2016/03/17 16:46:39]

This seems better than the above methods, overall, but is there a chance that
module_whitelist_store_ would be NULL _and_ the file exists? I think the answer
is no, but just wanted to confirm.

[proberge, 2016/03/17 18:11:00]

It's the same as the other whitelist delete logic (see lines 1682 to 1697). I
know very little about this code, but I think that if module_whitelist_store_ is
NULL then it can't be the corruption cause and will get deleted the next time it
is.

+  if (!r12)
+    RecordFailure(FAILURE_MODULE_WHITELIST_DELETE);
+
+  return r1 && r2 && r3 && r4 && r5 && r6 && r7 && r8 && r9 && r10 && r11 &&
+         r12;
 }
 
 void SafeBrowsingDatabaseNew::WritePrefixSet(const base::FilePath& db_filename,
                                              PrefixSetId prefix_set_id,
                                              FailureType write_failure_type) {
   DCHECK(db_task_runner_->RunsTasksOnCurrentThread());
 
   // Do not grab the lock to avoid contention while writing to disk. This is
   // safe as only this task runner can ever modify |state_manager_|'s prefix
   // sets anyways.
@@ skipping 170 matching lines @@
 
   // Histogram properties as in UMA_HISTOGRAM_COUNTS macro.
   base::HistogramBase* histogram_pointer = base::Histogram::FactoryGet(
       histogram_name, 1, 1000000, 50,
       base::HistogramBase::kUmaTargetedHistogramFlag);
 
   histogram_pointer->Add(file_size_kilobytes);
 }
 
 }  // namespace safe_browsing