Implement THROTTLED priority semantics.

net/base/network_throttle_manager_unittest.cc

Index: net/base/network_throttle_manager_unittest.cc
diff --git a/net/base/network_throttle_manager_unittest.cc b/net/base/network_throttle_manager_unittest.cc
index c39a7a894fd4a97a904e3fa70b624b44364555f5..cf23a06b6f601afe0eed368e9ef103df5e464353 100644
--- a/net/base/network_throttle_manager_unittest.cc
+++ b/net/base/network_throttle_manager_unittest.cc
@@ -2,10 +2,15 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "net/base/network_throttle_manager.h"
+#include "net/base/network_throttle_manager_impl.h"
 
 #include <memory>
 
+#include "base/bind.h"
+#include "base/callback.h"
+#include "base/callback_helpers.h"
+#include "base/memory/scoped_vector.h"
+#include "base/test/simple_test_tick_clock.h"
 #include "net/base/request_priority.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -19,33 +24,432 @@ class NetworkThrottleManagerTest : public testing::Test,
                                    NetworkThrottleManager::ThrottleDelegate {
  public:
   NetworkThrottleManagerTest()
-      : throttler_(NetworkThrottleManager::CreateThrottler()) {}
+      : throttle_state_change_count_(0),
+        last_throttle_state_change_(nullptr),
+        throttle_manager_(new NetworkThrottleManagerImpl) {}
 
  protected:
   std::unique_ptr<NetworkThrottleManager::Throttle> CreateThrottle(
       net::RequestPriority priority,
       bool expected_throttle_state) {
     std::unique_ptr<NetworkThrottleManager::Throttle> throttle(
-        throttler_->CreateThrottle(this, priority, false));
-    EXPECT_EQ(expected_throttle_state, throttle->IsThrottled());
+        throttle_manager_->CreateThrottle(this, priority, false));
+    EXPECT_EQ(expected_throttle_state, throttle->IsBlocked());
+    throttled_blocked_[throttle.get()] = !throttle->IsBlocked();

[Charlie Harrison, 2016/07/08 22:36:05]

Hmm. So I am only looking at this from top to bottom but it really seems like
this map should be returning the blocked status of the throttle, not the
unblocked status?

If I'm wrong then I think the name of the member should be changed.

[Randy Smith (Not in Mondays), 2016/08/29 19:44:51]

Wow, I wonder what sequence of global replaces resulted in that inversion.  I
scanned, and I don't appear to be using this variable anymore, so I just nuked
it everywhere.

     return throttle;
   }
 
+  std::unique_ptr<NetworkThrottleManager::Throttle>
+  CreateThrottleIgnoringLimits(net::RequestPriority priority) {
+    std::unique_ptr<NetworkThrottleManager::Throttle> throttle(
+        throttle_manager_->CreateThrottle(this, priority, true));
+    EXPECT_FALSE(throttle->IsBlocked());
+    throttled_blocked_[throttle.get()] = !throttle->IsBlocked();
+    return throttle;
+  }
+
+  int throttle_state_change_count() { return throttle_state_change_count_; }
+
+  NetworkThrottleManager::Throttle* last_throttle_state_change() {
+    return last_throttle_state_change_;
+  }
+
+  void SetUpcallCallback(const base::Closure& callback) {

[Charlie Harrison, 2016/07/08 22:36:06]

Can you document the upcall callback? It's a very general name. My brief reading
is: "closure to be called when a throttle state changes".

If that's correct, then a better name might be
"throttle_state_changed_callback".

[Randy Smith (Not in Mondays), 2016/08/29 19:44:51]

Good point; done.

+    upcall_callback_ = callback;
+  }
+
+  NetworkThrottleManagerImpl* throttle_manager() {
+    return throttle_manager_.get();
+  }
+
  private:
   // NetworkThrottleManager::Delegate
-  void OnThrottleStateChanged() override { ADD_FAILURE(); }
+  void OnThrottleStateChanged(
+      NetworkThrottleManager::Throttle* throttle) override {
+    EXPECT_FALSE(throttled_blocked_.find(throttle) == throttled_blocked_.end());
+    EXPECT_FALSE(throttled_blocked_[throttle]);
+    throttled_blocked_[throttle] = true;
+    ++throttle_state_change_count_;
+    last_throttle_state_change_ = throttle;
+    if (!upcall_callback_.is_null())
+      base::ResetAndReturn(&upcall_callback_).Run();
+  }
 
