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

Issue 645983 link

Starred by 0 users
Status: Fixed
Owner:
█ sgu...@chromium.org
Last visit > 30 days ago
Closed: Mar 2017
Cc:
clamy@chromium.org
boliu@chromium.org
█ nasko@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug
Proj-Servicification

Blocking:
issue 673806



Sign in to add a comment

Fix Android WebView tests.

Project Member Reported by jam@chromium.org, Sep 12 2016 
Fix Android WebView tests.

One major issue:
Currently it has a map that's keyed by RenderFrame IDs. That doesn't work with navigation requests, which only have a frame tree node id. Probably we'll need to have two maps.

Comment 1 by boliu@chromium.org, Sep 12 2016

Components: Mobile>WebView

Comment 2 Deleted

Comment 3 Deleted

Comment 4 Deleted

Comment 5 Deleted

Comment 6 Deleted

Comment 7 by sgu...@chromium.org, Sep 13 2016 

I think one of the idea jam brought up was why we could not get rid of AwContentsIoThreadClientImpl.  Jam is this what you will be looking at as the next step, or did I misunderstand?

Comment 8 Deleted

Comment 9 by jam@chromium.org, Sep 14 2016 

@sgurun: correct, I started looking at that but got sidetracked. I'm listing all the ways it's used to see which ones can be removed.

Comment 10 Deleted

Comment 11 by jam@chromium.org, Sep 19 2016 

here's a braindump from the methods of AWContentsIOThreadClient from when I looked last week (note this was from reading cs/, so it might be off in some places!)

