Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2890123004:
chromeos: Remove blanket file access logic from chromeos chrome on Linux (Closed)
Descriptionchromeos: 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)
satorux@chromium.org changed reviewers: + derat@chromium.org, mmenke@chromium.org
ptal
The CQ bit was checked by satorux@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== chromeos: Remove blanket file access logic from chromeos chrome on Linux Previously, access to any file was allowed via file: scheme if Chrome for Chrome OS runs on Linux desktop [1]. This logic was problematic when it comes to write an end-to-end browser test that ensures if 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 file with Chrome for Chrome OS on Linux desktop, 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 ========== to ========== chromeos: Remove blanket file access logic from chromeos chrome on Linux Previously, access to any file was allowed via file: scheme if Chrome for Chrome OS runs on Linux desktop [1]. This 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 file with Chrome for Chrome OS on Linux desktop, 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 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
LGTM
lgtm
turned out a lot of tests failed on chromeos bots... sorry for not checking the results before sending this out.
The CQ bit was checked by satorux@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== chromeos: Remove blanket file access logic from chromeos chrome on Linux Previously, access to any file was allowed via file: scheme if Chrome for Chrome OS runs on Linux desktop [1]. This 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 file with Chrome for Chrome OS on Linux desktop, 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 ========== to ========== 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 added 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 file with Chrome for Chrome OS on Linux desktop, 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 ==========
PTAL. added back kTestType conditiona, and updated the patch descriptionl. I think I can use this flag to emulate production behaviors (access to files is restricted) for browser tests.
lgtm
On 2017/05/22 22:25:31, Daniel Erat wrote: > lgtm LGTM as well
Description was changed from ========== 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 added 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 file with Chrome for Chrome OS on Linux desktop, 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 ========== to ========== 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 ==========
The CQ bit was checked by satorux@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1495520683546830,
"parent_rev": "a7e8c1c7a8b5ec19c82698eaee5c6aad7dead5aa", "commit_rev":
"561da39a7a22b9d1ff96fcc742bd32bc2c3543b6"}
Message was sent while issue was closed.
Description was changed from ========== 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 ========== to ========== 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/+/561da39a7a22b9d1ff96fcc742bd... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/561da39a7a22b9d1ff96fcc742bd... |