-  std::unique_ptr<NetworkThrottleManager> throttler_;
+  int throttle_state_change_count_;
+  NetworkThrottleManager::Throttle* last_throttle_state_change_;
+  std::unique_ptr<NetworkThrottleManagerImpl> throttle_manager_;
+  std::map<NetworkThrottleManager::Throttle*, bool> throttled_blocked_;
+  base::Closure upcall_callback_;
 };
 
-// Check to confirm that all created throttles start unthrottled for the
-// current null implementation.
+// Check to confirm that all created throttles at priorities other than
+// THROTTLED start unblocked.
 TEST_F(NetworkThrottleManagerTest, AllUnthrottled) {
   for (int i = MINIMUM_PRIORITY; i <= MAXIMUM_PRIORITY; ++i) {
+    if (i == THROTTLED)
+      continue;
     CreateThrottle(static_cast<RequestPriority>(i), false);
   }
 }
 
+// Check for basic semantics around the new THROTTLED level.
+TEST_F(NetworkThrottleManagerTest, ThrottledBlocking) {
+  std::unique_ptr<NetworkThrottleManager::Throttle> throttle1(
+      CreateThrottle(THROTTLED, false));
+  std::unique_ptr<NetworkThrottleManager::Throttle> throttle2(
+      CreateThrottle(THROTTLED, false));
+  std::unique_ptr<NetworkThrottleManager::Throttle> throttle3(
+      CreateThrottle(THROTTLED, true));
+  std::unique_ptr<NetworkThrottleManager::Throttle> throttle4(
+      CreateThrottle(THROTTLED, true));
+  std::unique_ptr<NetworkThrottleManager::Throttle> throttle5(
+      CreateThrottle(THROTTLED, true));
+
+  EXPECT_EQ(0, throttle_state_change_count());
+
+  throttle1.reset();
+  EXPECT_EQ(1, throttle_state_change_count());
+  EXPECT_EQ(throttle3.get(), last_throttle_state_change());
+
+  EXPECT_FALSE(throttle3->IsBlocked());
+  EXPECT_TRUE(throttle4->IsBlocked());
+  EXPECT_TRUE(throttle5->IsBlocked());
+
+  throttle2.reset();
+  EXPECT_EQ(2, throttle_state_change_count());
+  EXPECT_EQ(throttle4.get(), last_throttle_state_change());
+
+  EXPECT_FALSE(throttle3->IsBlocked());
+  EXPECT_FALSE(throttle4->IsBlocked());
+  EXPECT_TRUE(throttle5->IsBlocked());
+}
+
+// Check that THROTTLED semantics are dependent on all outstanding requests.
+TEST_F(NetworkThrottleManagerTest, ThrottledBlockingMultiPriority) {
+  std::unique_ptr<NetworkThrottleManager::Throttle> throttle1(
+      CreateThrottle(HIGHEST, false));
+  std::unique_ptr<NetworkThrottleManager::Throttle> throttle2(
+      CreateThrottle(LOW, false));
+  std::unique_ptr<NetworkThrottleManager::Throttle> throttle3(
+      CreateThrottle(IDLE, false));
+  std::unique_ptr<NetworkThrottleManager::Throttle> throttle4(
+      CreateThrottle(THROTTLED, true));
+  std::unique_ptr<NetworkThrottleManager::Throttle> throttle5(
+      CreateThrottle(THROTTLED, true));
+
+  EXPECT_EQ(0, throttle_state_change_count());
+
+  throttle1.reset();
+  EXPECT_EQ(0, throttle_state_change_count());
+  EXPECT_FALSE(throttle3->IsBlocked());
+  EXPECT_TRUE(throttle4->IsBlocked());
+  EXPECT_TRUE(throttle5->IsBlocked());
+
+  throttle2.reset();
+  EXPECT_EQ(1, throttle_state_change_count());
+  EXPECT_EQ(throttle4.get(), last_throttle_state_change());
+
+  EXPECT_FALSE(throttle3->IsBlocked());
+  EXPECT_FALSE(throttle4->IsBlocked());
+  EXPECT_TRUE(throttle5->IsBlocked());
+
+  throttle3.reset();
+  EXPECT_EQ(2, throttle_state_change_count());
+  EXPECT_EQ(throttle5.get(), last_throttle_state_change());
+
+  EXPECT_FALSE(throttle4->IsBlocked());
+  EXPECT_FALSE(throttle5->IsBlocked());
+}
+
+// Check that a SetPriority() away from THROTTLED results in unblocking
+// and an upcall.
+TEST_F(NetworkThrottleManagerTest, ThrottledSetPriority) {
+  std::unique_ptr<NetworkThrottleManager::Throttle> throttle1(
+      CreateThrottle(THROTTLED, false));
+  std::unique_ptr<NetworkThrottleManager::Throttle> throttle2(
+      CreateThrottle(THROTTLED, false));
+  std::unique_ptr<NetworkThrottleManager::Throttle> throttle3(
+      CreateThrottle(THROTTLED, true));
+  std::unique_ptr<NetworkThrottleManager::Throttle> throttle4(
+      CreateThrottle(THROTTLED, true));
+
+  EXPECT_EQ(0, throttle_state_change_count());
+
+  throttle3->SetPriority(LOW);
+  EXPECT_EQ(1, throttle_state_change_count());
+  EXPECT_EQ(throttle3.get(), last_throttle_state_change());
+  EXPECT_FALSE(throttle3->IsBlocked());
+  EXPECT_TRUE(throttle4->IsBlocked());
+}
+
+void ResetThrottles(ScopedVector<NetworkThrottleManager::Throttle> throttles) {
+  // All pointers in the vector should be deleted on exit.
+}
+
+// Note that this routine is dependent on priority setting

