Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(269)

Unified Diff: net/spdy/http2_priority_dependencies.h

Issue 1779733003: Fix bug in net::RequestPriority -> HTTP/2 dependency conversion. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 4 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: net/spdy/http2_priority_dependencies.h
diff --git a/net/spdy/http2_priority_dependencies.h b/net/spdy/http2_priority_dependencies.h
new file mode 100644
index 0000000000000000000000000000000000000000..8402f51ba0d334c6ce7b49a89e204298caef13d1
--- /dev/null
+++ b/net/spdy/http2_priority_dependencies.h
@@ -0,0 +1,70 @@
+// 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_HTTP2_PRIORITY_DEPENDENCIES_H_
+#define NET_HTTP2_PRIORITY_DEPENDENCIES_H_
+
+#include <list>
+#include <map>
+
+#include "net/base/request_priority.h"
+#include "net/spdy/spdy_protocol.h"
+
+namespace net {
+
+// A helper class encapsulating the state and logic to set dependencies of
+// HTTP2 streams based on their net::RequestPriority and the ordering
+// of creation and deletion of the streams.
+class Http2PriorityDependencies {
+ public:
+ Http2PriorityDependencies();
+ ~Http2PriorityDependencies();
+
+ // Called to override default behavior for testing. If |enabled|,
+ // |OnStreamSynSent()| will output correct header values; if
+ // |!enabled|, |*dependent_stream_id| will be set to 0 and
+ // |*exclusive| to false.
+ // Returns old default value (for resetting at end of test).
+ static bool SetEnabledDefaultForTesting(bool enabled);
+
+ // Called when a stream SYN is sent to the server. Note that in the
+ // case of server push, a stream may be created without this routine
+ // being called. In such cases, the client ignores the stream's priority
+ // (as the server is effectively overriding the client's notions of
+ // priority anyway.
Bence 2016/03/10 20:21:14 Nit: close opening parenthesis.
Randy Smith (Not in Mondays) 2016/03/16 19:38:10 Done.
+ // On return, |*dependent_stream_id| is set to the stream id that
+ // this stream should be made dependent on, and |*exclusive| set to
+ // whether that dependency should be exclusive.
+ void OnStreamSynSent(SpdyStreamId id,
+ RequestPriority priority,
+ SpdyStreamId* dependent_stream_id,
+ bool* exclusive);
+
+ void OnStreamDestruction(SpdyStreamId id);
+
+ private:
+ bool enabled_;
+
+ // The requirements for the internal data structure for this class are:
+ // a) Constant time insertion of entries at the end of the list,
+ // b) Fast removal of any entry based on its id.
+ // c) Constant time lookup of the entry at the end of the list.
+ // std::list would satisfy (a) & (c), but some form of map is
+ // needed for (b). The priority must be included in the map
+ // entries so that deletion can determine which list in id_priority_lists_
+ // to erase from.
Bence 2016/03/10 20:21:14 Makes sense. Too bad you have multiple lists and
Randy Smith (Not in Mondays) 2016/03/16 19:38:10 Acknowledged.
+ using IdList = std::list<std::pair<SpdyStreamId, net::RequestPriority>>;
+ using EntryMap = std::map<SpdyStreamId, IdList::iterator>;
+
+ IdList id_priority_lists_[NUM_PRIORITIES];
+
+ // Tracks the location of an id anywhere in the above vector of lists.
+ // Iterators to list elements remain valid until those particular elements
+ // are erased.
+ EntryMap entry_by_stream_id_;
+};
+
+} // namespace net
+
+#endif // NET_HTTP2_PRIORITY_DEPENDENCIES_H_

Powered by Google App Engine
This is Rietveld 408576698