New issue
Advanced search Search tips

Issue 787891 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug
Proj-Servicification

Blocking:
issue 598073



Sign in to add a comment

Get rid of WebContentsObserver::DidGetRedirectForResourceRequest

Project Member Reported by jam@chromium.org, Nov 22 2017

Issue description

Similar to  bug 783981 , we should get rid of this notification and replace it with DidRedirectNavigation. It looks like all the listeners only care about frame requests anyways. That way it'll work for network service path.
 

Comment 1 by jam@chromium.org, Nov 22 2017

This will fix
-WebViewTests/WebViewTest.Shim_TestLoadStartLoadRedirect/0
-WebViewTests/WebViewTest.Shim_TestLoadStartLoadRedirect/1

and partially
-WebViewTests/WebViewTest.Shim_TestWebRequestAPIWithHeaders/0
-WebViewTests/WebViewTest.Shim_TestWebRequestAPIWithHeaders/1
(which also depend on web request API)

Comment 2 by jam@chromium.org, Nov 22 2017

also
WebViewAPITest.TestWebRequestAPIWithHeaders
WebViewAPITest.TestLoadStartLoadRedirect
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 22 2017

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

commit 144e7724a1b8c58e51e556c8fdf55ccaac26d2f1
Author: John Abd-El-Malek <jam@chromium.org>
Date: Wed Nov 22 21:12:40 2017

Stop using deprecated WebContentsObserver::DidGetRedirectForResourceRequest in webview code.

This will go away in favor of DidRedirectNavigation, since the former doesn't work with network service.

BUG= 787891 

Change-Id: I3c872f36f870e3ebb71eebc496f400980637ffe9
Reviewed-on: https://chromium-review.googlesource.com/786396
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518747}
[modify] https://crrev.com/144e7724a1b8c58e51e556c8fdf55ccaac26d2f1/extensions/browser/guest_view/web_view/web_view_guest.cc
[modify] https://crrev.com/144e7724a1b8c58e51e556c8fdf55ccaac26d2f1/extensions/browser/guest_view/web_view/web_view_guest.h

Project Member

Comment 4 by bugdroid1@chromium.org, Nov 22 2017

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

commit cb599d07711b26abf0abef5b5fd1589d8fcfbe16
Author: John Abd-El-Malek <jam@chromium.org>
Date: Wed Nov 22 21:36:00 2017

Stop using deprecated WebContentsObserver::DidGetRedirectForResourceRequest in prerender code.

This will go away in favor of DidRedirectNavigation, since the former doesn't work with network service.

BUG= 787891 

Change-Id: I019001b77fe694c835d6348830309f917152e6d7
Reviewed-on: https://chromium-review.googlesource.com/786394
Reviewed-by: Matt Menke <mmenke@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518756}
[modify] https://crrev.com/cb599d07711b26abf0abef5b5fd1589d8fcfbe16/chrome/browser/prerender/prerender_contents.cc
[modify] https://crrev.com/cb599d07711b26abf0abef5b5fd1589d8fcfbe16/chrome/browser/prerender/prerender_contents.h
[modify] https://crrev.com/cb599d07711b26abf0abef5b5fd1589d8fcfbe16/chrome/browser/prerender/prerender_tab_helper.cc
[modify] https://crrev.com/cb599d07711b26abf0abef5b5fd1589d8fcfbe16/chrome/browser/prerender/prerender_tab_helper.h

Project Member

Comment 5 by bugdroid1@chromium.org, Nov 23 2017

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

commit b0d81669d862749d7ae369bdd390a64fa2cfdc17
Author: John Abd-El-Malek <jam@chromium.org>
Date: Thu Nov 23 18:54:41 2017

Stop using deprecated WebContentsObserver::DidGetRedirectForResourceRequest in identity code.

This will go away in favor of DidRedirectNavigation, since the former doesn't work with network service.

BUG= 787891 

Change-Id: Icd94e27b601088c837c28df8d7f6d2f604675ecd
Reviewed-on: https://chromium-review.googlesource.com/786273
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518996}
[modify] https://crrev.com/b0d81669d862749d7ae369bdd390a64fa2cfdc17/chrome/browser/extensions/api/identity/web_auth_flow.cc
[modify] https://crrev.com/b0d81669d862749d7ae369bdd390a64fa2cfdc17/chrome/browser/extensions/api/identity/web_auth_flow.h

