[Net] Assert that URLRequests with LOAD_IGNORE_LIMITS have MAXIMUM_PRIORITY

net/url_request/url_request.cc

Index: net/url_request/url_request.cc
diff --git a/net/url_request/url_request.cc b/net/url_request/url_request.cc
index dae9d0e5339f860eb066eea982ef63061636968e..c1d3956dadfbcb71e9a109416ee40325dd944a1d 100644
--- a/net/url_request/url_request.cc
+++ b/net/url_request/url_request.cc
@@ -498,6 +498,20 @@ int URLRequest::GetResponseCode() const {
   return job_->GetResponseCode();
 }
 
+void URLRequest::SetLoadFlags(int flags) {
+  if ((load_flags_ & LOAD_IGNORE_LIMITS) != (flags & LOAD_IGNORE_LIMITS)) {
+    DCHECK(!job_);
+    DCHECK(flags & LOAD_IGNORE_LIMITS);

[mmenke, 2013/11/01 19:06:39]

So the IGNORE_LIMITS flag can be set, but not unset?  We DCHECK when we override
the priority due to MAX_LIMITS, anyways, so I think it's fine to clear the
flag...Isn't it?

I think the only case where we really have to DCHECK here is when setting the
flag, when there's either a job or priority is not MAXIMUM_PRIORITY.

[akalin, 2013/11/01 23:15:51]

It's fine, but the only use case doesn't ever clear the flag, right? I'd like to
keep the API simple until we actually need to relax the restriction.

+    DCHECK_EQ(priority_, MAXIMUM_PRIORITY);
+  }
+  load_flags_ = flags;
+
+  // This should be a no-op given the above DCHECKs, but do this
+  // anyway for release mode.
+  if ((load_flags_ & LOAD_IGNORE_LIMITS) != 0)
+    SetPriority(MAXIMUM_PRIORITY);
+}
+
 // static
 void URLRequest::SetDefaultCookiePolicyToBlock() {
   CHECK(!g_url_requests_started);
@@ -960,6 +974,15 @@ int64 URLRequest::GetExpectedContentSize() const {
 void URLRequest::SetPriority(RequestPriority priority) {
   DCHECK_GE(priority, MINIMUM_PRIORITY);
   DCHECK_LE(priority, MAXIMUM_PRIORITY);
+
+  if (((load_flags_ & LOAD_IGNORE_LIMITS) != 0) &&

[mmenke, 2013/11/01 19:06:39]

optional nit:  Think !(load_flags_ & LOAD_IGNORE_LIMITS) is more intuitive

[akalin, 2013/11/01 23:15:51]

Done.

+      (priority != MAXIMUM_PRIORITY)) {

[mmenke, 2013/11/01 19:06:39]

nit:  Parens not needed.

[akalin, 2013/11/01 23:15:51]

Not needed, but I think it's clearer.

+    NOTREACHED();
+    // Maintain the invariant that requests with IGNORE_LIMITS set
+    // have MAXIMUM_PRIORITY for release mode.
+    priority = MAXIMUM_PRIORITY;

[mmenke, 2013/11/01 19:06:39]

optional nit:  In this case, priority_ will now be priority, so we can just
return early.  Doesn't save us anything, just think it makes the logic simpler.

[akalin, 2013/11/01 23:15:51]

Done.

+  }
+
   if (priority_ == priority)
     return;