Skip to content

Commit

Permalink
Revert "Fix WebRequestProxyingURLLoaderFactory crash after redirect"
Browse files Browse the repository at this point in the history
This reverts commit 066fecf.

Reason for revert: This change had some unintended effects on the interaction of WebRequest and URLLoaderThrottles.

Original change's description:
> Fix WebRequestProxyingURLLoaderFactory crash after redirect
> 
> When OnReceiveRedirect() is called, we expect to not receive any more
> events for that request until FollowRedirect() has been called and the
> request is restarted with the new URL. We were not clearing the
> bindings in this case, so the URLLoader was staying alive and calling
> OnBeforeSendHeaders() on the redirected URL. This could then result in
> OnBeforeSendHeaders() being called twice, once for the original request
> which gets redirected in //net, and once for the request in the correct
> flow when it gets restarted after FollowRedirect().
> 
> I believe this is pretty similar to the issue described in
> http://crbug.com/882661, except this one has to do with normal
> redirects instead of webRequest generated redirects.
> 
> The existing redirect tests catch this issue when using the
> 'extraHeaders' spec, so I made a version of them that runs with that
> enabled.
> 
> Bug: 918761
> Change-Id: Ifa551400e85c13ea3a297fba02a05deb5ccb1821
> Reviewed-on: https://chromium-review.googlesource.com/c/1409915
> Reviewed-by: Ken Rockot <rockot@google.com>
> Commit-Queue: Clark DuVall <cduvall@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#622685}

TBR=rockot@google.com,cduvall@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 918761
Change-Id: Ie8449c6fd6c94722ef8eb618ea1a16d429b74bbb
Reviewed-on: https://chromium-review.googlesource.com/c/1443913
Reviewed-by: Clark DuVall <cduvall@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#627183}(cherry picked from commit 6406a9f)
Reviewed-on: https://chromium-review.googlesource.com/c/1444812
Cr-Commit-Position: refs/branch-heads/3683@{#54}
Cr-Branched-From: e510299-refs/heads/master@{#625896}
  • Loading branch information
clarkduvall authored and John Abd-El-Malek committed Jan 30, 2019
1 parent f506ced commit 105c463
Show file tree
Hide file tree
Showing 4 changed files with 346 additions and 362 deletions.
16 changes: 0 additions & 16 deletions chrome/browser/extensions/api/web_request/web_request_apitest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -556,14 +556,6 @@ IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest, WebRequestRedirects) {
<< message_;
}

IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest,
WebRequestRedirectsWithExtraHeaders) {
ASSERT_TRUE(StartEmbeddedTestServer());
ASSERT_TRUE(RunExtensionSubtestWithArg("webrequest", "test_redirects.html",
"useExtraHeaders"))
<< message_;
}

// Tests that redirects from secure to insecure don't send the referrer header.
IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest,
WebRequestRedirectsToInsecure) {
Expand Down Expand Up @@ -596,14 +588,6 @@ IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest,
<< message_;
}

IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest,
WebRequestSubresourceRedirectsWithExtraHeaders) {
ASSERT_TRUE(StartEmbeddedTestServer());
ASSERT_TRUE(RunExtensionSubtestWithArg(
"webrequest", "test_subresource_redirects.html", "useExtraHeaders"))
<< message_;
}

// Fails often on Windows dbg bots. http://crbug.com/177163
#if defined(OS_WIN)
#define MAYBE_WebRequestNewTab DISABLED_WebRequestNewTab
Expand Down
Loading

0 comments on commit 105c463

Please sign in to comment.