Project Member

Comment 6 by bugdroid1@chromium.org, Nov 27 2017

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

commit ea9de63c8bc0762dfebfffbff66c7bb1299f4ca2
Author: John Abd-El-Malek <jam@chromium.org>
Date: Mon Nov 27 19:18:07 2017

Remove WebContentsObserver::DidGetRedirectForResourceRequest as it's not used anymore.

This notification wasn't compatible with the network service, and was redundant with DidRedirectNavigation.

Bug:  787891 
Change-Id: I9b567fa0df00ff7a2c5515120ca15b3ee0a93d41
Reviewed-on: https://chromium-review.googlesource.com/786320
Reviewed-by: Matt Menke <mmenke@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519360}
[modify] https://crrev.com/ea9de63c8bc0762dfebfffbff66c7bb1299f4ca2/chrome/browser/captive_portal/captive_portal_tab_helper.cc
[modify] https://crrev.com/ea9de63c8bc0762dfebfffbff66c7bb1299f4ca2/chrome/browser/chrome_security_exploit_browsertest.cc
[modify] https://crrev.com/ea9de63c8bc0762dfebfffbff66c7bb1299f4ca2/chrome/browser/extensions/api/identity/web_auth_flow.cc
[modify] https://crrev.com/ea9de63c8bc0762dfebfffbff66c7bb1299f4ca2/chrome/browser/extensions/api/web_navigation/web_navigation_api.cc
[modify] https://crrev.com/ea9de63c8bc0762dfebfffbff66c7bb1299f4ca2/chrome/browser/prerender/prerender_contents.cc
[modify] https://crrev.com/ea9de63c8bc0762dfebfffbff66c7bb1299f4ca2/chrome/browser/prerender/prerender_manager.cc
[modify] https://crrev.com/ea9de63c8bc0762dfebfffbff66c7bb1299f4ca2/chrome/browser/prerender/prerender_tab_helper.cc
[modify] https://crrev.com/ea9de63c8bc0762dfebfffbff66c7bb1299f4ca2/chrome/browser/safe_browsing/safe_browsing_navigation_observer.cc
[modify] https://crrev.com/ea9de63c8bc0762dfebfffbff66c7bb1299f4ca2/content/browser/loader/loader_delegate.h
[modify] https://crrev.com/ea9de63c8bc0762dfebfffbff66c7bb1299f4ca2/content/browser/loader/resource_dispatcher_host_impl.cc
[modify] https://crrev.com/ea9de63c8bc0762dfebfffbff66c7bb1299f4ca2/content/browser/loader/resource_dispatcher_host_unittest.cc
[modify] https://crrev.com/ea9de63c8bc0762dfebfffbff66c7bb1299f4ca2/content/browser/loader_delegate_impl.cc
[modify] https://crrev.com/ea9de63c8bc0762dfebfffbff66c7bb1299f4ca2/content/browser/loader_delegate_impl.h
[modify] https://crrev.com/ea9de63c8bc0762dfebfffbff66c7bb1299f4ca2/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/ea9de63c8bc0762dfebfffbff66c7bb1299f4ca2/content/browser/web_contents/web_contents_impl.h
[modify] https://crrev.com/ea9de63c8bc0762dfebfffbff66c7bb1299f4ca2/content/browser/web_contents/web_contents_impl_unittest.cc
[modify] https://crrev.com/ea9de63c8bc0762dfebfffbff66c7bb1299f4ca2/content/public/browser/resource_request_details.cc
[modify] https://crrev.com/ea9de63c8bc0762dfebfffbff66c7bb1299f4ca2/content/public/browser/resource_request_details.h
[modify] https://crrev.com/ea9de63c8bc0762dfebfffbff66c7bb1299f4ca2/content/public/browser/web_contents_observer.h
[modify] https://crrev.com/ea9de63c8bc0762dfebfffbff66c7bb1299f4ca2/content/test/browser_test_utils_browsertest.cc

Comment 7 by jam@chromium.org, Nov 27 2017

Status: Fixed (was: Started)

Sign in to add a comment