Implementation of network level throttler.

net/base/network_stream_throttler.h

Index: net/base/network_stream_throttler.h
diff --git a/net/base/network_stream_throttler.h b/net/base/network_stream_throttler.h
new file mode 100644
index 0000000000000000000000000000000000000000..841f42ad48de96695725e52d9d784047d456549c
--- /dev/null
+++ b/net/base/network_stream_throttler.h
@@ -0,0 +1,90 @@
+// Copyright 2016 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 NET_BASE_NETWORK_STREAM_THROTTLER_H_
+#define NET_BASE_NETWORK_STREAM_THROTTLER_H_
+
+#include <memory>
+
+#include "base/memory/weak_ptr.h"
+#include "net/base/priority_queue.h"
+#include "net/base/request_priority.h"
+
+namespace net {
+
+class NetworkStreamThrottler {
+ public:
+  // Abstract base class other classes can inherit from to get
+  // notifications from throttle state changes.
+  class Delegate {
+   public:
+    virtual ~Delegate() {}

[mmenke, 2016/04/29 16:57:28]

Make the destructor protected?  Doesn't matter much, but makes it clearer the
throttler doesn't delete these.

[Randy Smith (Not in Mondays), 2016/05/08 00:46:30]

Done.

+
+    // Called whenever the throttle state of this stream has changed.
+    // The new state can be determined through Throttle::throttled().
+    virtual void OnThrottleStateChanged() = 0;
+  };
+
+  // Concrete class owned by external stream representations that
+  // routes notifications.
+  class Throttle {
+   public:
+    ~Throttle();
+
+    bool throttled() { return throttled_; }

[mmenke, 2016/04/29 16:57:28]

const

[Randy Smith (Not in Mondays), 2016/05/08 00:46:30]

Done.

+
+    // The caller must make sure that |*delegate| outlives this
+    // object.
+    void set_delegate(Delegate* delegate) { delegate_ = delegate; }

[mmenke, 2016/04/29 16:57:28]

Can we make this a constructor argument instead?

[Randy Smith (Not in Mondays), 2016/05/08 00:46:30]

Sure.  It doesn't look like I ever called this function anywhere anyway.  Ooops
:-{.

+
+    // Note that this may result in a possibly reentrant call to
+    // |Delegate::OnThrottleStateChanged|.
+    void SetPriority(RequestPriority priority);
+
+   private:
+    friend NetworkStreamThrottler;
+
+    using QueuePointer = PriorityQueue<Throttle*>::Pointer;
+
+    Throttle(bool throttled,
+             RequestPriority priority,
+             base::WeakPtr<NetworkStreamThrottler> throttler);

[mmenke, 2016/04/29 16:57:28]

Is there any case where the Throttle can be destroyed before the
NetworkStreamThrottler?  Generally, if transactions outlive any component of the
network stack, we're doomed, anyways.

[Randy Smith (Not in Mondays), 2016/05/08 00:46:30]

I think you mean the other way around, and no, I don't think there is.  Though
that guarantee goes through many layers of ownership (as do all the guarantees
you're referring to).  I'll change the interface and put in some documentation
about that requirement.

+
+    void NotifyThrottleStateChanged(bool new_throttle_state);

[mmenke, 2016/04/29 16:57:28]

I don't think we need to take a bool?  Seems like we'll only unthrottle
requests.  If we want to start unthrottled (Which presumably we will), should
make that a constructor parameter.

[Randy Smith (Not in Mondays), 2016/05/08 00:46:30]

Hmmm.  This was planning for the long-term; I could imagine that the throttling
would go both ways, and eventually I'd like to do something when it does.  (E.g.
throttling IDLE connections when there are more than N connections of any type).
 But yeah, it probably makes sense to postpone that interface until it's
actually used; certainly HttpNetworkTransaction can't currently handle a
transition back to throttled.

I'm confused by your comment about a constructor parameter, though; to what
class?  Throttle's already got a parameter defining it's initial throttle state.
 (For this CL I want to start unthrottled and never change; in the next CL where
I make this class do something rather than just put in the plumbing, objects can
start in either state, depending on the throttler.

[mmenke, 2016/05/09 17:37:48]

I missed the constructor parameter when I made that comment.

+    QueuePointer queue_pointer() { return queue_pointer_; }

[mmenke, 2016/04/29 16:57:28]

const?

[Randy Smith (Not in Mondays), 2016/05/08 00:46:30]

Done.

+    void set_queue_pointer(const QueuePointer& pointer) {
+      queue_pointer_ = pointer;
+    }
+
+    bool throttled_;
+    RequestPriority priority_;
+    Delegate* delegate_;
+    PriorityQueue<Throttle*>::Pointer queue_pointer_;
+
+    base::WeakPtr<NetworkStreamThrottler> throttler_;
+
+    DISALLOW_COPY_AND_ASSIGN(Throttle);
+  };
+
+  NetworkStreamThrottler();
+  ~NetworkStreamThrottler();
+
+  std::unique_ptr<Throttle> CreateThrottle(RequestPriority priority,
+                                           bool ignore_limits);
+
+ private:
+  void OnStreamPriorityChanged(Throttle* throttle,
+                               RequestPriority old_priority,
+                               RequestPriority new_priority);
+  void OnStreamDestroyed(Throttle* throttle);
+
+  PriorityQueue<Throttle*> priority_queue_;
+  base::WeakPtrFactory<NetworkStreamThrottler> factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(NetworkStreamThrottler);
+};
+
+}  // namespace net
+
+#endif  // NET_BASE_NETWORK_STREAM_THROTTLER_H_