precache: Allow experiment bitsets of any size.

components/precache/core/precache_fetcher.cc

 // Copyright 2013 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 "components/precache/core/precache_fetcher.h"
 
 #include <algorithm>
 #include <limits>
 #include <set>
 #include <utility>
 #include <vector>
 
 #include "base/base64.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback.h"
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
 #include "base/containers/hash_tables.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
 #include "base/memory/ref_counted.h"
 #include "base/metrics/histogram_macros.h"
+#include "base/optional.h"
 #include "base/sha1.h"
 #include "base/strings/string_piece.h"
 #include "base/task_runner_util.h"
 #include "components/data_use_measurement/core/data_use_user_data.h"
 #include "components/precache/core/precache_database.h"
 #include "components/precache/core/precache_switches.h"
 #include "components/precache/core/proto/quota.pb.h"
 #include "components/precache/core/proto/unfinished_work.pb.h"
 #include "net/base/completion_callback.h"
 #include "net/base/escape.h"
@@ skipping 83 matching lines @@
   }
   if (!message->ParseFromString(response_string)) {
     DLOG(WARNING) << "Unable to parse proto served from "
                   << source.GetOriginalURL().spec();
     return false;
   }
   return true;
 }
 
 // Returns the resource selection bitset from the |manifest| for the given
-// |experiment_id|. By default all resource will be selected if the experiment
-// group is not found.
-uint64_t GetResourceBitset(const PrecacheManifest& manifest,
-                           uint32_t experiment_id) {
+// |experiment_id|. If the experiment group is not found, then this returns
+// nullopt, in which case all resources should be selected.
+base::Optional<std::vector<bool>> GetResourceBitset(
+    const PrecacheManifest& manifest,
+    uint32_t experiment_id) {
+  base::Optional<std::vector<bool>> ret;
   if (manifest.has_experiments()) {
     const auto& resource_bitset_map =
         manifest.experiments().resources_by_experiment_group();
-    const auto& resource_bitset_it = resource_bitset_map.find(experiment_id);
-    if (resource_bitset_it != resource_bitset_map.end())
-      return resource_bitset_it->second.bitset();
+    const auto& it = resource_bitset_map.find(experiment_id);
+    if (it != resource_bitset_map.end()) {
+      if (it->second.has_bitset()) {
+        const std::string& bitset = it->second.bitset();
+        ret.emplace(bitset.size() * 8);
+        for (size_t i = 0; i < bitset.size(); ++i) {
+          for (size_t j = 0; j < 8; ++j) {
+            if ((1 << j) & bitset[i])
+              ret.value()[i * 8 + j] = true;
+          }
+        }
+      } else if (it->second.has_deprecated_bitset()) {
+        uint64_t bitset = it->second.deprecated_bitset();
+        ret.emplace(64);
+        for (int i = 0; i < 64; ++i) {
+          if ((0x1ULL << i) & bitset)
+            ret.value()[i] = true;
+        }
+      }
+    }
   }
-  return ~0ULL;
+  // Only one return point to ensure RVO triggers.

[jamartin, 2017/02/22 22:14:01]

(opt) Not needed. Besides, AFIK, it is not so much that you return in a single
point but that you only return the same single variable.

[twifkak, 2017/02/22 23:55:03]

Fixed the comment. (I want to head off at the pass any future temptation to
define a local vector<bool> and then return it, causing an implicit construction
of the Optional and therefore who knows what performance.)

+  return ret;
 }
 
 // URLFetcherResponseWriter that ignores the response body, in order to avoid
 // the unnecessary memory usage. Use it rather than the default if you don't
 // care about parsing the response body. We use it below as a means to populate
 // the cache with requested resource URLs.
 class URLFetcherNullWriter : public net::URLFetcherResponseWriter {
  public:
   int Initialize(const net::CompletionCallback& callback) override {
     return net::OK;
@@ skipping 589 matching lines @@
   UpdateStats(source.response_bytes(), source.network_response_bytes());
   if (source.network_url_fetcher() == nullptr) {
     pool_.DeleteAll();  // Cancel any other ongoing request.
   } else {
     PrecacheManifest manifest;
 
     if (ParseProtoFromFetchResponse(*source.network_url_fetcher(), &manifest)) {
       const int32_t len =
           std::min(manifest.resource_size(),
                    unfinished_work_->config_settings().top_resources_count());
-      const uint64_t resource_bitset =
+      const base::Optional<std::vector<bool>> resource_bitset =
           GetResourceBitset(manifest, experiment_id_);
       for (int i = 0; i < len; ++i) {
-        if (((0x1ULL << i) & resource_bitset) &&
+        if ((!resource_bitset.has_value() || resource_bitset.value()[i]) &&
             manifest.resource(i).has_url()) {
           GURL url(manifest.resource(i).url());
           if (url.is_valid()) {
             double weight = ResourceWeight(
                 unfinished_work_->config_settings().resource_weight_function(),
                 manifest.resource(i).weight_ratio(), host_visits);
             if (weight >= unfinished_work_->config_settings().min_weight())
               resources_to_rank_.emplace_back(url, source.referrer(), weight);
           }
         }
@@ skipping 80 matching lines @@
       remaining = 0;
     quota_.set_remaining(
         used_bytes > quota_.remaining() ? 0U : quota_.remaining() - used_bytes);
     db_task_runner_->PostTask(
         FROM_HERE,
         base::Bind(&PrecacheDatabase::SaveQuota, precache_database_, quota_));
   }
 }
 
 }  // namespace precache