[Extensions] Add a static factory to create VerifiedContents.

This CL combines VerifiedContents class instantiation and initializing
it from a file to a single static factory. Consumer of
VerifiedContents always requires both, so it's slightly better with
the factory.

Bug: None
Test: None, internal cleanup.
Change-Id: I8938433c7f95b93425c25f5fda0299d23cfa65e3
Reviewed-on: https://chromium-review.googlesource.com/1149411Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Commit-Queue: Istiaque Ahmed <lazyboy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578000}

extensions/browser/content_verifier/content_hash.cc

@@ -47,10 +47,11 @@ std::unique_ptr<VerifiedContents> GetVerifiedContents(
     bool delete_invalid_file) {
   base::AssertBlockingAllowed();
   DCHECK(GetExtensionFileTaskRunner()->RunsTasksInCurrentSequence());
-  auto verified_contents = std::make_unique<VerifiedContents>(key.verifier_key);
   base::FilePath verified_contents_path =
       file_util::GetVerifiedContentsPath(key.extension_root);
-  if (!verified_contents->InitFrom(verified_contents_path)) {
+  std::unique_ptr<VerifiedContents> verified_contents =
+      VerifiedContents::Create(key.verifier_key, verified_contents_path);
+  if (!verified_contents) {
     if (delete_invalid_file &&
         !base::DeleteFile(verified_contents_path, false)) {
       LOG(WARNING) << "Failed to delete " << verified_contents_path.value();

extensions/browser/verified_contents.cc

@@ -10,6 +10,7 @@
 #include "base/base64url.h"
 #include "base/files/file_util.h"
 #include "base/json/json_reader.h"
+#include "base/memory/ptr_util.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/strings/string_util.h"
 #include "base/timer/elapsed_timer.h"
@@ -142,39 +143,44 @@ VerifiedContents::~VerifiedContents() {
 //     }
 //   ]
 // }
-bool VerifiedContents::InitFrom(const base::FilePath& path) {
+// static.
+std::unique_ptr<VerifiedContents> VerifiedContents::Create(
+    base::span<const uint8_t> public_key,
+    const base::FilePath& path) {
   ScopedUMARecorder uma_recorder;
+  // Note: VerifiedContents constructor is private.
+  auto verified_contents = base::WrapUnique(new VerifiedContents(public_key));
   std::string payload;
-  if (!GetPayload(path, &payload))
-    return false;
+  if (!verified_contents->GetPayload(path, &payload))
+    return nullptr;
 
   std::unique_ptr<base::Value> value(base::JSONReader::Read(payload));
   if (!value.get() || !value->is_dict())
-    return false;
+    return nullptr;
   DictionaryValue* dictionary = static_cast<DictionaryValue*>(value.get());
 
   std::string item_id;
   if (!dictionary->GetString(kItemIdKey, &item_id) ||
       !crx_file::id_util::IdIsValid(item_id)) {
-    return false;
+    return nullptr;
   }
-  extension_id_ = item_id;
+  verified_contents->extension_id_ = item_id;
 
   std::string version_string;
   if (!dictionary->GetString(kItemVersionKey, &version_string))
-    return false;
-  version_ = base::Version(version_string);
-  if (!version_.IsValid())
-    return false;
+    return nullptr;
+  verified_contents->version_ = base::Version(version_string);
+  if (!verified_contents->version_.IsValid())
+    return nullptr;
 
-  ListValue* hashes_list = NULL;
+  ListValue* hashes_list = nullptr;
   if (!dictionary->GetList(kContentHashesKey, &hashes_list))
-    return false;
+    return nullptr;
 
   for (size_t i = 0; i < hashes_list->GetSize(); i++) {
-    DictionaryValue* hashes = NULL;
+    DictionaryValue* hashes = nullptr;
     if (!hashes_list->GetDictionary(i, &hashes))
-      return false;
+      return nullptr;
     std::string format;
     if (!hashes->GetString(kFormatKey, &format) || format != kTreeHash)
       continue;
@@ -183,23 +189,23 @@ bool VerifiedContents::InitFrom(const base::FilePath& path) {
     int hash_block_size = 0;
     if (!hashes->GetInteger(kBlockSizeKey, &block_size) ||
         !hashes->GetInteger(kHashBlockSizeKey, &hash_block_size)) {
-      return false;
+      return nullptr;
     }
-    block_size_ = block_size;
+    verified_contents->block_size_ = block_size;
 
     // We don't support using a different block_size and hash_block_size at
     // the moment.
-    if (block_size_ != hash_block_size)
-      return false;
+    if (verified_contents->block_size_ != hash_block_size)
+      return nullptr;
 
-    ListValue* files = NULL;
+    ListValue* files = nullptr;
     if (!hashes->GetList(kFilesKey, &files))
-      return false;
+      return nullptr;
 
-    for (size_t j = 0; j < files->GetSize(); j++) {
-      DictionaryValue* data = NULL;
+    for (size_t j = 0; j < files->GetSize(); ++j) {
+      DictionaryValue* data = nullptr;
       if (!files->GetDictionary(j, &data))
-        return false;
+        return nullptr;
       std::string file_path_string;
       std::string encoded_root_hash;
       std::string root_hash;
@@ -209,13 +215,13 @@ bool VerifiedContents::InitFrom(const base::FilePath& path) {
           !base::Base64UrlDecode(encoded_root_hash,
                                  base::Base64UrlDecodePolicy::IGNORE_PADDING,
                                  &root_hash)) {
-        return false;
+        return nullptr;
       }
       base::FilePath file_path =
           base::FilePath::FromUTF8Unsafe(file_path_string);
       base::FilePath::StringType lowercase_file_path =
           base::ToLowerASCII(file_path.value());
-      RootHashes::iterator i = root_hashes_.insert(
+      RootHashes::iterator i = verified_contents->root_hashes_.insert(
           std::make_pair(lowercase_file_path, std::string()));
       i->second.swap(root_hash);
 
@@ -225,14 +231,15 @@ bool VerifiedContents::InitFrom(const base::FilePath& path) {
       // HasTreeHashRoot() and TreeHashRootEquals().
       base::FilePath::StringType trimmed_path;
       if (TrimDotSpaceSuffix(lowercase_file_path, &trimmed_path))
-        root_hashes_.insert(std::make_pair(trimmed_path, i->second));
+        verified_contents->root_hashes_.insert(
+            std::make_pair(trimmed_path, i->second));
 #endif  // defined(OS_WIN)
     }
 
     break;
   }
   uma_recorder.RecordSuccess();
-  return true;
+  return verified_contents;
 }
 
 bool VerifiedContents::HasTreeHashRoot(

extensions/browser/verified_contents.h

@@ -24,13 +24,16 @@ namespace extensions {
 // corruption of extension files on local disk.
 class VerifiedContents {
  public:
-  // Note: the public_key must remain valid for the lifetime of this object.
-  explicit VerifiedContents(base::span<const uint8_t> public_key);
   ~VerifiedContents();
 
-  // Returns true if we successfully parsed the verified_contents.json file at
-  // |path| and validated the enclosed signature. The
-  bool InitFrom(const base::FilePath& path);
+  // Returns verified contents after successfully parsing verified_contents.json
+  // file at |path| and validating the enclosed signature. Returns nullptr on
+  // failure.
+  // Note: |public_key| must remain valid for the lifetime of the returned
+  // object.
+  static std::unique_ptr<VerifiedContents> Create(
+      base::span<const uint8_t> public_key,
+      const base::FilePath& path);
 
   int block_size() const { return block_size_; }
   const std::string& extension_id() const { return extension_id_; }
@@ -46,6 +49,9 @@ class VerifiedContents {
   bool valid_signature() { return valid_signature_; }
 
  private:
+  // Note: the public_key must remain valid for the lifetime of this object.
+  explicit VerifiedContents(base::span<const uint8_t> public_key);
+
   // Returns the base64url-decoded "payload" field from the json at |path|, if
   // the signature was valid.
   bool GetPayload(const base::FilePath& path, std::string* payload);

extensions/browser/verified_contents_unittest.cc

@@ -71,13 +71,10 @@ std::unique_ptr<VerifiedContents> CreateTestVerifiedContents(
   if (!GetPublicKey(path.AppendASCII(kPublicKeyPem), &public_key))
     return nullptr;
 
-  auto contents = std::make_unique<VerifiedContents>(
-      base::as_bytes(base::make_span(public_key)));
   base::FilePath verified_contents_path =
       path.AppendASCII(verified_contents_filename);
-  if (!contents->InitFrom(verified_contents_path))
-    return nullptr;
-  return contents;
+  return VerifiedContents::Create(base::as_bytes(base::make_span(public_key)),
+                                  verified_contents_path);
 }
 
 }  // namespace