[Charlie Harrison, 2016/07/08 22:36:05]

nit: Can fit more words on this line.

[Randy Smith (Not in Mondays), 2016/08/29 19:44:52]

Done.

+// *not* resulting in destruction of any throttle and should only
+// be used in tests where that is true.
+void SetAllToPriority(
+    RequestPriority priority,
+    std::vector<NetworkThrottleManager::Throttle*> throttles) {
+  for (size_t i = 0; i < throttles.size(); ++i)
+    throttles[i]->SetPriority(priority);
+}
+
+// Check that tearing down all elements in the NTM on a SetPriority
+// upcall doesn't create any problems.
+TEST_F(NetworkThrottleManagerTest, ThrottleTeardown) {
+  ScopedVector<NetworkThrottleManager::Throttle> throttles;
+  std::unique_ptr<NetworkThrottleManager::Throttle> throttle_temporary;
+
+  throttle_temporary = CreateThrottle(THROTTLED, false);

[Charlie Harrison, 2016/07/08 22:36:06]

Not sure the temporary variable is needed until throttle3.

[Randy Smith (Not in Mondays), 2016/08/29 19:44:52]

Good point; done.

+  throttles.push_back(std::move(throttle_temporary.release()));
+
+  throttle_temporary = CreateThrottle(THROTTLED, false);
+  throttles.push_back(std::move(throttle_temporary.release()));
+
+  // Note that if there is more than one throttle blocked, then the
+  // number of throttle state changes is dependent on destruction order.
+  // So only one blocked throttle is created.
+
+  throttle_temporary = CreateThrottle(THROTTLED, true);
+  NetworkThrottleManager::Throttle* throttle3 = throttle_temporary.get();
+  throttles.push_back(std::move(throttle_temporary.release()));
+
+  SetUpcallCallback(base::Bind(&ResetThrottles, base::Passed(&throttles)));
+
+  EXPECT_EQ(0, throttle_state_change_count());
+
+  throttle3->SetPriority(LOW);
+  // If the test is functioning as expected, throttle3 now points to
+  // a deleted object and can no longer be indirected through.
+
+  EXPECT_EQ(1, throttle_state_change_count());
+  EXPECT_EQ(throttle3, last_throttle_state_change());
+}
+
+// Check that modifying all the priorities of the allocated throttles in
+// the callback works properly.
+TEST_F(NetworkThrottleManagerTest, ThrottlePriorityReset) {
+  std::unique_ptr<NetworkThrottleManager::Throttle> throttle1(
+      CreateThrottle(THROTTLED, false));
+  std::unique_ptr<NetworkThrottleManager::Throttle> throttle2(
+      CreateThrottle(THROTTLED, false));
+  std::unique_ptr<NetworkThrottleManager::Throttle> throttle3(
+      CreateThrottle(THROTTLED, true));
+  std::unique_ptr<NetworkThrottleManager::Throttle> throttle4(
+      CreateThrottle(THROTTLED, true));
+
+  std::vector<NetworkThrottleManager::Throttle*> throttles;
+  throttles.push_back(throttle1.get());
+  throttles.push_back(throttle2.get());
+  throttles.push_back(throttle3.get());
+
+  SetUpcallCallback(
+      base::Bind(&SetAllToPriority, MEDIUM, base::Passed(&throttles)));
+
+  EXPECT_EQ(0, throttle_state_change_count());
+  throttle3->SetPriority(HIGHEST);
+
+  // Expected result: throttles 1-3 @ medium priority (the callback

[Charlie Harrison, 2016/07/08 22:36:06]

nit: more words can fit on the line.

[Randy Smith (Not in Mondays), 2016/08/29 19:44:51]

Done.

+  // should have overridden the priority setting above), only throttle 4
+  // blocked  (throttle3 should have been unblocked by either of
+  // the priority changes), and one state changes (the unblocking).
+  EXPECT_EQ(MEDIUM, throttle1->Priority());
+  EXPECT_EQ(MEDIUM, throttle2->Priority());
+  EXPECT_EQ(MEDIUM, throttle3->Priority());
+  EXPECT_EQ(THROTTLED, throttle4->Priority());
+  EXPECT_FALSE(throttle1->IsBlocked());
+  EXPECT_FALSE(throttle2->IsBlocked());
+  EXPECT_FALSE(throttle3->IsBlocked());
+  EXPECT_TRUE(throttle4->IsBlocked());
+  EXPECT_EQ(1, throttle_state_change_count());
+}
+
+// Check that modifying the priority of a request from a non-THROTTLED
+// value to THROTTLED causes no change in behavior.
+TEST_F(NetworkThrottleManagerTest, ThrottlePriorityResetToThrottled) {
+  std::unique_ptr<NetworkThrottleManager::Throttle> throttle1(
+      CreateThrottle(THROTTLED, false));
+  std::unique_ptr<NetworkThrottleManager::Throttle> throttle2(
+      CreateThrottle(THROTTLED, false));
+  std::unique_ptr<NetworkThrottleManager::Throttle> throttle3(
+      CreateThrottle(LOW, false));
+  std::unique_ptr<NetworkThrottleManager::Throttle> throttle4(
+      CreateThrottle(THROTTLED, true));
+
+  EXPECT_EQ(0, throttle_state_change_count());
+  throttle3->SetPriority(THROTTLED);
+  EXPECT_EQ(0, throttle_state_change_count());
+
+  EXPECT_FALSE(throttle1->IsBlocked());
+  EXPECT_FALSE(throttle2->IsBlocked());
+  EXPECT_FALSE(throttle3->IsBlocked());
+  EXPECT_TRUE(throttle4->IsBlocked());
+
+  EXPECT_EQ(THROTTLED, throttle1->Priority());
+  EXPECT_EQ(THROTTLED, throttle2->Priority());
+  EXPECT_EQ(THROTTLED, throttle3->Priority());
+  EXPECT_EQ(THROTTLED, throttle4->Priority());
+}
+
+// Confirm that old requests don't count against the ilmit.

