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

Issue 717644 link

Starred by 5 users

Issue metadata

Status: Verified
Owner:
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 0
Type: Bug-Regression



Sign in to add a comment

Opening Google Docs from Files does not work

Project Member Reported by w...@chromium.org, May 2 2017

Issue description

Chrome Version: 59.0.3071.25 (Official Build) dev (64-bit)
OS: ChromeOS Panther

What steps will reproduce the problem?
(1) Create a Google Doc and put it in My drive.
(2) Sign-in to ChromeOS.
(3) Open Files and navigate to Drive.
(4) Double-click on the Google Doc.

What is the expected result?

Expect that the doc is opened in a new tab.

What happens instead?

Instead a tab opens, with an externalfile://... URL, but the document does not load.

Issue also appears to affect ChromeOS 60.0.3084.0 (Canary).
 

Comment 1 by w...@chromium.org, May 2 2017

Components: Platform>Apps>FileManager>Drive
Couldn't repro on same build with Elm 
Cc: abod...@chromium.org sdantul...@chromium.org
This has been a lingering problem for a few weeks now on both my Acer C720 and C302CA.
Are there any logs you can attach here?
I filed a report from my C302 just a minute ago after attempting to load two Drive files via the files app.
Cc: hashimoto@chromium.org
Owner: hirono@chromium.org
Status: Assigned (was: Untriaged)
Labels: M-60

Comment 9 by w...@chromium.org, May 3 2017

Re #7: hirono@, it looks like you implemented the externalfile handler - is there anything you're aware of that might explain this, or any specific diagnostics to look for?

FWIW I could not spot anything in the Chrome event log corresponding to the failure.
Issue not repro'd on peppy, cave on M59 (9460.23.0, 59.0.3071.35)

Repro'd on Kevin M60 (9517.0.0, 60.0.3086.0)
Reported a few moments ago via C720: 

Google Chrome	59.0.3071.35 (Official Build) dev (64-bit)
Revision	0
Platform	9460.23.0 (Official Build) dev-channel peppy
Firmware Version	Google_Peppy.4389.117.0
Interesting that we can't repro on cave/ peppy internally on the same build. Wondering if there are some extensions installed that are causing it.
Even with all extensions removed, it happens.  It loads the externalfile URL and the status bar in the lower left says "Waiting for docs.google.com" and then goes away without loading anything.
Issue is not related to specific user account.

M60: I tried repro on elm, kevin and link with same user account.
Issue repro'd on all of them on first time sign-in

After sign-out and sign in to same user account, issue is not reproduced.

M59: I am unable to repro so far
Just repro'd issue on M59  9460.23.0, 59.0.3071.35  Kevin. Issue persisted even after sign-out and sign-in to the user account 

Filed feedback report here: http://feedback/#/Report/59381873259

This issue could be duplicate of an issue I raised earlier 
because I am seeing both happen at the same time. https://bugs.chromium.org/p/chromium/issues/detail?id=715819  (File Manager Drive: Sharing failed message is seen frequently)


Cc: weifangsun@chromium.org
+weifangsun
Cc: clamy@chromium.org
clamy@: could this issue be related to 
https://bugs.chromium.org/p/chromium/issues/detail?id=709769
Cc: satorux@chromium.org
Please note: The TOK team is currently out through Fri due to a national holiday. Will ask them to take a look as a priority once they are back in the office.

+satorux@ for visibility.
@gkihumba: no I don't think so. If it's related to a PlzNavigate issue, I would rather say it was  issue 709771  which was fixed in 60.0.3088.0.

If it is  issue 709771 , note that it only happens on Canary & Dev.
Is 60.0.3088.0 the most recent Canary release on Chrome OS (I forgot to check this morning when I updated on C302).  If so, I did notice the problem was gone after updating this morning.
hirono, could you take a look?

Please check if the issue is reproducible with --enable-browser-side-navigation disabled in about flags. FWIW issue 709769 is reproducible only when the flag is enabled.
Status: Started (was: Assigned)
Sure let me take a look.
The issue was not reproduced on the latest canary.
Chrome OS 9531.0.0
Chrome 60.0.3089.0

