Use FileTypePolicies for is_archive and is_supported classifications.

chrome/common/safe_browsing/file_type_policies.cc

Index: chrome/common/safe_browsing/file_type_policies.cc
diff --git a/chrome/common/safe_browsing/file_type_policies.cc b/chrome/common/safe_browsing/file_type_policies.cc
index 09fa1fb15702a0cdacfddf3078c0c4c8da349ee3..c668bef380bd8790f9b5d5b71fe9fc8e7f4a6c6a 100644
--- a/chrome/common/safe_browsing/file_type_policies.cc
+++ b/chrome/common/safe_browsing/file_type_policies.cc
@@ -2,17 +2,37 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "chrome/common/safe_browsing/file_type_policies.h"
+
 #include "base/logging.h"
 #include "base/metrics/sparse_histogram.h"
 #include "base/strings/string_util.h"
-#include "chrome/common/safe_browsing/download_protection_util.h"
-#include "chrome/common/safe_browsing/file_type_policies.h"
 #include "chrome/grit/browser_resources.h"
 #include "chrome/grit/generated_resources.h"
 #include "ui/base/resource/resource_bundle.h"
 
 namespace safe_browsing {
 
+using base::AutoLock;
+
+// --- FileTypePoliciesManager methods ---
+FileTypePoliciesManager::FileTypePoliciesManager() {
+  if (FileTypePolicies::global_instance_ != nullptr)
+    return;
+  file_type_policies_.reset(new FileTypePolicies());
+  file_type_policies_->PopulateFromResourceBundle();
+  FileTypePolicies::SetGlobalInstance(file_type_policies_.get());
+}
+
+FileTypePoliciesManager::~FileTypePoliciesManager() {
+  if (file_type_policies_.get())
+    FileTypePolicies::SetGlobalInstance(nullptr);

[asanka, 2016/05/18 18:47:54]

This one is a bit iffy. I'd lean towards leaking the FileTypePolicies object
because the pointer returned by GetGlobalInstance() is supposed to be thread
safe and has no lifetime guarantees.

Or, alternatively, we could use a singleton here. Those get cleaned up at AtExit
time, which should be safe.

[Nathan Parker, 2016/05/18 23:53:28]

I've switched to Singleton.  Actually, this in combination with the fact that
I've made all access/update thread-safe means it can be constructed on any
thread, so lifetime management is a lot simpler.  Nice!

+}
+
+
+// --- FileTypePolicies methods ---
+FileTypePolicies* FileTypePolicies::global_instance_ = nullptr;
+
 FileTypePolicies::FileTypePolicies() {
   // Setup a file-type policy to use if the ResourceBundle is unreadable.
   // This should normally never be used.
@@ -23,7 +43,22 @@ FileTypePolicies::FileTypePolicies() {
   settings->set_auto_open_hint(DownloadFileType::DISALLOW_AUTO_OPEN);
 }
 
-FileTypePolicies::~FileTypePolicies() {}
+FileTypePolicies::~FileTypePolicies() {
+  AutoLock lock(lock_);  // DCHECK fail if the lock is held.
+}
+
+// static
+FileTypePolicies* FileTypePolicies::GlobalInstance() {
+  DCHECK(global_instance_ != nullptr)
+      << "You must instantiate FileTypePoliciesManager in "
+         "this process before calling GlobalInstance()";
+  return global_instance_;
+}
+
+// static
+void FileTypePolicies::SetGlobalInstance(FileTypePolicies* instance) {
+  global_instance_ = instance;
+}
 
 void FileTypePolicies::ReadResourceBundle(std::string* binary_pb) {
   ui::ResourceBundle& bundle = ui::ResourceBundle::GetSharedInstance();
@@ -32,6 +67,7 @@ void FileTypePolicies::ReadResourceBundle(std::string* binary_pb) {
 
 void FileTypePolicies::RecordUpdateMetrics(UpdateResult result,
                                            const std::string& src_name) {
+  lock_.AssertAcquired();
   // src_name should be "ResourceBundle" or "DynamicUpdate".
   UMA_HISTOGRAM_SPARSE_SLOWLY(
       "SafeBrowsing.FileTypeUpdate." + src_name + "Result",
@@ -48,6 +84,7 @@ void FileTypePolicies::RecordUpdateMetrics(UpdateResult result,
 }
 
 void FileTypePolicies::PopulateFromResourceBundle() {
+  AutoLock lock(lock_);
   std::string binary_pb;
   ReadResourceBundle(&binary_pb);
   UpdateResult result = PopulateFromBinaryPb(binary_pb);
@@ -55,12 +92,15 @@ void FileTypePolicies::PopulateFromResourceBundle() {
 }
 
 void FileTypePolicies::PopulateFromDynamicUpdate(const std::string& binary_pb) {
+  AutoLock lock(lock_);
   UpdateResult result = PopulateFromBinaryPb(binary_pb);
   RecordUpdateMetrics(result, "DynamicUpdate");
 }
 
 FileTypePolicies::UpdateResult FileTypePolicies::PopulateFromBinaryPb(
     const std::string& binary_pb) {
+  lock_.AssertAcquired();
+
   // Parse the proto and do some validation on it.
   if (binary_pb.empty())
     return UpdateResult::FAILED_EMPTY;
@@ -105,16 +145,26 @@ FileTypePolicies::UpdateResult FileTypePolicies::PopulateFromBinaryPb(
   return UpdateResult::SUCCESS;
 }
 
-float FileTypePolicies::SampledPingProbability() const {
+float FileTypePolicies::SampledPingProbability() {
+  AutoLock lock(lock_);
   return config_ ? config_->sampled_ping_probability() : 0.0;
 }
 
 // static
+const base::FilePath::StringType FileTypePolicies::GetFileExtension(
+    const base::FilePath& file) {
+  // Remove trailing space and period characters from the extension.
+  base::FilePath::StringType file_basename = file.BaseName().value();
+  base::FilePath::StringPieceType trimmed_filename = base::TrimString(
+      file_basename, FILE_PATH_LITERAL(". "), base::TRIM_TRAILING);
+  return base::FilePath(trimmed_filename).FinalExtension();
+}
+
+// static
 std::string FileTypePolicies::CanonicalizedExtension(
     const base::FilePath& file) {
   // The policy list is all ASCII, so a non-ASCII extension won't be in it.
-  const base::FilePath::StringType ext =
-      download_protection_util::GetFileExtension(file);
+  const base::FilePath::StringType ext = GetFileExtension(file);
   std::string ascii_ext =
       base::ToLowerASCII(base::FilePath(ext).MaybeAsASCII());
   if (ascii_ext[0] == '.')
@@ -122,15 +172,14 @@ std::string FileTypePolicies::CanonicalizedExtension(
   return ascii_ext;
 }
 
-const DownloadFileType& FileTypePolicies::PolicyForFile(
-    const base::FilePath& file) {
+const DownloadFileType& FileTypePolicies::PolicyForExtension(
+    const std::string& ascii_ext) {
+  lock_.AssertAcquired();
   // This could happen if the ResourceBundle is corrupted.
   if (!config_) {
     DCHECK(false);
     return last_resort_default_;
   }
-
-  std::string ascii_ext = CanonicalizedExtension(file);
   auto itr = file_type_by_ext_.find(ascii_ext);
   if (itr != file_type_by_ext_.end())
     return *itr->second;
@@ -138,18 +187,37 @@ const DownloadFileType& FileTypePolicies::PolicyForFile(
     return config_->default_file_type();
 }
 
-const DownloadFileType::PlatformSettings& FileTypePolicies::SettingsForFile(
+DownloadFileType FileTypePolicies::PolicyForFile(const base::FilePath& file) {
+  const std::string ext = CanonicalizedExtension(file);
+  AutoLock lock(lock_);
+  return PolicyForExtension(ext);
+}
+
+DownloadFileType::PlatformSettings FileTypePolicies::SettingsForFile(
     const base::FilePath& file) {
-  DCHECK_EQ(1, PolicyForFile(file).platform_settings().size());
-  return PolicyForFile(file).platform_settings(0);
+  const std::string ext = CanonicalizedExtension(file);
+  AutoLock lock(lock_);
+  DCHECK_EQ(1, PolicyForExtension(ext).platform_settings().size());
+  return PolicyForExtension(ext).platform_settings(0);
 }
 
 int64_t FileTypePolicies::UmaValueForFile(const base::FilePath& file) {
-  return PolicyForFile(file).uma_value();
+  const std::string ext = CanonicalizedExtension(file);
+  AutoLock lock(lock_);
+  return PolicyForExtension(ext).uma_value();
+}
+
+bool FileTypePolicies::IsArchiveFile(const base::FilePath& file) {
+  const std::string ext = CanonicalizedExtension(file);
+  AutoLock lock(lock_);
+  return PolicyForExtension(ext).is_archive();
 }
 
-bool FileTypePolicies::IsFileAnArchive(const base::FilePath& file) {
-  return PolicyForFile(file).is_archive();
+bool FileTypePolicies::IsCheckedBinaryFile(const base::FilePath& file) {
+  const std::string ext = CanonicalizedExtension(file);
+  AutoLock lock(lock_);
+  return PolicyForExtension(ext).ping_setting() ==
+         DownloadFileType::FULL_PING;
 }
 
 }  // namespace safe_browsing