Deleted ExtensionThrottleEntryInterface.

ExtensionThrottleEntryInterface was never strictly necessary.
Now that ExtensionThrottleManager does not expose the
ExtensionThrottleEntry to any caller (excluding tests) it
no longer needs to be a reference counted object.

Also, removed the back reference to the ExtensionThrottleManager
in the ExtensionThrottleEntry as that was not necessary.

Bug: none
Change-Id: I5188f7a024304b9159879a9d0763a075c8bfba3c
Reviewed-on: https://chromium-review.googlesource.com/1132353
Commit-Queue: Chris Mumford <cmumford@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577268}

extensions/renderer/BUILD.gn

@@ -113,7 +113,6 @@ jumbo_source_set("renderer") {
     "extension_port.h",
     "extension_throttle_entry.cc",
     "extension_throttle_entry.h",
-    "extension_throttle_entry_interface.h",
     "extension_throttle_manager.cc",
     "extension_throttle_manager.h",
     "extension_url_loader_throttle.cc",

extensions/renderer/extension_throttle_entry.cc

@@ -13,7 +13,6 @@
 #include "base/rand_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/values.h"
-#include "extensions/renderer/extension_throttle_manager.h"
 #include "net/base/load_flags.h"
 
 namespace extensions {
@@ -44,13 +43,10 @@ const double ExtensionThrottleEntry::kDefaultJitterFactor = 0.4;
 const int ExtensionThrottleEntry::kDefaultMaximumBackoffMs = 15 * 60 * 1000;
 const int ExtensionThrottleEntry::kDefaultEntryLifetimeMs = 2 * 60 * 1000;
 
-ExtensionThrottleEntry::ExtensionThrottleEntry(
-    ExtensionThrottleManager* manager,
-    const std::string& url_id)
-    : ExtensionThrottleEntry(manager, url_id, false) {}
+ExtensionThrottleEntry::ExtensionThrottleEntry(const std::string& url_id)
+    : ExtensionThrottleEntry(url_id, false) {}
 
 ExtensionThrottleEntry::ExtensionThrottleEntry(
-    ExtensionThrottleManager* manager,
     const std::string& url_id,
     bool ignore_user_gesture_load_flag_for_tests)
     : sliding_window_period_(
@@ -58,16 +54,13 @@ ExtensionThrottleEntry::ExtensionThrottleEntry(
       max_send_threshold_(kDefaultMaxSendThreshold),
       is_backoff_disabled_(false),
       backoff_entry_(&backoff_policy_),
-      manager_(manager),
       url_id_(url_id),
       ignore_user_gesture_load_flag_for_tests_(
           ignore_user_gesture_load_flag_for_tests) {
-  DCHECK(manager_);
   Initialize();
 }
 
 ExtensionThrottleEntry::ExtensionThrottleEntry(
-    ExtensionThrottleManager* manager,
     const std::string& url_id,
     const net::BackoffEntry::Policy* backoff_policy,
     bool ignore_user_gesture_load_flag_for_tests)
@@ -76,7 +69,6 @@ ExtensionThrottleEntry::ExtensionThrottleEntry(
       max_send_threshold_(kDefaultMaxSendThreshold),
       is_backoff_disabled_(false),
       backoff_entry_(&backoff_policy_),
-      manager_(manager),
       url_id_(url_id),
       ignore_user_gesture_load_flag_for_tests_(
           ignore_user_gesture_load_flag_for_tests) {
@@ -85,29 +77,12 @@ ExtensionThrottleEntry::ExtensionThrottleEntry(
   DCHECK_GE(backoff_policy->jitter_factor, 0.0);
   DCHECK_LT(backoff_policy->jitter_factor, 1.0);
   DCHECK_GE(backoff_policy->maximum_backoff_ms, 0);
-  DCHECK(manager_);
 
   Initialize();
   backoff_policy_ = *backoff_policy;
 }
 
 bool ExtensionThrottleEntry::IsEntryOutdated() const {
-  // This function is called by the ExtensionThrottleManager to determine
-  // whether entries should be discarded from its url_entries_ map.  We
-  // want to ensure that it does not remove entries from the map while there
-  // are clients (objects other than the manager) holding references to
-  // the entry, otherwise separate clients could end up holding separate
-  // entries for a request to the same URL, which is undesirable.  Therefore,
-  // if an entry has more than one reference (the map will always hold one),
-  // it should not be considered outdated.
-  //
-  // We considered whether to make ExtensionThrottleEntry objects
-  // non-refcounted, but since any means of knowing whether they are
-  // currently in use by others than the manager would be more or less
-  // equivalent to a refcount, we kept them refcounted.
-  if (!HasOneRef())
-    return false;
-
   // If there are send events in the sliding window period, we still need this
   // entry.
   if (!send_log_.empty() &&
@@ -122,10 +97,6 @@ void ExtensionThrottleEntry::DisableBackoffThrottling() {
   is_backoff_disabled_ = true;
 }
 
-void ExtensionThrottleEntry::DetachManager() {
-  manager_ = NULL;
-}
-
 bool ExtensionThrottleEntry::ShouldRejectRequest(int request_load_flags) const {
   bool reject_request = false;
   if (!is_backoff_disabled_ &&

extensions/renderer/extension_throttle_entry.h

@@ -12,13 +12,10 @@
 #include "base/containers/queue.h"
 #include "base/macros.h"
 #include "base/time/time.h"
-#include "extensions/renderer/extension_throttle_entry_interface.h"
 #include "net/base/backoff_entry.h"
 
 namespace extensions {
 
-class ExtensionThrottleManager;
-
 // ExtensionThrottleEntry represents an entry of ExtensionThrottleManager.
 // It analyzes requests of a specific URL over some period of time, in order to
 // deduce the back-off time for every request.
@@ -30,7 +27,7 @@ class ExtensionThrottleManager;
 // destination and provide guidance (to the application level only) on whether
 // too many requests have been sent and when a good time to send the next one
 // would be. This is never used to deny requests at the network level.
-class ExtensionThrottleEntry : public ExtensionThrottleEntryInterface {
+class ExtensionThrottleEntry {
  public:
   // Sliding window period.
   static const int kDefaultSlidingWindowPeriodMs;
@@ -57,24 +54,23 @@ class ExtensionThrottleEntry : public ExtensionThrottleEntryInterface {
   // Time after which the entry is considered outdated.
   static const int kDefaultEntryLifetimeMs;
 
-  // The manager object's lifetime must enclose the lifetime of this object.
-  ExtensionThrottleEntry(ExtensionThrottleManager* manager,
-                         const std::string& url_id);
+  // |url_id| is a unique entry ID.
+  explicit ExtensionThrottleEntry(const std::string& url_id);
 
   // Same as above, but exposes the option to ignore
   // net::LOAD_MAYBE_USER_GESTURE flag of the request.
-  ExtensionThrottleEntry(ExtensionThrottleManager* manager,
-                         const std::string& url_id,
+  ExtensionThrottleEntry(const std::string& url_id,
                          bool ignore_user_gesture_load_flag_for_tests);
 
   // The life span of instances created with this constructor is set to
   // infinite, and the number of initial errors to ignore is set to 0.
   // It is only used by unit tests.
-  ExtensionThrottleEntry(ExtensionThrottleManager* manager,
-                         const std::string& url_id,
+  ExtensionThrottleEntry(const std::string& url_id,
                          const net::BackoffEntry::Policy* backoff_policy,
                          bool ignore_user_gesture_load_flag_for_tests);
 
+  virtual ~ExtensionThrottleEntry();
+
   // Used by the manager, returns true if the entry needs to be garbage
   // collected.
   bool IsEntryOutdated() const;
@@ -82,21 +78,47 @@ class ExtensionThrottleEntry : public ExtensionThrottleEntryInterface {
   // Causes this entry to never reject requests due to back-off.
   void DisableBackoffThrottling();
 
-  // Causes this entry to NULL its manager pointer.
-  void DetachManager();
-
-  // Implementation of ExtensionThrottleEntryInterface.
-  bool ShouldRejectRequest(int request_load_flags) const override;
+  // Returns true when we have encountered server errors and are doing
+  // exponential back-off, unless |request_load_flags| indicates the
+  // request is likely to be user-initiated, or the NetworkDelegate returns
+  // false for |CanThrottleRequest(request)|.
+  //
+  // URLRequestHttpJob checks this method prior to every request; it
+  // cancels requests if this method returns true.
+  //
+  // Note: See load_flags.h for more detail on |request_load_flags|.
+  bool ShouldRejectRequest(int request_load_flags) const;
+
+  // Calculates a recommended sending time for the next request and reserves it.
+  // The sending time is not earlier than the current exponential back-off
+  // release time or |earliest_time|. Moreover, the previous results of
+  // the method are taken into account, in order to make sure they are spread
+  // properly over time.
+  // Returns the recommended delay before sending the next request, in
+  // milliseconds. The return value is always positive or 0.
+  // Although it is not mandatory, respecting the value returned by this method
+  // is helpful to avoid traffic overload.
   int64_t ReserveSendingTimeForNextRequest(
-      const base::TimeTicks& earliest_time) override;
-  base::TimeTicks GetExponentialBackoffReleaseTime() const override;
-  void UpdateWithResponse(int status_code) override;
-  void ReceivedContentWasMalformed(int response_code) override;
-  const std::string& GetURLIdForDebugging() const override;
+      const base::TimeTicks& earliest_time);
 
- protected:
-  ~ExtensionThrottleEntry() override;
+  // Returns the time after which requests are allowed.
+  base::TimeTicks GetExponentialBackoffReleaseTime() const;
+
+  // This method needs to be called each time a response is received.
+  void UpdateWithResponse(int status_code);
 
+  // Lets higher-level modules, that know how to parse particular response
+  // bodies, notify of receiving malformed content for the given URL. This will
+  // be handled by the throttler as if an HTTP 503 response had been received to
+  // the request, i.e. it will count as a failure, unless the HTTP response code
+  // indicated is already one of those that will be counted as an error.
+  void ReceivedContentWasMalformed(int response_code);
+
+  // Get the URL ID associated with this entry. Should only be used for
+  // debugging purpose.
+  const std::string& GetURLIdForDebugging() const;
+
+ protected:
   void Initialize();
 
   // Returns true if the given response code is considered a success for
@@ -148,11 +170,8 @@ class ExtensionThrottleEntry : public ExtensionThrottleEntryInterface {
   // Access it through GetBackoffEntry() to allow a unit test seam.
   net::BackoffEntry backoff_entry_;
 
-  // Weak back-reference to the manager object managing us.
-  ExtensionThrottleManager* manager_;
-
   // Canonicalized URL string that this entry is for; used for logging only.
-  std::string url_id_;
+  const std::string url_id_;
 
   bool ignore_user_gesture_load_flag_for_tests_;
 

extensions/renderer/extension_throttle_entry_interface.h

-// 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.
-
-#ifndef EXTENSIONS_RENDERER_EXTENSION_THROTTLE_ENTRY_INTERFACE_H_
-#define EXTENSIONS_RENDERER_EXTENSION_THROTTLE_ENTRY_INTERFACE_H_
-
-#include <stdint.h>
-
-#include <string>
-
-#include "base/macros.h"
-#include "base/memory/ref_counted.h"
-#include "base/time/time.h"
-
-namespace extensions {
-
-// Interface provided on entries of the URL request throttler manager.
-class ExtensionThrottleEntryInterface
-    : public base::RefCountedThreadSafe<ExtensionThrottleEntryInterface> {
- public:
-  ExtensionThrottleEntryInterface() {}
-
-  // Returns true when we have encountered server errors and are doing
-  // exponential back-off, unless |request_load_flags| indicates the
-  // request is likely to be user-initiated, or the NetworkDelegate returns
-  // false for |CanThrottleRequest(request)|.
-  //
-  // URLRequestHttpJob checks this method prior to every request; it
-  // cancels requests if this method returns true.
-  //
-  // Note: See load_flags.h for more detail on |request_load_flags|.
-  virtual bool ShouldRejectRequest(int request_load_flags) const = 0;
-
-  // Calculates a recommended sending time for the next request and reserves it.
-  // The sending time is not earlier than the current exponential back-off
-  // release time or |earliest_time|. Moreover, the previous results of
-  // the method are taken into account, in order to make sure they are spread
-  // properly over time.
-  // Returns the recommended delay before sending the next request, in
-  // milliseconds. The return value is always positive or 0.
-  // Although it is not mandatory, respecting the value returned by this method
-  // is helpful to avoid traffic overload.
-  virtual int64_t ReserveSendingTimeForNextRequest(
-      const base::TimeTicks& earliest_time) = 0;
-
-  // Returns the time after which requests are allowed.
-  virtual base::TimeTicks GetExponentialBackoffReleaseTime() const = 0;
-
-  // This method needs to be called each time a response is received.
-  virtual void UpdateWithResponse(int status_code) = 0;
-
-  // Lets higher-level modules, that know how to parse particular response
-  // bodies, notify of receiving malformed content for the given URL. This will
-  // be handled by the throttler as if an HTTP 503 response had been received to
-  // the request, i.e. it will count as a failure, unless the HTTP response code
-  // indicated is already one of those that will be counted as an error.
-  virtual void ReceivedContentWasMalformed(int response_code) = 0;
-
-  // Get the URL ID associated with this entry. Should only be used for
-  // debugging purpose.
-  virtual const std::string& GetURLIdForDebugging() const = 0;
-
- protected:
-  friend class base::RefCountedThreadSafe<ExtensionThrottleEntryInterface>;
-  virtual ~ExtensionThrottleEntryInterface() {}
-
- private:
-  friend class base::RefCounted<ExtensionThrottleEntryInterface>;
-  DISALLOW_COPY_AND_ASSIGN(ExtensionThrottleEntryInterface);
-};
-
-}  // namespace extensions
-
-#endif  // EXTENSIONS_RENDERER_EXTENSION_THROTTLE_ENTRY_INTERFACE_H_

extensions/renderer/extension_throttle_manager.cc

@@ -32,16 +32,6 @@ ExtensionThrottleManager::ExtensionThrottleManager()
 
 ExtensionThrottleManager::~ExtensionThrottleManager() {
   base::AutoLock auto_lock(lock_);
-  // Since the manager object might conceivably go away before the
-  // entries, detach the entries' back-pointer to the manager.
-  UrlEntryMap::iterator i = url_entries_.begin();
-  while (i != url_entries_.end()) {
-    if (i->second.get() != NULL) {
-      i->second->DetachManager();
-    }
-    ++i;
-  }
-
   // Delete all entries.
   url_entries_.clear();
 }
@@ -54,8 +44,8 @@ ExtensionThrottleManager::MaybeCreateURLLoaderThrottle(
   return std::make_unique<ExtensionURLLoaderThrottle>(this);
 }
 
-scoped_refptr<ExtensionThrottleEntryInterface>
-ExtensionThrottleManager::RegisterRequestUrl(const GURL& url) {
+ExtensionThrottleEntry* ExtensionThrottleManager::RegisterRequestUrl(
+    const GURL& url) {
   // Internal function, no locking.
 
   // Normalize the url.
@@ -64,26 +54,25 @@ ExtensionThrottleManager::RegisterRequestUrl(const GURL& url) {
   // Periodically garbage collect old entries.
   GarbageCollectEntriesIfNecessary();
 
-  // Find the entry in the map or create a new NULL entry.
-  scoped_refptr<ExtensionThrottleEntry>& entry = url_entries_[url_id];
+  // Find the entry in the map or create a new null entry.
+  std::unique_ptr<ExtensionThrottleEntry>& entry = url_entries_[url_id];
 
   // If the entry exists but could be garbage collected at this point, we
   // start with a fresh entry so that we possibly back off a bit less
   // aggressively (i.e. this resets the error count when the entry's URL
   // hasn't been requested in long enough).
-  if (entry.get() && entry->IsEntryOutdated()) {
-    entry = NULL;
-  }
+  if (entry && entry->IsEntryOutdated())
+    entry.reset();
 
   // Create the entry if needed.
-  if (entry.get() == NULL) {
+  if (!entry) {
     if (backoff_policy_for_tests_) {
-      entry = new ExtensionThrottleEntry(
-          this, url_id, backoff_policy_for_tests_.get(),
-          ignore_user_gesture_load_flag_for_tests_);
+      entry.reset(
+          new ExtensionThrottleEntry(url_id, backoff_policy_for_tests_.get(),
+                                     ignore_user_gesture_load_flag_for_tests_));
     } else {
-      entry = new ExtensionThrottleEntry(
-          this, url_id, ignore_user_gesture_load_flag_for_tests_);
+      entry.reset(new ExtensionThrottleEntry(
+          url_id, ignore_user_gesture_load_flag_for_tests_));
     }
 
     // We only disable back-off throttling on an entry that we have
@@ -92,12 +81,12 @@ ExtensionThrottleManager::RegisterRequestUrl(const GURL& url) {
     if (net::IsLocalhost(url)) {
       // TODO(joi): Once sliding window is separate from back-off throttling,
       // we can simply return a dummy implementation of
-      // ExtensionThrottleEntryInterface here that never blocks anything.
+      // ExtensionThrottleEntry here that never blocks anything.
       entry->DisableBackoffThrottling();
     }
   }
 
-  return entry;
+  return entry.get();
 }
 
 bool ExtensionThrottleManager::ShouldRejectRequest(const GURL& request_url,
@@ -114,7 +103,7 @@ bool ExtensionThrottleManager::ShouldRejectRedirect(
   {
     base::AutoLock auto_lock(lock_);
     const std::string url_id = GetIdFromUrl(request_url);
-    scoped_refptr<ExtensionThrottleEntry>& entry = url_entries_[url_id];
+    ExtensionThrottleEntry* entry = url_entries_[url_id].get();
     DCHECK(entry);
     entry->UpdateWithResponse(redirect_info.status_code);
   }
@@ -127,7 +116,7 @@ void ExtensionThrottleManager::WillProcessResponse(
   if (response_head.network_accessed) {
     base::AutoLock auto_lock(lock_);
     const std::string url_id = GetIdFromUrl(response_url);
-    scoped_refptr<ExtensionThrottleEntry>& entry = url_entries_[url_id];
+    ExtensionThrottleEntry* entry = url_entries_[url_id].get();
     DCHECK(entry);
     entry->UpdateWithResponse(response_head.headers->response_code());
   }
@@ -141,7 +130,7 @@ void ExtensionThrottleManager::SetBackoffPolicyForTests(
 
 void ExtensionThrottleManager::OverrideEntryForTests(
     const GURL& url,
-    ExtensionThrottleEntry* entry) {
+    std::unique_ptr<ExtensionThrottleEntry> entry) {
   base::AutoLock auto_lock(lock_);
   // Normalize the url.
   std::string url_id = GetIdFromUrl(url);
@@ -149,7 +138,7 @@ void ExtensionThrottleManager::OverrideEntryForTests(
   // Periodically garbage collect old entries.
   GarbageCollectEntriesIfNecessary();
 
-  url_entries_[url_id] = entry;
+  url_entries_[url_id] = std::move(entry);
 }
 
 void ExtensionThrottleManager::EraseEntryForTests(const GURL& url) {
@@ -200,14 +189,9 @@ void ExtensionThrottleManager::GarbageCollectEntriesIfNecessary() {
 }
 
 void ExtensionThrottleManager::GarbageCollectEntries() {
-  UrlEntryMap::iterator i = url_entries_.begin();
-  while (i != url_entries_.end()) {
-    if ((i->second)->IsEntryOutdated()) {
-      url_entries_.erase(i++);
-    } else {
-      ++i;
-    }
-  }
+  base::EraseIf(url_entries_, [](const auto& entry) {
+    return entry.second->IsEntryOutdated();
+  });
 
   // In case something broke we want to make sure not to grow indefinitely.
   while (url_entries_.size() > kMaximumNumberOfEntries) {

extensions/renderer/extension_throttle_manager.h

@@ -76,7 +76,8 @@ class ExtensionThrottleManager {
   // Registers a new entry in this service and overrides the existing entry (if
   // any) for the URL. The service will hold a reference to the entry.
   // It is only used by unit tests.
-  void OverrideEntryForTests(const GURL& url, ExtensionThrottleEntry* entry);
+  void OverrideEntryForTests(const GURL& url,
+                             std::unique_ptr<ExtensionThrottleEntry> entry);
 
   // Sets whether to ignore net::LOAD_MAYBE_USER_GESTURE of the request for
   // testing. Otherwise, requests will not be throttled when they may have been
@@ -95,17 +96,10 @@ class ExtensionThrottleManager {
   // transformation.
   std::string GetIdFromUrl(const GURL& url) const;
 
-  // TODO(xunjieli): Remove this method and replace with
-  // ShouldRejectRequest(request) and UpdateWithResponse(request, status_code),
-  // which will also allow ExtensionThrottleEntry to no longer be reference
-  // counted, and ExtensionThrottleEntryInterface to be removed.
-
   // Must be called for every request, returns the URL request throttler entry
   // associated with the URL. The caller must inform this entry of some events.
-  // Please refer to extension_throttle_entry_interface.h for further
-  // informations.
-  scoped_refptr<ExtensionThrottleEntryInterface> RegisterRequestUrl(
-      const GURL& url);
+  // Please refer to extension_throttle_entry.h for further information.
+  ExtensionThrottleEntry* RegisterRequestUrl(const GURL& url);
 
   // Method that does the actual work of garbage collecting.
   void GarbageCollectEntries();
@@ -126,19 +120,14 @@ class ExtensionThrottleManager {
   // kRequestBetweenCollecting constant.
   void GarbageCollectEntriesIfNecessary();
 
-  // From each URL we generate an ID composed of the scheme, host, port and path
-  // that allows us to uniquely map an entry to it.
-  typedef std::map<std::string, scoped_refptr<ExtensionThrottleEntry>>
-      UrlEntryMap;
-
   // Maximum number of entries that we are willing to collect in our map.
   static const unsigned int kMaximumNumberOfEntries;
   // Number of requests that will be made between garbage collection.
   static const unsigned int kRequestsBetweenCollecting;
 
-  // Map that contains a list of URL ID and their matching
-  // ExtensionThrottleEntry.
-  UrlEntryMap url_entries_;
+  // Map that contains a list of URL ID (composed of the scheme, host, port and
+  // path) and their matching ExtensionThrottleEntry.
+  std::map<std::string, std::unique_ptr<ExtensionThrottleEntry>> url_entries_;
 
   // This keeps track of how many requests have been made. Used with
   // GarbageCollectEntries.
@@ -149,7 +138,7 @@ class ExtensionThrottleManager {
 
   bool ignore_user_gesture_load_flag_for_tests_;
 
-  // This is NULL when it is not set for tests.
+  // This is null when it is not set for tests.
   std::unique_ptr<net::BackoffEntry::Policy> backoff_policy_for_tests_;
 
   // Used to synchronize all public methods.

extensions/renderer/extension_throttle_simulation_unittest.cc

@@ -25,7 +25,7 @@
 #include "base/message_loop/message_loop.h"
 #include "base/rand_util.h"
 #include "base/time/time.h"
-#include "extensions/renderer/extension_throttle_manager.h"
+#include "extensions/renderer/extension_throttle_entry.h"
 #include "extensions/renderer/extension_throttle_test_support.h"
 #include "net/base/load_flags.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -287,10 +287,12 @@ class Server : public DiscreteTimeSimulation::Actor {
 // Mock throttler entry used by Requester class.
 class MockExtensionThrottleEntry : public ExtensionThrottleEntry {
  public:
-  explicit MockExtensionThrottleEntry(ExtensionThrottleManager* manager)
-      : ExtensionThrottleEntry(manager, std::string()),
+  MockExtensionThrottleEntry()
+      : ExtensionThrottleEntry(std::string()),
         backoff_entry_(&backoff_policy_, &fake_clock_) {}
 
+  ~MockExtensionThrottleEntry() override {}
+
   const BackoffEntry* GetBackoffEntry() const override {
     return &backoff_entry_;
   }
@@ -303,9 +305,6 @@ class MockExtensionThrottleEntry : public ExtensionThrottleEntry {
     fake_clock_.set_now(fake_time);
   }
 
- protected:
-  ~MockExtensionThrottleEntry() override {}
-
  private:
   mutable TestTickClock fake_clock_;
   BackoffEntry backoff_entry_;
@@ -377,11 +376,11 @@ class RequesterResults {
 // requests a specific resource.
 class Requester : public DiscreteTimeSimulation::Actor {
  public:
-  Requester(MockExtensionThrottleEntry* throttler_entry,
+  Requester(std::unique_ptr<MockExtensionThrottleEntry> throttler_entry,
             const TimeDelta& time_between_requests,
             Server* server,
             RequesterResults* results)
-      : throttler_entry_(throttler_entry),
+      : throttler_entry_(std::move(throttler_entry)),
         time_between_requests_(time_between_requests),
         last_attempt_was_failure_(false),
         server_(server),
@@ -454,7 +453,7 @@ class Requester : public DiscreteTimeSimulation::Actor {
   TimeDelta last_downtime_duration() const { return last_downtime_duration_; }
 
  private:
-  scoped_refptr<MockExtensionThrottleEntry> throttler_entry_;
+  std::unique_ptr<MockExtensionThrottleEntry> throttler_entry_;
   const TimeDelta time_between_requests_;
   TimeDelta request_jitter_;
   TimeTicks time_of_last_attempt_;
@@ -474,33 +473,30 @@ void SimulateAttack(Server* server,
   const size_t kNumAttackers = 50;
   const size_t kNumClients = 50;
   DiscreteTimeSimulation simulation;
-  ExtensionThrottleManager manager;
   std::vector<std::unique_ptr<Requester>> requesters;
   for (size_t i = 0; i < kNumAttackers; ++i) {
     // Use a tiny time_between_requests so the attackers will ping the
     // server at every tick of the simulation.
-    scoped_refptr<MockExtensionThrottleEntry> throttler_entry(
-        new MockExtensionThrottleEntry(&manager));
+    auto throttler_entry = std::make_unique<MockExtensionThrottleEntry>();
     if (!enable_throttling)
       throttler_entry->DisableBackoffThrottling();
 
     Requester* attacker =
-        new Requester(throttler_entry.get(), TimeDelta::FromMilliseconds(1),
-                      server, attacker_results);
+        new Requester(std::move(throttler_entry),
+                      TimeDelta::FromMilliseconds(1), server, attacker_results);
     attacker->SetStartupJitter(TimeDelta::FromSeconds(120));
     requesters.push_back(base::WrapUnique(attacker));
     simulation.AddActor(attacker);
   }
   for (size_t i = 0; i < kNumClients; ++i) {
     // Normal clients only make requests every 2 minutes, plus/minus 1 minute.
-    scoped_refptr<MockExtensionThrottleEntry> throttler_entry(
-        new MockExtensionThrottleEntry(&manager));
+    auto throttler_entry = std::make_unique<MockExtensionThrottleEntry>();
     if (!enable_throttling)
       throttler_entry->DisableBackoffThrottling();
 
     Requester* client =
-        new Requester(throttler_entry.get(), TimeDelta::FromMinutes(2), server,
-                      client_results);
+        new Requester(std::move(throttler_entry), TimeDelta::FromMinutes(2),
+                      server, client_results);
     client->SetStartupJitter(TimeDelta::FromSeconds(120));
     client->SetRequestJitter(TimeDelta::FromMinutes(1));
     requesters.push_back(base::WrapUnique(client));
@@ -572,14 +568,12 @@ double SimulateDowntime(const TimeDelta& duration,
   Server server(std::numeric_limits<int>::max(), 1.0);
   server.SetDowntime(start_downtime, duration);
 
-  ExtensionThrottleManager manager;
-  scoped_refptr<MockExtensionThrottleEntry> throttler_entry(
-      new MockExtensionThrottleEntry(&manager));
+  auto throttler_entry = std::make_unique<MockExtensionThrottleEntry>();
   if (!enable_throttling)
     throttler_entry->DisableBackoffThrottling();
 
-  Requester requester(throttler_entry.get(), average_client_interval, &server,
-                      NULL);
+  Requester requester(std::move(throttler_entry), average_client_interval,
+                      &server, NULL);
   requester.SetStartupJitter(duration / 3);
   requester.SetRequestJitter(average_client_interval);
 

extensions/renderer/extension_throttle_unittest.cc

@@ -24,16 +24,15 @@ namespace {
 
 class MockExtensionThrottleEntry : public ExtensionThrottleEntry {
  public:
-  explicit MockExtensionThrottleEntry(ExtensionThrottleManager* manager)
-      : ExtensionThrottleEntry(manager, std::string()),
+  MockExtensionThrottleEntry()
+      : ExtensionThrottleEntry(std::string()),
         backoff_entry_(&backoff_policy_, &fake_clock_) {
     InitPolicy();
   }
-  MockExtensionThrottleEntry(ExtensionThrottleManager* manager,
-                             const TimeTicks& exponential_backoff_release_time,
+  MockExtensionThrottleEntry(const TimeTicks& exponential_backoff_release_time,
                              const TimeTicks& sliding_window_release_time,
                              const TimeTicks& fake_now)
-      : ExtensionThrottleEntry(manager, std::string()),
+      : ExtensionThrottleEntry(std::string()),
         fake_clock_(fake_now),
         backoff_entry_(&backoff_policy_, &fake_clock_) {
     InitPolicy();
@@ -42,6 +41,8 @@ class MockExtensionThrottleEntry : public ExtensionThrottleEntry {
     set_sliding_window_release_time(sliding_window_release_time);
   }
 
+  ~MockExtensionThrottleEntry() override {}
+
   void InitPolicy() {
     // Some tests become flaky if we have jitter.
     backoff_policy_.jitter_factor = 0.0;
@@ -85,9 +86,6 @@ class MockExtensionThrottleEntry : public ExtensionThrottleEntry {
     ExtensionThrottleEntry::set_sliding_window_release_time(release_time);
   }
 
- protected:
-  ~MockExtensionThrottleEntry() override {}
-
  private:
   mutable TestTickClock fake_clock_;
   BackoffEntry backoff_entry_;
@@ -101,8 +99,7 @@ class MockExtensionThrottleManager : public ExtensionThrottleManager {
     return ExtensionThrottleManager::GetIdFromUrl(url);
   }
 
-  scoped_refptr<ExtensionThrottleEntryInterface> RegisterRequestUrl(
-      const GURL& url) {
+  ExtensionThrottleEntry* RegisterRequestUrl(const GURL& url) {
     return ExtensionThrottleManager::RegisterRequestUrl(url);
   }
 
@@ -122,9 +119,9 @@ class MockExtensionThrottleManager : public ExtensionThrottleManager {
     std::string fake_url_string("http://www.fakeurl.com/");
     fake_url_string.append(base::IntToString(create_entry_index_++));
     GURL fake_url(fake_url_string);
-    OverrideEntryForTests(
-        fake_url, new MockExtensionThrottleEntry(this, time, TimeTicks::Now(),
-                                                 TimeTicks::Now()));
+    OverrideEntryForTests(fake_url,
+                          std::make_unique<MockExtensionThrottleEntry>(
+                              time, TimeTicks::Now(), TimeTicks::Now()));
   }
 
  private:
@@ -165,12 +162,12 @@ class ExtensionThrottleEntryTest : public testing::Test {
 
   TimeTicks now_;
   MockExtensionThrottleManager manager_;  // Dummy object, not used.
-  scoped_refptr<MockExtensionThrottleEntry> entry_;
+  std::unique_ptr<MockExtensionThrottleEntry> entry_;
 };
 
 void ExtensionThrottleEntryTest::SetUp() {
   now_ = TimeTicks::Now();
-  entry_ = new MockExtensionThrottleEntry(&manager_);
+  entry_.reset(new MockExtensionThrottleEntry());
   entry_->ResetToBlank(now_);
 }
 
@@ -365,7 +362,7 @@ TEST(ExtensionThrottleManagerTest, IsHostBeingRegistered) {
 TEST(ExtensionThrottleManagerTest, LocalHostOptedOut) {
   MockExtensionThrottleManager manager;
   // A localhost entry should always be opted out.
-  scoped_refptr<ExtensionThrottleEntryInterface> localhost_entry =
+  ExtensionThrottleEntry* localhost_entry =
       manager.RegisterRequestUrl(GURL("http://localhost/hello"));
   EXPECT_FALSE(localhost_entry->ShouldRejectRequest(net::LOAD_NORMAL));
   for (int i = 0; i < 10; ++i) {
@@ -384,7 +381,7 @@ TEST(ExtensionThrottleManagerTest, LocalHostOptedOut) {
 TEST(ExtensionThrottleManagerTest, ClearOnNetworkChange) {
   for (int i = 0; i < 2; ++i) {
     MockExtensionThrottleManager manager;
-    scoped_refptr<ExtensionThrottleEntryInterface> entry_before =
+    ExtensionThrottleEntry* entry_before =
         manager.RegisterRequestUrl(GURL("http://www.example.com/"));
     for (int j = 0; j < 10; ++j) {
       entry_before->UpdateWithResponse(503);
@@ -402,7 +399,7 @@ TEST(ExtensionThrottleManagerTest, ClearOnNetworkChange) {
         FAIL();
     }
 
-    scoped_refptr<ExtensionThrottleEntryInterface> entry_after =
+    ExtensionThrottleEntry* entry_after =
         manager.RegisterRequestUrl(GURL("http://www.example.com/"));
     EXPECT_FALSE(entry_after->ShouldRejectRequest(net::LOAD_NORMAL));
   }