Add a status message "Waiting for extension Foo..." when there's a request

net/url_request/url_request.h

Index: net/url_request/url_request.h
diff --git a/net/url_request/url_request.h b/net/url_request/url_request.h
index a42fd3a2d8647bf4570bfbf5ada4c4610001357a..65bd26b7460c20b2d01975c2a74fb312dc7b206b 100644
--- a/net/url_request/url_request.h
+++ b/net/url_request/url_request.h
@@ -429,8 +429,13 @@ class NET_API URLRequest : NON_EXPORTED_BASE(public base::NonThreadSafe) {
     return extra_request_headers_;
   }
 
-  // Returns the current load state for the request.
-  LoadState GetLoadState() const;
+  // Returns the current load state for the request. |param| is an optional

[willchan no longer on Chromium, 2011/07/29 07:34:38]

I think you're working forward from the assumption that NetworkDelegate implies
extensions, and therefore when we're blocked on the delegate, we must always
want a parameter to attach to the LoadState. I'd argue this is not necessarily
true. There are possible future consumers who won't want this.

Therefore, I think that the network stack client should be storing this. Whoever
is calling GetLoadState() (RDH, right?) should, if the value indicates we're
blocked on the delegate, query the appropriate object (the delegate probably) to
figure out the reason for the blockage, using the URLRequest* as a lookup key.

I guess then we'd need to add a new method to the NetworkDelegate to allow
querying why a URLRequest* is blocked. I'm not sure I really like adding it to
NetworkDelegate since not all consumers need it and it burdens them with an
implementation, but I guess the alternative is unwieldy (having content/
subclass NetworkDelegate to introduce a new interface with a single added
method. I think this is the only alternative since content/ can't directly
depend on ChromeNetworkDelegate/ExtensionWebRequestAPI since those are chrome/
classes). So I guess I'm fine with adding an interface to NetworkDelegate for
this, with a comment indicating we should consider moving it out in the future.

[Matt Perry, 2011/07/29 21:30:49]

Agreed, I took care to avoid this assumption. That's why I made the param
optional. Extensions will use it, but other delegate consumers might not.
Doesn't this suffer from the same problem you point out above? :) Namely, the
assumption that "blocked on NetworkDelegate" implies "there's some string
explaining why".

+  // parameter describing details related to the load state. Not all load states
+  // have a parameter.
+  LoadStateWithParam GetLoadState() const;
+  void SetLoadStateParam(const std::string& param) {
+    load_state_param_ = param;
+  }
 
   // Returns the current upload progress in bytes.
   uint64 GetUploadProgress() const;
@@ -603,6 +608,11 @@ class NET_API URLRequest : NON_EXPORTED_BASE(public base::NonThreadSafe) {
   // cancel the request instead, call Cancel().
   void ContinueDespiteLastError();
 
+  // Called when the delegate blocks or unblocks this request when intercepting

[willchan no longer on Chromium, 2011/07/29 07:34:38]

These should be private. I think you made them public for URLRequestHttpJob to
call. I'm trying to tighten the interfaces. Any special interface needed to
access details in URLRequest from a job should be private in URLRequest, and be
protected in URLRequestJob. So URLRequestHttpJob should access these member
functions via its parent's protected member function.

[eroman, 2011/08/05 01:54:22]

Matt, can you address Will's comment? (Note that URLRequest job is friended).

[Matt Perry, 2011/08/08 22:14:33]

Done.

+  // certain requests.
+  void SetBlockedOnDelegate();
+  void SetUnblockedOnDelegate();
+
   // Used to specify the context (cookie store, cache) for this request.
   const URLRequestContext* context() const;
   void set_context(const URLRequestContext* context);
@@ -772,6 +782,14 @@ class NET_API URLRequest : NON_EXPORTED_BASE(public base::NonThreadSafe) {
   // A globally unique identifier for this request.
   const uint64 identifier_;
 
+  // True if this request is blocked waiting for the network delegate to resume
+  // it.
+  bool blocked_on_delegate_;
+
+  // An optional parameter that provides additional information about the load
+  // state. Only used with the LOAD_STATE_WAITING_FOR_DELEGATE state.
+  std::string load_state_param_;
+
   base::debug::LeakTracker<URLRequest> leak_tracker_;
 
   // Callback passed to the network delegate to notify us when a blocked request