AwCookieAccessPolicy::GetShouldAcceptThirdPartyCookies(
  impl calls Java_AwContentsIoThreadClient_shouldAcceptThirdPartyCookies
    implemented in
      AwContents.java's's IoThreadClientImpl: uses mSettings which is thread safe per Bo
      AwServiceWorkerController.java shouldAcceptThirdPartyCookies: returns false
  -this class is called by
    -net::NetworkDelegate: this is a request, so navigation requests will come here
    -content::ContentBrowserClient: we always have RFID


AwRequestInterceptor
  -looks like that creates an async urlrequestjob


AwNetworkDelegate::OnHeadersReceived
  -can be async

IoThreadClientThrottle::WillStartRequest
  seems like it's async

IoThreadClientThrottle::ShouldBlockRequest
  needs sync response


AwResourceDispatcherHostDelegate::RequestComplete
  async


AwResourceDispatcherHostDelegate::DownloadStarting 
  async


AwResourceDispatcherHostDelegate::OnResponseStarted
  async

Comment 12 by sgu...@chromium.org, Sep 19 2016

Owner: sgu...@chromium.org

Comment 13 by sgu...@chromium.org, Sep 20 2016 

As the next goal I will try to move whatever I can reasonably move out of AwContentsIoThreadClientImpl. Then Jam will take over to add a map. We want the use case for this map to be as narrow as possible.

Comment 14 by sgu...@chromium.org, Sep 20 2016 

Note that not everything can be moved out, because things like accessing the third party cookie setting still has to happen on IO thread and thus needs a secondary map.

Comment 15 by boliu@chromium.org, Sep 20 2016 

Moved out of AwContentsIoThreadClientImpl to? Existing/new interfaces on the UI thread? Need to be careful here in case any callback to apps changes order and break compatibility.

Comment 16 by sgu...@chromium.org, Sep 20 2016 

haven't decided yet. yes, order is important.

Comment 17 by jam@chromium.org, Sep 20 2016

Blocking: 504347

Comment 18 by sgu...@chromium.org, Oct 20 2016 

Moved NewDownload out of AwContentsIoThreadClientImpl to AwContentsClientBridge in https://chromiumcodereview.appspot.com/2425423004/.
Project Member

Comment 19 by bugdroid1@chromium.org, Oct 21 2016 

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

commit 021942e368d71dea0a25b339db6cab32b12a641a
Author: sgurun <sgurun@chromium.org>
Date: Fri Oct 21 16:38:06 2016

Use GetWebContentsGetterForRequest for compatibility with PlzNavigate

After plznavigate, GetChildId/GetRenderFrameID will return -1. Instead,
use GetWebContentsGetterForRequest.

BUG= 645983 

Review-Url: https://chromiumcodereview.appspot.com/2438913002
Cr-Commit-Position: refs/heads/master@{#426809}

[modify] https://crrev.com/021942e368d71dea0a25b339db6cab32b12a641a/android_webview/browser/renderer_host/aw_resource_dispatcher_host_delegate.cc
Project Member

Comment 20 by bugdroid1@chromium.org, Oct 21 2016 

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

commit a0039eafcbc9329d0559e5ac2247501c7f522430
Author: sgurun <sgurun@chromium.org>
Date: Fri Oct 21 21:47:17 2016

Move NewLoginRequest plumbing to AwContentsClientBridge

This is the continuation of the work described in https://codereview.chromium.org/2425423004/. This moves NewLoginRequest out of AwContentsIoThreadClient.

BUG= 645983 

Review-Url: https://chromiumcodereview.appspot.com/2437423002
Cr-Commit-Position: refs/heads/master@{#426898}

[modify] https://crrev.com/a0039eafcbc9329d0559e5ac2247501c7f522430/android_webview/browser/aw_contents_client_bridge_base.cc
[modify] https://crrev.com/a0039eafcbc9329d0559e5ac2247501c7f522430/android_webview/browser/aw_contents_client_bridge_base.h
[modify] https://crrev.com/a0039eafcbc9329d0559e5ac2247501c7f522430/android_webview/browser/aw_contents_io_thread_client.h
[modify] https://crrev.com/a0039eafcbc9329d0559e5ac2247501c7f522430/android_webview/browser/renderer_host/aw_resource_dispatcher_host_delegate.cc
[modify] https://crrev.com/a0039eafcbc9329d0559e5ac2247501c7f522430/android_webview/java/src/org/chromium/android_webview/AwContents.java
[modify] https://crrev.com/a0039eafcbc9329d0559e5ac2247501c7f522430/android_webview/java/src/org/chromium/android_webview/AwContentsClientBridge.java
[modify] https://crrev.com/a0039eafcbc9329d0559e5ac2247501c7f522430/android_webview/java/src/org/chromium/android_webview/AwContentsIoThreadClient.java
[modify] https://crrev.com/a0039eafcbc9329d0559e5ac2247501c7f522430/android_webview/java/src/org/chromium/android_webview/AwServiceWorkerController.java
[modify] https://crrev.com/a0039eafcbc9329d0559e5ac2247501c7f522430/android_webview/native/aw_contents_client_bridge.cc
[modify] https://crrev.com/a0039eafcbc9329d0559e5ac2247501c7f522430/android_webview/native/aw_contents_client_bridge.h
[modify] https://crrev.com/a0039eafcbc9329d0559e5ac2247501c7f522430/android_webview/native/aw_contents_io_thread_client_impl.cc
[modify] https://crrev.com/a0039eafcbc9329d0559e5ac2247501c7f522430/android_webview/native/aw_contents_io_thread_client_impl.h
Project Member

Comment 21 by bugdroid1@chromium.org, Dec 9 2016 

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

commit f199141ef70d76697647830f498b25706c0b0d0a
Author: sgurun <sgurun@chromium.org>
Date: Fri Dec 09 23:26:35 2016

Move onReceivedError and onReceivedHttpError out of AwContentsIoThreadClientImpl

Move onReceivedError and onReceivedHttpError out of AwContentsIoThreadClientImpl to AwContentsClientBridge. This is a continuation of the work started in
https://codereview.chromium.org/2425423004/ and is the third patch.

BUG= 645983 

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

[modify] https://crrev.com/f199141ef70d76697647830f498b25706c0b0d0a/android_webview/BUILD.gn
[modify] https://crrev.com/f199141ef70d76697647830f498b25706c0b0d0a/android_webview/browser/aw_contents_client_bridge_base.h
[modify] https://crrev.com/f199141ef70d76697647830f498b25706c0b0d0a/android_webview/browser/aw_contents_io_thread_client.h
[modify] https://crrev.com/f199141ef70d76697647830f498b25706c0b0d0a/android_webview/browser/net/aw_network_delegate.cc
[add] https://crrev.com/f199141ef70d76697647830f498b25706c0b0d0a/android_webview/browser/net/aw_web_resource_request.cc
[add] https://crrev.com/f199141ef70d76697647830f498b25706c0b0d0a/android_webview/browser/net/aw_web_resource_request.h
[modify] https://crrev.com/f199141ef70d76697647830f498b25706c0b0d0a/android_webview/browser/renderer_host/aw_resource_dispatcher_host_delegate.cc
[modify] https://crrev.com/f199141ef70d76697647830f498b25706c0b0d0a/android_webview/java/src/org/chromium/android_webview/AwContents.java
[modify] https://crrev.com/f199141ef70d76697647830f498b25706c0b0d0a/android_webview/java/src/org/chromium/android_webview/AwContentsClientBridge.java
[modify] https://crrev.com/f199141ef70d76697647830f498b25706c0b0d0a/android_webview/java/src/org/chromium/android_webview/AwContentsIoThreadClient.java
[modify] https://crrev.com/f199141ef70d76697647830f498b25706c0b0d0a/android_webview/java/src/org/chromium/android_webview/AwServiceWorkerController.java
[modify] https://crrev.com/f199141ef70d76697647830f498b25706c0b0d0a/android_webview/native/aw_contents_client_bridge.cc
[modify] https://crrev.com/f199141ef70d76697647830f498b25706c0b0d0a/android_webview/native/aw_contents_client_bridge.h
[modify] https://crrev.com/f199141ef70d76697647830f498b25706c0b0d0a/android_webview/native/aw_contents_io_thread_client_impl.cc
[modify] https://crrev.com/f199141ef70d76697647830f498b25706c0b0d0a/android_webview/native/aw_contents_io_thread_client_impl.h
Project Member

Comment 22 by bugdroid1@chromium.org, Dec 13 2016 

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

commit 0781d318118277e85563bc57c6d2e19b505f2a17
Author: boliu <boliu@chromium.org>
Date: Tue Dec 13 18:00:26 2016

Revert of Move onReceivedError and onReceivedHttpError out of AwContentsIoThreadClientImpl (patchset #7 id:120001 of https://codereview.chromium.org/2558223002/ )

Reason for revert:
BUG=673736

Original issue's description:
> Move onReceivedError and onReceivedHttpError out of AwContentsIoThreadClientImpl
>
> Move onReceivedError and onReceivedHttpError out of AwContentsIoThreadClientImpl to AwContentsClientBridge. This is a continuation of the work started in
> https://codereview.chromium.org/2425423004/ and is the third patch.
>
> BUG= 645983 
>
> Committed: https://crrev.com/f199141ef70d76697647830f498b25706c0b0d0a
> Cr-Commit-Position: refs/heads/master@{#437688}

TBR=sgurun@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 645983 

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

[modify] https://crrev.com/0781d318118277e85563bc57c6d2e19b505f2a17/android_webview/BUILD.gn
[modify] https://crrev.com/0781d318118277e85563bc57c6d2e19b505f2a17/android_webview/browser/aw_contents_client_bridge_base.h
[modify] https://crrev.com/0781d318118277e85563bc57c6d2e19b505f2a17/android_webview/browser/aw_contents_io_thread_client.h
[modify] https://crrev.com/0781d318118277e85563bc57c6d2e19b505f2a17/android_webview/browser/net/aw_network_delegate.cc
[delete] https://crrev.com/689d5629cf7d5d23897fb99b847eaa0999289867/android_webview/browser/net/aw_web_resource_request.cc
[delete] https://crrev.com/689d5629cf7d5d23897fb99b847eaa0999289867/android_webview/browser/net/aw_web_resource_request.h
[modify] https://crrev.com/0781d318118277e85563bc57c6d2e19b505f2a17/android_webview/browser/renderer_host/aw_resource_dispatcher_host_delegate.cc
[modify] https://crrev.com/0781d318118277e85563bc57c6d2e19b505f2a17/android_webview/java/src/org/chromium/android_webview/AwContents.java
[modify] https://crrev.com/0781d318118277e85563bc57c6d2e19b505f2a17/android_webview/java/src/org/chromium/android_webview/AwContentsClientBridge.java
[modify] https://crrev.com/0781d318118277e85563bc57c6d2e19b505f2a17/android_webview/java/src/org/chromium/android_webview/AwContentsIoThreadClient.java
[modify] https://crrev.com/0781d318118277e85563bc57c6d2e19b505f2a17/android_webview/java/src/org/chromium/android_webview/AwServiceWorkerController.java
[modify] https://crrev.com/0781d318118277e85563bc57c6d2e19b505f2a17/android_webview/native/aw_contents_client_bridge.cc
[modify] https://crrev.com/0781d318118277e85563bc57c6d2e19b505f2a17/android_webview/native/aw_contents_client_bridge.h
[modify] https://crrev.com/0781d318118277e85563bc57c6d2e19b505f2a17/android_webview/native/aw_contents_io_thread_client_impl.cc
[modify] https://crrev.com/0781d318118277e85563bc57c6d2e19b505f2a17/android_webview/native/aw_contents_io_thread_client_impl.h

Comment 23 by jam@chromium.org, Dec 14 2016

Blocking: -504347 673806
Labels: Proj-PlzNavigate-Blocking

Comment 24 by jam@chromium.org, Dec 14 2016

Status: Started (was: Available)
Project Member

Comment 25 by bugdroid1@chromium.org, Dec 15 2016 

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

commit a982815efb09df02eac3c2f716360d2420c3c94b
Author: sgurun <sgurun@chromium.org>
Date: Thu Dec 15 19:25:18 2016

Reland Move onReceivedError and onReceivedHttpError out of AwContentsIoThreadClientImpl

This was reverted due to crbug.com/673736. Fixed.

Move onReceivedError and onReceivedHttpError out of AwContentsIoThreadClientImpl to AwContentsClientBridge. This is a continuation of the work started in
https://codereview.chromium.org/2425423004/ and is the third patch.

BUG= 645983 

Committed: https://crrev.com/f199141ef70d76697647830f498b25706c0b0d0a
Cr-Commit-Position: refs/heads/master@{#437688}

patch from issue 2558223002 at patchset 120001 (http://crrev.com/2558223002#ps120001)

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

[modify] https://crrev.com/a982815efb09df02eac3c2f716360d2420c3c94b/android_webview/BUILD.gn
[modify] https://crrev.com/a982815efb09df02eac3c2f716360d2420c3c94b/android_webview/browser/aw_contents_client_bridge_base.h
[modify] https://crrev.com/a982815efb09df02eac3c2f716360d2420c3c94b/android_webview/browser/aw_contents_io_thread_client.h
[modify] https://crrev.com/a982815efb09df02eac3c2f716360d2420c3c94b/android_webview/browser/net/aw_network_delegate.cc
[add] https://crrev.com/a982815efb09df02eac3c2f716360d2420c3c94b/android_webview/browser/net/aw_web_resource_request.cc
[add] https://crrev.com/a982815efb09df02eac3c2f716360d2420c3c94b/android_webview/browser/net/aw_web_resource_request.h
[modify] https://crrev.com/a982815efb09df02eac3c2f716360d2420c3c94b/android_webview/browser/renderer_host/aw_resource_dispatcher_host_delegate.cc
[modify] https://crrev.com/a982815efb09df02eac3c2f716360d2420c3c94b/android_webview/java/src/org/chromium/android_webview/AwContents.java
[modify] https://crrev.com/a982815efb09df02eac3c2f716360d2420c3c94b/android_webview/java/src/org/chromium/android_webview/AwContentsClientBridge.java
[modify] https://crrev.com/a982815efb09df02eac3c2f716360d2420c3c94b/android_webview/java/src/org/chromium/android_webview/AwContentsIoThreadClient.java
[modify] https://crrev.com/a982815efb09df02eac3c2f716360d2420c3c94b/android_webview/java/src/org/chromium/android_webview/AwServiceWorkerController.java
[modify] https://crrev.com/a982815efb09df02eac3c2f716360d2420c3c94b/android_webview/javatests/src/org/chromium/android_webview/test/AwServiceWorkerClientTest.java
[modify] https://crrev.com/a982815efb09df02eac3c2f716360d2420c3c94b/android_webview/native/aw_contents_client_bridge.cc
[modify] https://crrev.com/a982815efb09df02eac3c2f716360d2420c3c94b/android_webview/native/aw_contents_client_bridge.h
[modify] https://crrev.com/a982815efb09df02eac3c2f716360d2420c3c94b/android_webview/native/aw_contents_io_thread_client_impl.cc
[modify] https://crrev.com/a982815efb09df02eac3c2f716360d2420c3c94b/android_webview/native/aw_contents_io_thread_client_impl.h

Comment 26 by jam@chromium.org, Feb 13 2017 

After the WebContentsObserverProxy changes, it looks like the number of failures in android_webview_test_apk  dropped a bit (it used to be around 90). The latest try run with PlzNavigate enable has 77 failures there.

https://build.chromium.org/p/tryserver.chromium.android/builders/linux_android_rel_ng/builds/231186

Selim: will you have time to look at this soon? Thanks

Comment 27 by sgu...@chromium.org, Feb 13 2017 

this week, definitely.
Project Member

Comment 28 by bugdroid1@chromium.org, Mar 9 2017 

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

commit 339b2ea6f222dcdaa3036e6e47f8aa1300343f38
Author: jam <jam@chromium.org>
Date: Thu Mar 09 05:15:24 2017

Fix ResourceDispatcherHostDelegate::OnResponseStarted not getting called for navigation requests.

It was being called by AsyncResourceHandler which isn't used for navigation requests. Move the call to ResourceDispatcherHostImpl instead.

This fixes
org.chromium.android_webview.test.AwContentsClientAutoLoginTest_testAutoLoginOnGoogleCom
org.chromium.android_webview.test.AwContentsClientAutoLoginTest_testAutoLoginOnGoogleCom_with__--webview-sandboxed-renderer_
org.chromium.android_webview.test.AwContentsClientAutoLoginTest_testAutoLoginOnNonGoogle
org.chromium.android_webview.test.AwContentsClientAutoLoginTest_testAutoLoginOnNonGoogle_with__--webview-sandboxed-renderer_
org.chromium.android_webview.test.AwContentsClientAutoLoginTest_testAutoLoginWithNullAccount
org.chromium.android_webview.test.AwContentsClientAutoLoginTest_testAutoLoginWithNullAccount_with__--webview-sandboxed-renderer_
with PlzNavigate.

BUG= 645983 

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

[modify] https://crrev.com/339b2ea6f222dcdaa3036e6e47f8aa1300343f38/content/browser/loader/DEPS
[modify] https://crrev.com/339b2ea6f222dcdaa3036e6e47f8aa1300343f38/content/browser/loader/async_resource_handler.cc
[modify] https://crrev.com/339b2ea6f222dcdaa3036e6e47f8aa1300343f38/content/browser/loader/async_resource_handler_unittest.cc
[modify] https://crrev.com/339b2ea6f222dcdaa3036e6e47f8aa1300343f38/content/browser/loader/mojo_async_resource_handler.cc
[modify] https://crrev.com/339b2ea6f222dcdaa3036e6e47f8aa1300343f38/content/browser/loader/mojo_async_resource_handler_unittest.cc
[modify] https://crrev.com/339b2ea6f222dcdaa3036e6e47f8aa1300343f38/content/browser/loader/resource_dispatcher_host_impl.cc
[modify] https://crrev.com/339b2ea6f222dcdaa3036e6e47f8aa1300343f38/content/browser/loader/resource_dispatcher_host_impl.h
[modify] https://crrev.com/339b2ea6f222dcdaa3036e6e47f8aa1300343f38/content/browser/loader/resource_loader.cc
[modify] https://crrev.com/339b2ea6f222dcdaa3036e6e47f8aa1300343f38/content/browser/loader/resource_loader_delegate.h
[modify] https://crrev.com/339b2ea6f222dcdaa3036e6e47f8aa1300343f38/content/browser/loader/resource_loader_unittest.cc
Project Member

Comment 29 by bugdroid1@chromium.org, Mar 10 2017 

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

commit 0299edae682a3dd0c5f4ffd3fddde60b124695db
Author: jam <jam@chromium.org>
Date: Fri Mar 10 00:49:22 2017

Fix stopped navigations not having the correct error code in their navigation handle.

This fixes
ClientOnPageFinishedTest_testCalledOnCancelingProvisionalLoad
ClientOnPageFinishedTest_testCalledOnCancelingProvisionalLoad_with__--webview-sandboxed-renderer_
ClientOnPageFinishedTest_testNotCalledOnDomModificationWithJavascriptUrlAfterNonCommittedLoadFromApi
ClientOnPageFinishedTest_testNotCalledOnDomModificationWithJavascriptUrlAfterNonCommittedLoadFromApi_with__--webview-sandboxed-renderer_
ClientOnReceivedError2Test#testNotCalledOnStopLoading
ClientOnReceivedError2Test#testNotCalledOnStopLoading with {--webview-sandboxed-renderer}
with PlzNavigate.

BUG= 645983 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

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

[modify] https://crrev.com/0299edae682a3dd0c5f4ffd3fddde60b124695db/content/browser/frame_host/frame_tree_node.cc
Project Member

Comment 31 by bugdroid1@chromium.org, Mar 13 2017 

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

commit 761cdd9d8d97595aef3302c952cb73a63c3aa75f
Author: jam <jam@chromium.org>
Date: Mon Mar 13 17:08:52 2017

Fix the browser not being notified when a webview embedder handles a navigation.

This ended up causing browser code to not get the notification that a navigation completed.

This fixes
ClientOnPageFinishedTest#testCalledAfterRedirectedUrlIsOverridden
ClientOnPageFinishedTest#testCalledAfterRedirectedUrlIsOverridden with {--webview-sandboxed-renderer}
with PlzNavigate

BUG= 645983 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

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

[modify] https://crrev.com/761cdd9d8d97595aef3302c952cb73a63c3aa75f/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/761cdd9d8d97595aef3302c952cb73a63c3aa75f/content/browser/frame_host/render_frame_host_impl.h
[modify] https://crrev.com/761cdd9d8d97595aef3302c952cb73a63c3aa75f/content/common/frame_messages.h
[modify] https://crrev.com/761cdd9d8d97595aef3302c952cb73a63c3aa75f/content/renderer/render_frame_impl.cc
Project Member

Comment 32 by bugdroid1@chromium.org, Mar 13 2017 

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

commit 8a995f1eb09b00382fc071d347d59cc87c2544b2
Author: jam <jam@chromium.org>
Date: Mon Mar 13 20:10:35 2017

Fix shouldOverrideUrlLoading being called with the wrong is_redirect value with PlzNavigate.

The problem was that with PlzNavigate, when the navigation in the renderer happens we already have the full list of redirects. So the logic to determine if the first url was a redirect was failing. Fix that, and also remove an assert that legitimately happens when the embedder cancels a redirect.

This fixes
AwContentsClientShouldOverrideUrlLoadingTest_testCalledOn302Redirect
AwContentsClientShouldOverrideUrlLoadingTest_testCalledOn302Redirect_with__--webview-sandboxed-renderer_
AwContentsClientShouldOverrideUrlLoadingTest_testNullContentsClientWithServerRedirect
AwContentsClientShouldOverrideUrlLoadingTest_testNullContentsClientWithServerRedirect_with__--webview-sandboxed-renderer_
with PlzNavigate.

BUG= 645983 

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

[modify] https://crrev.com/8a995f1eb09b00382fc071d347d59cc87c2544b2/content/child/web_url_loader_impl.cc
[modify] https://crrev.com/8a995f1eb09b00382fc071d347d59cc87c2544b2/content/renderer/render_frame_impl.cc

Comment 33 by jam@chromium.org, Mar 13 2017

Description: Show this description

Comment 34 by jam@chromium.org, Mar 13 2017

Description: Show this description

Comment 35 by jam@chromium.org, Mar 13 2017

Summary: Fix Android WebView tests. (was: AwContentsIoThreadClientImpl needs to work with PlzNavigate)
Project Member

Comment 36 by bugdroid1@chromium.org, Mar 14 2017 

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

commit 8436163dfed3d0a9889099095bdd2971c2bdc1c6
Author: jam <jam@chromium.org>
Date: Tue Mar 14 02:17:13 2017

Fix AwContentsClientShouldInterceptRequestTest#testBaseUrlOfLoadDataSentInRefererHeader with PlzNavigate.

With PlzNavigate, data URLs are intercepted while without PlzNavigate they are not. So make the test logic handle both cases.

BUG= 645983 

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

[modify] https://crrev.com/8436163dfed3d0a9889099095bdd2971c2bdc1c6/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientShouldInterceptRequestTest.java
Project Member

Comment 37 by bugdroid1@chromium.org, Mar 14 2017 

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

commit 55e6fd485c6e8dd9ee7f69627ce1068cfb7c4e50
Author: jam <jam@chromium.org>
Date: Tue Mar 14 02:24:26 2017

Fix org.chromium.android_webview.test.AwSettingsTest#testUpdatingUserAgentWhileLoadingCausesReload with PlzNavigate.

The test is meant to verify that a reload occurs if the user agent override is set during a navigation. It was testing that by looking for two did-stop-navigation events, which is an implementation detail fo the old navigation code paths. With PlzNavigate, only one did-stop-navigation callback is fired. Change the test to look for a history entry with reload=true instead.

BUG= 645983 

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

[modify] https://crrev.com/55e6fd485c6e8dd9ee7f69627ce1068cfb7c4e50/android_webview/javatests/src/org/chromium/android_webview/test/AwSettingsTest.java
Project Member

Comment 38 by bugdroid1@chromium.org, Mar 14 2017 

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

commit 439e9ae5b8b0b3e7a2007ee017ac54fd4b8fefc6
Author: jam <jam@chromium.org>
Date: Tue Mar 14 03:05:54 2017

Fix user agent overridding with PlzNavigate.

This fixes
org.chromium.android_webview.test.AwSettingsTest#testUserAgentWithTestServer
with PlzNavigate.

BUG= 645983 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

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

[modify] https://crrev.com/439e9ae5b8b0b3e7a2007ee017ac54fd4b8fefc6/content/browser/frame_host/interstitial_page_impl.cc
[modify] https://crrev.com/439e9ae5b8b0b3e7a2007ee017ac54fd4b8fefc6/content/browser/frame_host/interstitial_page_impl.h
[modify] https://crrev.com/439e9ae5b8b0b3e7a2007ee017ac54fd4b8fefc6/content/browser/frame_host/navigation_request.cc
[modify] https://crrev.com/439e9ae5b8b0b3e7a2007ee017ac54fd4b8fefc6/content/browser/frame_host/navigator_delegate.h
Project Member

Comment 39 by bugdroid1@chromium.org, Mar 14 2017 

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

commit 87bd05ec8da9f4de31622dba5cb8817249d7c85b
Author: jam <jam@chromium.org>
Date: Tue Mar 14 19:10:25 2017

Ensure that if appcache is disabled for a page that navigation requests don't use it.

This fixes
AwSettingsTest#testAppCacheWithTwoViews
with PlzNavigate.

BUG= 645983 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

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

[modify] https://crrev.com/87bd05ec8da9f4de31622dba5cb8817249d7c85b/content/browser/frame_host/navigation_request.cc
Project Member

Comment 40 by bugdroid1@chromium.org, Mar 14 2017 

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

commit 75284b4387797789ea4d56447e1ab3d6f8091199
Author: jam <jam@chromium.org>
Date: Tue Mar 14 20:29:13 2017

Fix LoadDataWithBaseUrlTest#testloadDataWithBaseUrlCallsOnPageStarted with PlzNavigate.

Ensure that when loadDataWithBaseUrl is called, onPageStarted uses the base url for the data url.

BUG= 645983 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

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

[modify] https://crrev.com/75284b4387797789ea4d56447e1ab3d6f8091199/chrome/browser/ui/android/external_protocol_dialog_android.cc
[modify] https://crrev.com/75284b4387797789ea4d56447e1ab3d6f8091199/components/navigation_interception/intercept_navigation_throttle.cc
[modify] https://crrev.com/75284b4387797789ea4d56447e1ab3d6f8091199/components/navigation_interception/navigation_params.cc
[modify] https://crrev.com/75284b4387797789ea4d56447e1ab3d6f8091199/components/navigation_interception/navigation_params.h
[modify] https://crrev.com/75284b4387797789ea4d56447e1ab3d6f8091199/components/navigation_interception/navigation_params_android.cc
[modify] https://crrev.com/75284b4387797789ea4d56447e1ab3d6f8091199/content/browser/frame_host/navigation_handle_impl.cc
[modify] https://crrev.com/75284b4387797789ea4d56447e1ab3d6f8091199/content/browser/frame_host/navigation_handle_impl.h
[modify] https://crrev.com/75284b4387797789ea4d56447e1ab3d6f8091199/content/browser/frame_host/navigator_impl.cc
[modify] https://crrev.com/75284b4387797789ea4d56447e1ab3d6f8091199/content/public/browser/navigation_handle.h
Project Member

Comment 41 by bugdroid1@chromium.org, Mar 14 2017 

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

commit db15896bc5eaa54e21480834265cfd82bd6cb8a5
Author: jam <jam@chromium.org>
Date: Tue Mar 14 20:59:22 2017

Don't block blob URLs by webview URL filtering.

blob URLs are both used by PlzNavigate and sites that are whitelisted.

This fixes
PolicyUrlFilteringTest#testWhitelistedUrl
with PlzNavigate.

BUG= 645983 

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

[modify] https://crrev.com/db15896bc5eaa54e21480834265cfd82bd6cb8a5/android_webview/browser/net/aw_network_delegate.cc

Comment 42 by sgu...@chromium.org, Mar 15 2017

Status: Fixed (was: Started)

Comment 44 by laforge@google.com, Nov 7 2017

Components: Internals>Network>Service

Comment 45 by laforge@google.com, Nov 7 2017

Components: -Internals>Network>Service Internals>Services>Network
Apologies, applied the wrong component in bulk.
►

Sign in to add a comment