Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(89)

Issue 2890123004: chromeos: Remove blanket file access logic from chromeos chrome on Linux (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 months ago by satorux1
Modified:
4 months ago
Reviewers:
Daniel Erat, mmenke
CC:
chromium-reviews, cbentzel+watch_chromium.org, net-reviews_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

chromeos: Remove blanket file access logic from chromeos chrome on Linux crrev.com/472352 removed kTestType conditional from ChromeNetworkDelegate for Chrome OS, but this change adds it back, and instead remove blanket file access logic based on IsRunningOnChromeOS(). This blanket file access logic was problematic when it comes to write an end-to-end browser test that ensures access control of file: scheme is working correctly (i.e. with this logic, browser_tests for Chrome OS that runs on Linux desktop can access to any files). If you need to load your files with Chrome for Chrome OS on Linux desktop [1], please place these files under /tmp, that's white listed in ChromeNetworkDelegate::IsAccessAllowed(). [1] This build config is for development only, not for production. BUG=677933 TEST=access to file:///tmp is allowed on Linux desktop, but file:/// is not Review-Url: https://codereview.chromium.org/2890123004 Cr-Commit-Position: refs/heads/master@{#473821} Committed: https://chromium.googlesource.com/chromium/src/+/561da39a7a22b9d1ff96fcc742bd32bc2c3543b6

Patch Set 1 #

Patch Set 2 : add back kTestType #

Messages

Total messages: 24 (15 generated)
satorux1
ptal
4 months ago (2017-05-19 05:07:15 UTC) #2
mmenke
LGTM
4 months ago (2017-05-19 19:43:08 UTC) #8
Daniel Erat
lgtm
4 months ago (2017-05-21 00:36:28 UTC) #9
satorux1
turned out a lot of tests failed on chromeos bots... sorry for not checking the ...
4 months ago (2017-05-22 07:54:45 UTC) #10
satorux1
PTAL. added back kTestType conditiona, and updated the patch descriptionl. I think I can use ...
4 months ago (2017-05-22 21:49:18 UTC) #16
Daniel Erat
lgtm
4 months ago (2017-05-22 22:25:31 UTC) #17
mmenke
On 2017/05/22 22:25:31, Daniel Erat wrote: > lgtm LGTM as well
4 months ago (2017-05-22 22:26:39 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2890123004/20001
4 months ago (2017-05-23 06:24:59 UTC) #21
commit-bot: I haz the power
4 months ago (2017-05-23 06:30:00 UTC) #24
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/561da39a7a22b9d1ff96fcc742bd...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld b40b6558b