New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 650717 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

When swapping in a ResourceHandler, we don't call OnWillStart on it

Project Member Reported by mmenke@chromium.org, Sep 27 2016

Issue description

MimeSniffingResourceHandler / InterceptingResourceHandler can swap in a new ResourceHandler to replace part of the chain.  When this happens, they don't call OnWillStart on the new ResourceHandler.  This, along with another bug, lead to issue 640545.  This also means we don't send the new ResourceHandler download updates, if it's a standard AsyncResourceHandler, which seems potentially bad.

This has to be fixed carefully - if we want to keep behavior the same, we need to take care of the old ResourceHandler before we call OnWillStart on the new one.  If not, a test that relies on this will fail, and we may show the page loading while we show the save as dialog (Not positive of that).

More generally, the swapping mechanism / ResourceHandler layering mechanism is a mess, and we way want to think about cleaning it up.
 

Comment 1 by mmenke@chromium.org, Sep 27 2016

Oh, and my in-progress CL (Which I'm putting on hold for the moment), which does the wrong thing, in terms of ordering:  https://codereview.chromium.org/2366753002/

Comment 2 by mmenke@chromium.org, Sep 29 2016

Owner: mmenke@chromium.org
Status: Assigned (was: Available)
Looks like yhirano has done 90% of the work to get this working, I'll go ahead and hook up the last bits once his CL lands.
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 25 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/90cf7fc2ae2d522c9a0d5c9dee461f6b79277df9

commit 90cf7fc2ae2d522c9a0d5c9dee461f6b79277df9
Author: mmenke <mmenke@chromium.org>
Date: Mon Oct 24 23:59:27 2016

When InterceptingResourceHandler swaps in a handler, call OnWillStart.

The new ResourceHandler's OnWillStart was never being called, which
violates the ResourceHandler's API contract, and has caused at least one
crash previously (Though that fixed by another CL).

BUG= 650717 

Review-Url: https://codereview.chromium.org/2436163002
Cr-Commit-Position: refs/heads/master@{#427183}

[modify] https://crrev.com/90cf7fc2ae2d522c9a0d5c9dee461f6b79277df9/content/browser/loader/intercepting_resource_handler.cc
[modify] https://crrev.com/90cf7fc2ae2d522c9a0d5c9dee461f6b79277df9/content/browser/loader/intercepting_resource_handler.h
[modify] https://crrev.com/90cf7fc2ae2d522c9a0d5c9dee461f6b79277df9/content/browser/loader/intercepting_resource_handler_unittest.cc

Comment 4 by mmenke@chromium.org, Oct 25 2016

Status: Fixed (was: Assigned)

Sign in to add a comment