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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 week ago by satorux1
Modified:
3 days, 21 hours 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
1 week ago (2017-05-19 05:07:15 UTC) #2
mmenke
LGTM
1 week ago (2017-05-19 19:43:08 UTC) #8
Daniel Erat
lgtm
6 days, 3 hours 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 days, 20 hours 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 days, 6 hours ago (2017-05-22 21:49:18 UTC) #16
Daniel Erat
lgtm
4 days, 5 hours 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 days, 5 hours 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
3 days, 21 hours ago (2017-05-23 06:24:59 UTC) #21
commit-bot: I haz the power
3 days, 21 hours 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 650457f06