Set blocked_on_delegate_ when a URLRequest is blocked during redirects

Set blocked_on_delegate_ when a URLRequest is blocked by
its NetworkDelegate during a redirect.

R=eroman@chromium.org
BUG=159197
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=167600

[eroman, 2012-11-03 00:57:58]

I see this in my review queue, but it is not yet published. Is it ready for my
eyes?

[mmenke, 2012-11-03 01:06:59]

On 2012/11/03 00:57:58, eroman wrote:
> I see this in my review queue, but it is not yet published. Is it ready for my
> eyes?

No.  Looks like I broke something in set 2.  Normally add the "R=" to a patch
set only after a successful try run, but wasn't expecting any issues with this
one.  :)

[mmenke, 2012-11-03 01:07:53]

On 2012/11/03 01:06:59, Matt Menke wrote:
> On 2012/11/03 00:57:58, eroman wrote:
> > I see this in my review queue, but it is not yet published. Is it ready for
my
> > eyes?
> 
> No.  Looks like I broke something in set 2.  Normally add the "R=" to a patch
> set only after a successful try run, but wasn't expecting any issues with this
> one.  :)

Oh, and I'll remove it from your queue until I figure it out.

[mmenke, 2012-11-07 17:25:25]

PTAL.

We still have no logging of ResourceHandlers/Throttlers that delay starting a
request, but at least they're pretty easy to spot in logs.  I'm thinking we
should have every such entity that chooses to delay a request result in adding
an event to the NetLog, but am leaving that for another CL.

https://codereview.chromium.org/11364057/diff/8008/net/url_request/url_reques...
File net/url_request/url_request.cc (right):

https://codereview.chromium.org/11364057/diff/8008/net/url_request/url_reques...
net/url_request/url_request.cc:321: if (blocked_on_delegate_ &&
!load_state_param_.empty()) {
This is need because the text we display on the lower left while a page load
expects a load_state_param_ in LOAD_STATE_WAITING_FOR_DELEGATE.  Could have
added a new string, but "Waiting on something-or-other" doesn't seem like it
would be terribly illuminating.

[mmenke, 2012-11-13 20:36:55]

eroman:  ping!

[eroman, 2012-11-13 20:42:02]

d'oh forgot. looking now!

[eroman, 2012-11-13 20:50:11]

lgtm

https://codereview.chromium.org/11364057/diff/8008/net/base/net_log_event_typ...
File net/base/net_log_event_type_list.h (right):

https://codereview.chromium.org/11364057/diff/8008/net/base/net_log_event_typ...
net/base/net_log_event_type_list.h:718: // Measures the time a net::URLRequest
is blocked waiting either the
typo: "waiting" -> "waiting for"

https://codereview.chromium.org/11364057/diff/8008/net/url_request/url_reques...
File net/url_request/url_request.cc (right):

https://codereview.chromium.org/11364057/diff/8008/net/url_request/url_reques...
net/url_request/url_request.cc:321: if (blocked_on_delegate_ &&
!load_state_param_.empty()) {
On 2012/11/07 17:25:25, Matt Menke wrote:
> This is need because the text we display on the lower left while a page load
> expects a load_state_param_ in LOAD_STATE_WAITING_FOR_DELEGATE.  Could have
> added a new string, but "Waiting on something-or-other" doesn't seem like it
> would be terribly illuminating.

Do you know what circumstances this happens, and what the text ends up being?

[mmenke, 2012-11-13 21:20:40]

On 2012/11/13 20:50:11, eroman wrote:
> lgtm
> 
>
https://codereview.chromium.org/11364057/diff/8008/net/base/net_log_event_typ...
> File net/base/net_log_event_type_list.h (right):
> 
>
https://codereview.chromium.org/11364057/diff/8008/net/base/net_log_event_typ...
> net/base/net_log_event_type_list.h:718: // Measures the time a net::URLRequest
> is blocked waiting either the
> typo: "waiting" -> "waiting for"
> 
>
https://codereview.chromium.org/11364057/diff/8008/net/url_request/url_reques...
> File net/url_request/url_request.cc (right):
> 
>
https://codereview.chromium.org/11364057/diff/8008/net/url_request/url_reques...
> net/url_request/url_request.cc:321: if (blocked_on_delegate_ &&
> !load_state_param_.empty()) {
> On 2012/11/07 17:25:25, Matt Menke wrote:
> > This is need because the text we display on the lower left while a page load
> > expects a load_state_param_ in LOAD_STATE_WAITING_FOR_DELEGATE.  Could have
> > added a new string, but "Waiting on something-or-other" doesn't seem like it
> > would be terribly illuminating.
> 
> Do you know what circumstances this happens, and what the text ends up being?

We only have the parameter set when an extension blocks us.

Other cases that I'm aware of:

Before requests start, we wait on SafeBrowsing and the extension system to start
up.  Since the request isn't yet started, we aren't technically in the blocked
on a delegate state, we return LOAD_STATE_IDLE, and don't display anything.

For downloads, we wait for a roundtrip to the UI thread to see if the
TabContentsSomethingOrOtherDelegateThingyHelperObserverHostViewClientDoohicky
objects before starting to read the body.  I'd guess we're in
LOAD_STATE_READING_RESPONSE at that point, and not displaying anything.

The big case I'm aware of (And that inspired this CL) is that redirects have to
wait on approval from another process.  If that other process never gives us
permission, people blame our poor innocent network stack for NPAPI Flash bugs. 
We're in STATE_IDLE at this point, and don't display anything.

[mmenke, 2012-11-14 01:38:09]

https://codereview.chromium.org/11364057/diff/8008/net/base/net_log_event_typ...
File net/base/net_log_event_type_list.h (right):

https://codereview.chromium.org/11364057/diff/8008/net/base/net_log_event_typ...
net/base/net_log_event_type_list.h:718: // Measures the time a net::URLRequest
is blocked waiting either the
On 2012/11/13 20:50:11, eroman wrote:
> typo: "waiting" -> "waiting for"

Done.

[commit-bot: I haz the power, 2012-11-14 01:38:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mmenke@chromium.org/11364057/10005

[commit-bot: I haz the power, 2012-11-14 05:04:31]

Change committed as 167600