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

Issue 640072 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

Fix inconsistency in implementation of AllowCrossRendererResourceLoad w.r.t. page transition logic

Project Member Reported by rob@robwu.nl, Aug 23 2016

Issue description

To fix a regression ( bug 633963 ), I submitted a very conservative patch (45ec6fecc32bff5541c237c98228122f3252902e).

I believe that the correct approach is https://codereview.chromium.org/2249423002/patch/20001/30001 for the reasons given in  https://crbug.com/633963#c14 .

If the cross-renderer navigation logic in the browser turns out to be correct, then the implementation in the renderer is wrong. In either case, the implementation differ and they should converge towards the same behavior.
 
Owner: rob@robwu.nl
Status: Assigned (was: Untriaged)
I'm going to assign this one to rob@, feel free to reassign.
Cc: rob@robwu.nl lfg@chromium.org
Owner: alex...@chromium.org
Status: Started (was: Assigned)
I've got a CL that implements Rob's suggestion in progress for this.
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 24 2017

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

commit a7d08ae5f1e081b95ae6f4e1f163cbd0be6dc075
Author: alexmos <alexmos@chromium.org>
Date: Fri Mar 24 01:23:01 2017

Fix webview-accessible resource checks in AllowCrossRendererResourceLoadHelper

AllowCrossRendererResourceLoadHelper is supposed to check whether a
resource that a webview guest tries to load is webview-accessible [1],
and denies the load if it isn't.  However, it currently has an
exception for all web-triggerable page transitions, blindly allowing
them through.  This allows a webview to happily navigate itself to
non-webview-accessible resources.

The page transition exception seems wrong, and rob@robwu.nl made a
detailed analysis of how we ended up with it in
 https://crbug.com/633963#c14 .  This CL removes it.  The fix was
originally proposed by Rob in  https://crbug.com/640072 .

[1] https://developer.chrome.com/apps/tags/webview#local_resources

BUG= 640072 , 691941 

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

[modify] https://crrev.com/a7d08ae5f1e081b95ae6f4e1f163cbd0be6dc075/chrome/browser/apps/guest_view/web_view_browsertest.cc
[modify] https://crrev.com/a7d08ae5f1e081b95ae6f4e1f163cbd0be6dc075/chrome/test/data/extensions/platform_apps/web_view/load_webview_accessible_resource/embedder.js
[modify] https://crrev.com/a7d08ae5f1e081b95ae6f4e1f163cbd0be6dc075/extensions/browser/url_request_util.cc

Status: Fixed (was: Started)
Project Member

Comment 5 by bugdroid1@chromium.org, Apr 5 2017

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

commit 968dd8975ab73d3abecd20c171bd4e3a6af7bf71
Author: alexmos <alexmos@chromium.org>
Date: Wed Apr 05 22:27:05 2017

Add a test for webview using a blob URL while at a webview-accessible resource.

This was written to try out a theory on  issue 706537 .  Nothing turned
out not to be broken, but I thought this test might be useful anyway.
There doesn't appear to be an existing test for this, and it
gets us some coverage for behavior on which the ChromeOS Files app
relies.

BUG= 640072 , 706537 

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

[modify] https://crrev.com/968dd8975ab73d3abecd20c171bd4e3a6af7bf71/chrome/browser/apps/guest_view/web_view_browsertest.cc
[modify] https://crrev.com/968dd8975ab73d3abecd20c171bd4e3a6af7bf71/chrome/test/data/extensions/platform_apps/web_view/load_webview_accessible_resource/embedder.js

Sign in to add a comment