Have MimeSniffingResourceHandler call WillStartRequest on new Handlers.

Have MimeSniffingResourceHandler call WillStartRequest on new Handlers.

Previously, when swapping in a new ResourceHandler, this method was not
being called on them.  This, combined with another bug, was resulting
in crashes.

Could have the InterceptingResourceHandler invoke this function instead,
the two reasons for having the MimeSniffingResourceHandler do it instead
are that it already implements ResourceController and ties it to a
DoLoop, and that there's a symmetry in bringing the new ResourceHandler
to the same state the old one was in before swapping it in. One the down
side, both the InterceptingResourceHandler and the
MimeSniffingResourceHandler share the responsibilty for handling sending
data to both the old and new handlers for the transfer, though this
was already the previous state as well, just to a lesser extent.

BUG=640545

[mmenke, 2016-09-23 02:47:23]

The CQ bit was checked by mmenke@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-23 02:47:44]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-23 02:58:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-23 02:58:36]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[mmenke, 2016-09-23 21:54:16]

The CQ bit was checked by mmenke@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-23 21:54:42]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-23 22:29:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-23 22:29:07]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[mmenke, 2016-09-26 20:42:05]

The CQ bit was checked by mmenke@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-26 20:42:33]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[mmenke, 2016-09-26 21:03:19]

mmenke@chromium.org changed reviewers:
+ asanka@chromium.org

[mmenke, 2016-09-26 21:03:19]

I want to land the other CL before this one, out of paranoia (Duplicate
throttles just seems bad enough that I don't want to make things more
complicated there).

I put the call in MimeSniffingResourceHandler for two reasons:
1)  It's already a ResourceController with a DoLoop, so it was simpler.
2)  If OnWillStart cancels through the ResourceController, we could end up
telling a ResourceController whose turn it currently is not to cancel the
request.  Sure we do that elsewhere, just doesn't seem like a great idea.

[mmenke, 2016-09-26 21:04:25]

On 2016/09/26 21:03:19, mmenke wrote:
> I want to land the other CL before this one, out of paranoia (Duplicate
> throttles just seems bad enough that I don't want to make things more
> complicated there).
> 
> I put the call in MimeSniffingResourceHandler for two reasons:
> 1)  It's already a ResourceController with a DoLoop, so it was simpler.
> 2)  If OnWillStart cancels through the ResourceController, we could end up
> telling a ResourceController whose turn it currently is not to cancel the
> request.  Sure we do that elsewhere, just doesn't seem like a great idea.

Hrm...Actually, suppose 2) isn't an issue.  It's still easier, though.  :)

[commit-bot: I haz the power, 2016-09-26 21:13:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-26 21:13:14]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[mmenke, 2016-09-26 22:22:30]

On 2016/09/26 21:04:25, mmenke wrote:
> On 2016/09/26 21:03:19, mmenke wrote:
> > I want to land the other CL before this one, out of paranoia (Duplicate
> > throttles just seems bad enough that I don't want to make things more
> > complicated there).
> > 
> > I put the call in MimeSniffingResourceHandler for two reasons:
> > 1)  It's already a ResourceController with a DoLoop, so it was simpler.
> > 2)  If OnWillStart cancels through the ResourceController, we could end up
> > telling a ResourceController whose turn it currently is not to cancel the
> > request.  Sure we do that elsewhere, just doesn't seem like a great idea.
> 
> Hrm...Actually, suppose 2) isn't an issue.  It's still easier, though.  :)

Oops, hold off on this one - a test expects the navigation to be cancelled, and
then the prompt to appear, and this CL flips the order.  I'll need to either fix
the test, or rework the code (Probably the latter).

[mmenke, 2016-09-27 17:34:56]

On 2016/09/26 22:22:30, mmenke wrote:
> On 2016/09/26 21:04:25, mmenke wrote:
> > On 2016/09/26 21:03:19, mmenke wrote:
> > > I want to land the other CL before this one, out of paranoia (Duplicate
> > > throttles just seems bad enough that I don't want to make things more
> > > complicated there).
> > > 
> > > I put the call in MimeSniffingResourceHandler for two reasons:
> > > 1)  It's already a ResourceController with a DoLoop, so it was simpler.
> > > 2)  If OnWillStart cancels through the ResourceController, we could end up
> > > telling a ResourceController whose turn it currently is not to cancel the
> > > request.  Sure we do that elsewhere, just doesn't seem like a great idea.
> > 
> > Hrm...Actually, suppose 2) isn't an issue.  It's still easier, though.  :)
> 
> Oops, hold off on this one - a test expects the navigation to be cancelled,
and
> then the prompt to appear, and this CL flips the order.  I'll need to either
fix
> the test, or rework the code (Probably the latter).

I'm going to put this CL off to the side, for now - doesn't seem urgent, and
there are regressions to investigate.  Will file a new bug for this.

[mmenke, 2016-09-27 17:41:40]

Description was changed from

==========
Have MimeSniffingResourceHandler call WillStartRequest on new Handlers.

Previously, when swapping in a new ResourceHandler, this method was not
being called on them.  This, combined with another bug, was resulting
in crashes.

Could have the InterceptingResourceHandler invoke this function instead,
the two reasons for having the MimeSniffingResourceHandler do it instead
are that it already implements ResourceController and ties it to a
DoLoop, and that there's a symmetry in bringing the new ResourceHandler
to the same state the old one was in before swapping it in. One the down
side, both the InterceptingResourceHandler and the
MimeSniffingResourceHandler share the responsibilty for handling sending
data to both the old and new handlers for the transfer, though this
was already the previous state as well, just to a lesser extent.

BUG=640545
==========

to

==========
Have MimeSniffingResourceHandler call WillStartRequest on new Handlers.

Previously, when swapping in a new ResourceHandler, this method was not
being called on them.  This, combined with another bug, was resulting
in crashes.

Could have the InterceptingResourceHandler invoke this function instead,
the two reasons for having the MimeSniffingResourceHandler do it instead
are that it already implements ResourceController and ties it to a
DoLoop, and that there's a symmetry in bringing the new ResourceHandler
to the same state the old one was in before swapping it in. One the down
side, both the InterceptingResourceHandler and the
MimeSniffingResourceHandler share the responsibilty for handling sending
data to both the old and new handlers for the transfer, though this
was already the previous state as well, just to a lesser extent.

BUG=640545
==========

[mmenke, 2016-09-27 17:41:40]

mmenke@chromium.org changed reviewers:
- asanka@chromium.org