[Charlie Harrison, 2016/07/08 22:36:05]

s/ilmit/limit

[Randy Smith (Not in Mondays), 2016/08/29 19:44:51]

Done.

+TEST_F(NetworkThrottleManagerTest, DontCountAgedRequests) {
+  base::TimeTicks now(base::TimeTicks::Now());
+  base::SimpleTestTickClock* clock(new base::SimpleTestTickClock);
+
+  throttle_manager()->SetTickClockForTesting(

[Charlie Harrison, 2016/07/08 22:36:05]

Optional: eek I don't like the passing ownership of the clock here. Could you
expose a tick_clock_for_testing() here?

[Randy Smith (Not in Mondays), 2016/08/29 19:44:51]

I'm feeling an aversion against that, which I guess I'll abstract as "If I'm
going to do something ugly, I'd like to make that maximally confined to the
tests as opposed to the production interface".  So I'd rather not add another
interface to the NetworkThrottleManager class. 

Also note (though it's ugliness here rather than in the header, so doesn't weigh
as heavily with me) that if I expose tick_clock_for_testing() I have to static
cast it here to a SimpleTestTickClock so that I can get access to the
SetNowTicks method.  My gut suggests that's not worth it.  If you feel
differently, would you be willing to do your own deconstruction and weighing of
the different uglinesses involved?

[Charlie Harrison, 2016/08/29 21:10:25]

I will defer to you here, I don't care too much.

+      std::unique_ptr<base::TickClock>(clock));
+
+  const int age_in_days_of_old_throttles = 4;
+
+  // Confirm default median and timing means that 4 days is long enough ago
+  // to be aged out.
+  EXPECT_GT(age_in_days_of_old_throttles * 24 * 60 * 60 * 1000,
+            NetworkThrottleManagerImpl::kInitialMedianInMS *

[Charlie Harrison, 2016/07/08 22:36:05]

Just noticed this but shouldn't it be "Ms" instead of "MS"? I know we are
avoiding URL for Url nowadays.

[Randy Smith (Not in Mondays), 2016/08/29 19:44:52]

Sure.  Done.

+                NetworkThrottleManagerImpl::kMedianLifetimeMultiple);

[Charlie Harrison, 2016/07/08 22:36:06]

This check seems wrong. Can the median*median_multiple be pulled into a static
function or another constant?

[Randy Smith (Not in Mondays), 2016/08/29 19:44:51]

Just confirming: Your only concern is about using the multiplication as opposed
to a constant?  If so, done, but I don't see that as fitting with "This check
seems wrong".

[Charlie Harrison, 2016/08/29 21:10:25]

Sorry this was a very unclear comment :(

I think I meant (it was a while ago) that median*median_multiple is not a clear
representation of "aged out". I.e. you are testing an implementation level
detail not an interface level detail.

+
+  clock->SetNowTicks(now -
+                     base::TimeDelta::FromDays(age_in_days_of_old_throttles));
+  std::unique_ptr<NetworkThrottleManager::Throttle> throttle1(
+      CreateThrottle(IDLE, false));
+  std::unique_ptr<NetworkThrottleManager::Throttle> throttle2(
+      CreateThrottle(IDLE, false));
+
+  clock->SetNowTicks(now);
+  std::unique_ptr<NetworkThrottleManager::Throttle> throttle3(
+      CreateThrottle(LOW, false));
+
+  // First throttled request should not be blocked.
+  std::unique_ptr<NetworkThrottleManager::Throttle> throttle4(
+      CreateThrottle(THROTTLED, false));
+
+  // Second should be.
+  std::unique_ptr<NetworkThrottleManager::Throttle> throttle5(
+      CreateThrottle(THROTTLED, true));
+
+  // Destroying the old requests should not result in any upcalls.
+  EXPECT_EQ(0, throttle_state_change_count());
+  throttle1.reset();
+  EXPECT_EQ(0, throttle_state_change_count());

[Charlie Harrison, 2016/07/08 22:36:06]

This is because they weren't blocked right? Can you include a test where there
are 3 old throttles?

[Randy Smith (Not in Mondays), 2016/08/29 19:44:51]

Sorry, I don't think I understand your thinking, so I'm not sure I understand
the purpose of your request.  It has nothing to do with whether the old requests
were blocked or not; old requests "don't count" against the active request
limit.  And the old requests are all non-THROTTLED priority, so they won't be
blocked even if I put in a third.  Give me a bit more of a sense of what you're
looking for?

This does bring up the possibility of a THROTTLED requests being blocked for a
while, and aging out *because* it's been blocked for a while.  Which suggests
that the age of requests should be measured from the point at which they become
unblocked.  I'll look into that.

[Charlie Harrison, 2016/08/29 21:10:25]

Again I was being unclear. I'm not quite sure what I was thinking :P
However, thinking about it now, I wonder if we could include a test where there
is a blocked, throttled requests that eventually goes through because the two
blockees are aged out. This aligns well with your point to measure age at time
unblocked.

+  throttle2.reset();
+  EXPECT_EQ(0, throttle_state_change_count());
+
+  // But destroying a new request should result in a state change.
+  throttle3.reset();
+  EXPECT_EQ(1, throttle_state_change_count());
+  EXPECT_EQ(throttle5.get(), last_throttle_state_change());
+}
+
+// Confirm that a slew of throttles of a specific age will shift the
+// median for determining "aged requests" to that age.
+TEST_F(NetworkThrottleManagerTest, ShiftMedian) {
+  base::TimeTicks now(base::TimeTicks::Now());
+  base::SimpleTestTickClock* clock(new base::SimpleTestTickClock);
+
+  throttle_manager()->SetTickClockForTesting(
+      std::unique_ptr<base::TickClock>(clock));
+
+  // Setup two throttles of age *just short* of aging out; confirm
+  // they result in blocked THROTTLED requests.
+  clock->SetNowTicks(
+      now - base::TimeDelta::FromMilliseconds(
+                NetworkThrottleManagerImpl::kInitialMedianInMS *
+                    NetworkThrottleManagerImpl::kMedianLifetimeMultiple -
+                1));
+  std::unique_ptr<NetworkThrottleManager::Throttle> throttle1(
+      CreateThrottle(IDLE, false));
+  std::unique_ptr<NetworkThrottleManager::Throttle> throttle2(
+      CreateThrottle(IDLE, false));
+  clock->SetNowTicks(now);
+
+  // Two throttles need to be created because of an implementation issue;
+  // there isn't a way to force recomputation of outstanding requests on a
+  // test-provoked time shift, so this test relies on the first throttle
+  // creation to do it for the second throttle.
+  // First one can't be throttled since that result is indeterminate because

[Charlie Harrison, 2016/07/08 22:36:06]

The first one .... an implementation detail

[Randy Smith (Not in Mondays), 2016/08/29 19:44:52]

Done.

+  // of implementation detail (SetNowTicks() doesn't force a recomputation of
+  // how many requests have aged out).

[Charlie Harrison, 2016/07/08 22:36:06]

You could expose another for testing method...

[Randy Smith (Not in Mondays), 2016/08/29 19:44:51]

Actually, I already did.  Let's see if I can incorporate it in this test.

<time passes>

Well, that was interesting.  The comment I felt compelled to add before
ConditionallyTriggerTimerForTesting() 
is beginning to push my "too complex" buttons; you may want to specifically give
that comment a review.

Done.

+  std::unique_ptr<NetworkThrottleManager::Throttle> throttle3(
+      CreateThrottle(IDLE, false));
+  std::unique_ptr<NetworkThrottleManager::Throttle> throttle4(
+      CreateThrottle(THROTTLED, true));
+
+  throttle1.reset();
+  throttle2.reset();
+  throttle3.reset();
+  throttle4.reset();
+
+  // Create 100 throttles and destroy them, effectively with lifetime zero.
+  // This should substantially decrease the median age estimate.

[Charlie Harrison, 2016/07/08 22:36:06]

This test makes me nervous about fast subresource aborts skewing the median
downwards. I wonder if we should only count requests that succeed in the
recomputation of the median.

[Randy Smith (Not in Mondays), 2016/08/29 19:44:52]

Heh.  That would be a plumbing nightmare (i.e. probably require an architectural
re-think) since destructors don't usually take arguments :-}.  Do you have a
sense as to how often and in what context fast subresource aborts occur?  I'd
really like to just sweep this one under the rug, especially since I don't think
it's vital that we get the median estimation exactly right.

[Charlie Harrison, 2016/08/29 21:10:25]

Fast subresource aborts occur with the js api window.stop() or by pressing the
stop button on a loading page, or by navigating away from a loading page.

This will generally abort all in-flight subresources. ABORTED/OK = 3.5%.

RE: complexity. Yeah I feel you. Maybe you could have the delegate pipe
information into the throttle? I'd have to think about it but I'm fine punting
it.

+  for (int i = 0; i < 100; ++i) {
+    std::unique_ptr<NetworkThrottleManager::Throttle> tmp(
+        CreateThrottle(IDLE, false));
+  }
+
+  // The identical test above should no longer result in blocked throttles.
+  clock->SetNowTicks(
+      now - base::TimeDelta::FromMilliseconds(
+                NetworkThrottleManagerImpl::kInitialMedianInMS *
+                    NetworkThrottleManagerImpl::kMedianLifetimeMultiple -
+                1));
+  std::unique_ptr<NetworkThrottleManager::Throttle> throttle5(
+      CreateThrottle(IDLE, false));
+  std::unique_ptr<NetworkThrottleManager::Throttle> throttle6(
+      CreateThrottle(IDLE, false));
+  clock->SetNowTicks(now);
+  // First one can't be throttled since that result is indeterminate because
+  // of implementation detail (SetNowTicks() doesn't force a recomputation of
+  // how many requests have aged out).
+  std::unique_ptr<NetworkThrottleManager::Throttle> throttle7(
+      CreateThrottle(IDLE, false));
+  std::unique_ptr<NetworkThrottleManager::Throttle> throttle8(
+      CreateThrottle(THROTTLED, false));
+}
+
+// Confirm that just "aging out" reuqests will result in unblocking

[Charlie Harrison, 2016/07/08 22:36:06]

s/reuqests/requests

[Randy Smith (Not in Mondays), 2016/08/29 19:44:52]

Done.

+// blocked requests.
+TEST_F(NetworkThrottleManagerTest, AgeInvalidThrottles) {
+  base::TimeTicks now(base::TimeTicks::Now());
+  base::SimpleTestTickClock* clock(new base::SimpleTestTickClock);
+
+  throttle_manager()->SetTickClockForTesting(
+      std::unique_ptr<base::TickClock>(clock));
+  clock->SetNowTicks(base::TimeTicks::Now());
+
+  std::unique_ptr<NetworkThrottleManager::Throttle> throttle1(
+      CreateThrottle(IDLE, false));
+  std::unique_ptr<NetworkThrottleManager::Throttle> throttle2(
+      CreateThrottle(IDLE, false));
+  clock->SetNowTicks(now);
+  std::unique_ptr<NetworkThrottleManager::Throttle> throttle3(
+      CreateThrottle(THROTTLED, true));
+
+  EXPECT_EQ(0, throttle_state_change_count());
+  clock->SetNowTicks(
+      now + base::TimeDelta::FromMilliseconds(
+                NetworkThrottleManagerImpl::kInitialMedianInMS *
+                (NetworkThrottleManagerImpl::kMedianLifetimeMultiple + 1)));
+  throttle_manager()->ConditionallyTriggerTimerForTesting();
+  EXPECT_EQ(1, throttle_state_change_count());
+  EXPECT_EQ(throttle3.get(), last_throttle_state_change());
+  EXPECT_FALSE(throttle3->IsBlocked());
+}
+
+// Confirm that "ignore_limits" boolean is respected.
+TEST_F(NetworkThrottleManagerTest, IgnoreLimits) {
+  std::unique_ptr<NetworkThrottleManager::Throttle> throttle1(
+      CreateThrottle(HIGHEST, false));
+  std::unique_ptr<NetworkThrottleManager::Throttle> throttle2(
+      CreateThrottle(LOW, false));
+  std::unique_ptr<NetworkThrottleManager::Throttle> throttle3(
+      CreateThrottle(IDLE, false));
+  std::unique_ptr<NetworkThrottleManager::Throttle> throttle4(
+      CreateThrottle(THROTTLED, true));
+  std::unique_ptr<NetworkThrottleManager::Throttle> throttle5(
+      CreateThrottleIgnoringLimits(THROTTLED));
+}
+
 }  // namespace
 
 }  // namespace net