Checking with 60.0.3086.0...
Still not reproducible with 60.0.3086.0.
Checking with 59.0.3071.35


Reproduced on 59.0.3071.35 with --enable-browser-side-navigation
Not reproduced on 59.0.3071.35 without --enable-browser-side-navigation

Checking on 60.0.3089.0 with --enable-browser-side-navigation

Cc: -clamy@chromium.org hirono@chromium.org
Owner: clamy@chromium.org
On 60.0.3089.0

Reproduced with --enable-browser-side-navigation
Not reproduced without --enable-browser-side-navigation

So it looks relevant with browser side navigation.

clamy@ - Could you take a look at this?
Components: UI>Browser>Navigation
Labels: Proj-PlzNavigate-Blocking
+Proj-PlzNavigate-Blocking label as per issue 709769.

Is --enable-browser-side-navigation still enabled on Canary and Dev on Chrome OS? Otherwise, I think we should disable the flag for Chrome OS for all channels until the known issues (this bug and issue 709769) are resolved, as suggested at crbug.com/709769#c27
Labels: -Pri-1 Pri-0
Upping the priority on this as it's blocking our already late M59 beta.
Cc: nasko@chromium.org
Comment 30: This shouldn't be a beta-blocker, right?  The PlzNavigate field trial is only on Canary and Dev, not on Beta.

Comment 29: I'm asking nasko@ to look at disabling the Canary/Dev trial on Chrome OS, since clamy@ is OOO.
Cc: clamy@chromium.org
Owner: arthurso...@chromium.org
Comment 30: No, this should not be a beta blocker, as PlzNavigate is not enabled beyond Canary and Dev channel.

Comment 29: I've sent for review a CL to disable the experiment on ChromeOS. We will enable it once we have a fix for this.

Comment 28: Does it repro with Chrome for ChromeOS build or does one need physical machine to run ChromeOS on?
Labels: -ReleaseBlock-Beta
The change to disable PlzNavigate on ChromeOS has landed. Restarting to pick up a fresh Finch setup should get you back out of the trial.
Cc: yamaguchi@chromium.org
Re #32: 
> Comment 28: Does it repro with Chrome for ChromeOS build or does one need physical machine to run ChromeOS on?

I'm guessing it can repro on Chrome for Chrome OS build on Linux desktop. yamaguchi@, could you send nasko@ instructions on how to test Files app + Drive on Linux desktop?
#32 and #35: Just in case it's useful, I've successfully debugged the Files app + Drive on Linux for other issues using instructions in https://bugs.chromium.org/p/chromium/issues/detail?id=705295#c22.
I could also reproduce the issue on Linux desktop with ToT.
You'd need 2 flags when starting Chrome to reproduce the issue on Linux, after following the build instruction https://sites.google.com/a/google.com/chromeos-team-tok/getting-started/set-up-dev-environmnet

 ${OUT_DIR}/chrome --login-manager --enable-browser-side-navigation

* --login-manager is needed to access your Google Drive.
* The issue will not happen if you omit --enable-browser-side-navigation after it's disabled by default.

This reproduces without the Files app when processing a redirect of  externalfile://... URL specifically.
When entering the URL like externalfile://... to the browser, the same issues happens only when the flag is set.

https://cs.chromium.org/chromium/src/net/url_request/url_request.cc?sq=package:chromium&dr&l=800
As far as I saw, RedirectInfo has the new right redirect URL at this point. So we'd need to look into processings after that.
I will be OOO for the end of the week.
I almost found what was the bug.

The external-file: url is "redirected" to its real "https:" url in ExternalFileURLRequestJob. The way it does the redirect is maybe not compatible with PlzNavigate. 
CommonNavigationParams.url is updated, but
RequestNavigationParams::original_url is not updated.

Then after a round-trip in the renderer:
ChildProcessSecurityPolicyImpl::CanRequestURL(...) is used with the "external-file" url and the request is blocked.

