Supervised user whitelists optimization: Store raw sha1 hashes rather than strings

chrome/browser/supervised_user/supervised_user_site_list.cc

 // Copyright 2014 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/supervised_user/supervised_user_site_list.h"
 
+#include <algorithm>
+
 #include "base/files/file_util.h"
 #include "base/json/json_file_value_serializer.h"
 #include "base/logging.h"
 #include "base/metrics/histogram_macros.h"
-#include "base/sha1.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/task_runner_util.h"
 #include "base/values.h"
 #include "content/public/browser/browser_thread.h"
 #include "url/gurl.h"
 
 const int kSitelistFormatVersion = 2;
 
 const char kEntryPointUrlKey[] = "entry_point_url";
 const char kHostnameHashesKey[] = "hostname_hashes";
 const char kSitelistFormatVersionKey[] = "version";
@@ skipping 10 matching lines @@
       deserializer.Deserialize(&error_code, &error_msg);
   if (!value) {
     LOG(ERROR) << "Couldn't load site list " << path.value() << ": "
                << error_msg;
   }
   return value.Pass();
 }
 
 }  // namespace
 
+SupervisedUserSiteList::HostnameHash::HostnameHash(
+    const std::string& hostname) {
+  base::SHA1HashBytes(reinterpret_cast<const unsigned char*>(hostname.c_str()),
+                      hostname.size(), bytes_.data());
+}
+
+SupervisedUserSiteList::HostnameHash::HostnameHash(
+    const std::vector<uint8>& bytes) {
+  DCHECK_GE(bytes.size(), base::kSHA1Length);

[Bernhard Bauer, 2015/12/03 11:43:21]

Would you mind making this a CHECK just to be a bit safer about accessing memory
out of bounds in a release build?

[Marc Treib, 2015/12/03 12:25:11]

Hm... sure, I guess. Done.

+  std::copy_n(bytes.begin(), base::kSHA1Length, bytes_.begin());

[Bernhard Bauer, 2015/12/03 11:43:21]

Actually, is copy_n already allowed? I can't find it (positive or negative) on
https://chromium-cpp.appspot.com/.

Or maybe just use std::copy with begin() and end()?

[Marc Treib, 2015/12/03 12:25:11]

Hm, <algorithms> is still under "to be discussed". TBH I didn't notice that
copy_n was only added in C++11.
Anyway, regular copy will do just as well.

+}
+
+bool SupervisedUserSiteList::HostnameHash::operator==(
+    const HostnameHash& rhs) const {
+  return bytes_ == rhs.bytes_;
+}
+
+size_t SupervisedUserSiteList::HostnameHash::hash() const {
+  // This just returns the first sizeof(size_t) bytes of |bytes_|.
+  return *reinterpret_cast<const size_t*>(bytes_.data());
+}
+
 void SupervisedUserSiteList::Load(const base::string16& title,
                                   const base::FilePath& path,
                                   const LoadedCallback& callback) {
   base::PostTaskAndReplyWithResult(
       content::BrowserThread::GetBlockingPool()
           ->GetTaskRunnerWithShutdownBehavior(
               base::SequencedWorkerPool::CONTINUE_ON_SHUTDOWN)
           .get(),
       FROM_HERE, base::Bind(&ReadFileOnBlockingThread, path),
       base::Bind(&SupervisedUserSiteList::OnJsonLoaded, title, path,
@@ skipping 13 matching lines @@
         LOG(ERROR) << "Invalid whitelist entry";
         continue;
       }
 
       patterns_.push_back(pattern);
     }
   }
 
   if (hostname_hashes) {
     for (const base::Value* entry : *hostname_hashes) {
-      // |hash| should be a hex-encoded SHA1 hash.
-      std::string hash;
-      if (!entry->GetAsString(&hash) || hash.size() != 2 * base::kSHA1Length) {
+      // |hash_str| should be a hex-encoded SHA1 hash string.
+      std::string hash_str;
+      std::vector<uint8> hash_bytes;
+      if (!entry->GetAsString(&hash_str) ||
+          hash_str.size() != 2 * base::kSHA1Length ||
+          !base::HexStringToBytes(hash_str, &hash_bytes)) {
         LOG(ERROR) << "Invalid hostname_hashes entry";
         continue;
       }
-      // TODO(treib): Check that |hash| has only characters from [0-9a-fA-F].
-      // Or just store the raw bytes (from base::HexStringToBytes).
-      hostname_hashes_.push_back(hash);
+      DCHECK(hash_bytes.size() == base::kSHA1Length);

[Bernhard Bauer, 2015/12/03 11:43:21]

DCHECK_EQ?

[Marc Treib, 2015/12/03 12:25:11]

Done.

+      hostname_hashes_.push_back(HostnameHash(hash_bytes));
     }
   }
 
   if (patterns_.empty() && hostname_hashes_.empty())
     LOG(WARNING) << "Site list is empty!";
 }
 
 SupervisedUserSiteList::~SupervisedUserSiteList() {
 }
 
@@ skipping 34 matching lines @@
 
   base::ListValue* patterns = nullptr;
   dict->GetList(kWhitelistKey, &patterns);
 
   base::ListValue* hostname_hashes = nullptr;
   dict->GetList(kHostnameHashesKey, &hostname_hashes);
 
   callback.Run(make_scoped_refptr(new SupervisedUserSiteList(
       title, GURL(entry_point_url), patterns, hostname_hashes)));
 }