Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(15)

Issue 2193603004: Ignore desktop creation errors in the sandbox. (Closed)

Created:
4 years, 4 months ago by forshaw
Modified:
4 years, 4 months ago
Reviewers:
Will Harris
CC:
chromium-reviews, darin-cc_chromium.org, jam, rickyz+watch_chromium.org, wfh+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Ignore desktop creation errors in the sandbox. This patch reverts the original behaviour that we ignore errors when creating the alternative desktop. Also this includes a change to try and open the window station with lower privileges if the first request fails, which works fine for operation but not for testing. Finally it logs any failure to create the desktop as a warning in the existing histogram. CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win10_chromium_x64_rel_ng TEST=1. Test on Windows 7 2. Download PSTools from https://technet.microsoft.com/en-us/sysinternals/pstools.aspx 3. Run cmd.exe as an administrator 4. Run the command 'psexec -s -i 0 c:\path\to\chrome.exe' 5. Click on the icon which indicates there a message on the service desktop. This will switch to the service desktop where chrome is running. 6. Verify that the browser is useable BUG=615396 Committed: https://crrev.com/37dd5680ab4edbbf730e8540825b9caa8db0d911 Cr-Commit-Position: refs/heads/master@{#409229}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+17 lines, -6 lines) Patch
M content/common/sandbox_win.cc View 3 chunks +11 lines, -4 lines 0 comments Download
M sandbox/win/src/window.cc View 1 chunk +6 lines, -2 lines 0 comments Download

Messages

Total messages: 14 (5 generated)
forshaw
PTAL
4 years, 4 months ago (2016-07-29 17:56:39 UTC) #2
Will Harris
lgtm
4 years, 4 months ago (2016-08-01 22:31:19 UTC) #3
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/2193603004/1
4 years, 4 months ago (2016-08-01 22:31:47 UTC) #5
commit-bot: I haz the power
Your CL relies on deprecated CQ feature(s): * Specifying master names in CQ_INCLUDE_TRYBOTS part of ...
4 years, 4 months ago (2016-08-01 22:31:49 UTC) #6
commit-bot: I haz the power
Try jobs failed on following builders: win10_chromium_x64_rel_ng on tryserver.chromium.win (JOB_TIMED_OUT, http://build.chromium.org/p/tryserver.chromium.win/builders/win10_chromium_x64_rel_ng/builds/229)
4 years, 4 months ago (2016-08-02 05:29:40 UTC) #8
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/2193603004/1
4 years, 4 months ago (2016-08-02 16:37:58 UTC) #10
commit-bot: I haz the power
Your CL relies on deprecated CQ feature(s): * Specifying master names in CQ_INCLUDE_TRYBOTS part of ...
4 years, 4 months ago (2016-08-02 16:38:00 UTC) #11
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years, 4 months ago (2016-08-02 17:48:06 UTC) #12
commit-bot: I haz the power
4 years, 4 months ago (2016-08-02 17:51:11 UTC) #14
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/37dd5680ab4edbbf730e8540825b9caa8db0d911
Cr-Commit-Position: refs/heads/master@{#409229}

Powered by Google App Engine
This is Rietveld 408576698