Project Member

Comment 40 by bugdroid1@chromium.org, May 26 2017

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

commit ecb25eb8968a45120ee434af2d3da30baebc6b63
Author: jam <jam@chromium.org>
Date: Fri May 26 20:06:11 2017

Fix Docs links from Files app on ChromeOS not working with PlzNavigate.

The cause was that there was a redirect from externalfile scheme to https. The PlzNavigate code chose the right ending process based on the final URL. However when the renderer made a request for the stream URL, it does attach the original URL (pre-redirect) along. ResourceDispatcherHostImpl::ShouldServiceRequest was incorrectly aborting the URL because that process rightly does not have access to the externalfile scheme.

The fix is to skip the ChildProcessSecurityPolicy::CanRequestURL check in ShouldServiceRequest for the PlzNavigate stream request. The browser already picked the right process, and the stream URL is unguessable. The change also skips most of ContinuePendingBeginRequest() and CreateResourceHandler() for PlzNavigate stream requests to minimize the risk that they contain code that could be exploited.

BUG= 717644 

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

[modify] https://crrev.com/ecb25eb8968a45120ee434af2d3da30baebc6b63/content/browser/loader/resource_dispatcher_host_impl.cc
[modify] https://crrev.com/ecb25eb8968a45120ee434af2d3da30baebc6b63/content/browser/loader/resource_dispatcher_host_impl.h

Comment 41 by jam@chromium.org, May 26 2017

Labels: Merge-Request-60
Project Member

Comment 42 by sheriffbot@chromium.org, May 27 2017

Labels: -Merge-Request-60 Hotlist-Merge-Approved Merge-Approved-60
Your change meets the bar and is auto-approved for M60. Please go ahead and merge the CL to branch 3112 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), josafat@(ChromeOS), bustamante@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 43 by bugdroid1@chromium.org, May 30 2017

Labels: -merge-approved-60 merge-merged-3112
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/12c73ea1a9ada42efdc3e2b245e718214d6f3125

commit 12c73ea1a9ada42efdc3e2b245e718214d6f3125
Author: John Abd-El-Malek <jam@chromium.org>
Date: Tue May 30 16:20:58 2017

Fix Docs links from Files app on ChromeOS not working with PlzNavigate.

The cause was that there was a redirect from externalfile scheme to https. The PlzNavigate code chose the right ending process based on the final URL. However when the renderer made a request for the stream URL, it does attach the original URL (pre-redirect) along. ResourceDispatcherHostImpl::ShouldServiceRequest was incorrectly aborting the URL because that process rightly does not have access to the externalfile scheme.

The fix is to skip the ChildProcessSecurityPolicy::CanRequestURL check in ShouldServiceRequest for the PlzNavigate stream request. The browser already picked the right process, and the stream URL is unguessable. The change also skips most of ContinuePendingBeginRequest() and CreateResourceHandler() for PlzNavigate stream requests to minimize the risk that they contain code that could be exploited.

BUG= 717644 

Review-Url: https://codereview.chromium.org/2908593002
Cr-Original-Commit-Position: refs/heads/master@{#475097}
Review-Url: https://codereview.chromium.org/2910063003 .
Cr-Commit-Position: refs/branch-heads/3112@{#31}
Cr-Branched-From: b6460e24cf59f429d69de255538d0fc7a425ccf9-refs/heads/master@{#474897}

[modify] https://crrev.com/12c73ea1a9ada42efdc3e2b245e718214d6f3125/content/browser/loader/resource_dispatcher_host_impl.cc
[modify] https://crrev.com/12c73ea1a9ada42efdc3e2b245e718214d6f3125/content/browser/loader/resource_dispatcher_host_impl.h

Comment 44 by jam@chromium.org, May 30 2017

Status: Fixed (was: Started)
Cc: dhadd...@chromium.org mkarkada@chromium.org
Status: Verified (was: Fixed)
Verified on Chrome OS 9592.71.0, 60.0.3112.80 (stable build) on devices caroline and elm. 

Sign in to add a comment