Attempt3 at landing this. The previous attempt failed on Windows XP because the \Sessions\Session i…

Attempt3 at landing this. The previous attempt failed on Windows XP because the \Sessions\Session id\BaseNamedObjects path
does not always exist on Windows XP. It only exists for terminal server sessions.

Relanding this with fixes for the SyncPolicyTest.TestEvent and SyncPolicyTest.TestEventReadOnly tests.

Replace the CreateEvent/OpenEvent patches with their Nt counterparts like
NtOpenEvent and NtCreateEvent.

Reason being :- We patch these APIS via the Export table patch which does not
work with bound imports. This results in our
patched functions never getting called.

This should fix the GPU process hang with the XP presentation path.

The change from the previous patch is to resolve the BaseNamedObjects path via the \Sessions\BNOLinks directory which
contains the BaseNamedObjects symbolic links for the running sessions

BUG=305815
R=cpu@chromium.org, rvargas@chromium.org, cpu, rvargas

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=231063

[cpu_(ooo_6.6-7.5), 2013-10-24 22:06:50]

can you paste here what is the new code?

[rvargas (doing something else), 2013-10-24 22:18:28]

lgtm

It would have been nice to have a diff vs the first version :(

https://codereview.chromium.org/41193002/diff/1/sandbox/win/src/sync_policy.cc
File sandbox/win/src/sync_policy.cc (right):

https://codereview.chromium.org/41193002/diff/1/sandbox/win/src/sync_policy.c...
sandbox/win/src/sync_policy.cc:40: HANDLE symbolic_link_directory = NULL;
nit: move to right before use

https://codereview.chromium.org/41193002/diff/1/sandbox/win/src/sync_policy.c...
sandbox/win/src/sync_policy.cc:51: NOTREACHED() << "Failed to open symbolic link
directory. Error: "
nit: remove | dlog

https://codereview.chromium.org/41193002/diff/1/sandbox/win/src/sync_policy.c...
sandbox/win/src/sync_policy.cc:65: NOTREACHED() << "Failed to open symbolic link
Error: " << status;
nit: remove

https://codereview.chromium.org/41193002/diff/1/sandbox/win/src/sync_policy.c...
sandbox/win/src/sync_policy.cc:73: if (!target_length) {
status != STATUS_BUFFER_OVERFLOW ?

https://codereview.chromium.org/41193002/diff/1/sandbox/win/src/sync_policy.c...
sandbox/win/src/sync_policy.cc:75: NOTREACHED() << "Failed to get length for
symbolic link target. Error: "
nit: remove

https://codereview.chromium.org/41193002/diff/1/sandbox/win/src/sync_policy.c...
sandbox/win/src/sync_policy.cc:88: NOTREACHED() << "Failed to resolve symbolic
link. Error: " << status;
nit: remove

https://codereview.chromium.org/41193002/diff/1/sandbox/win/src/sync_policy.c...
sandbox/win/src/sync_policy.cc:115: NOTREACHED() << "Failed to resolve
BaseNamedObjects path. Error: "
nit: remove

[ananta, 2013-10-24 22:59:43]

On 2013/10/24 22:06:50, cpu wrote:
> can you paste here what is the new code?

The change is in sync_policy.cc. Please look at the new function
ResolveSymbolicLink which resolves the symbolic
link within the directory and its call site in the GetBaseNamedObjectsDirectory
function.

Thanks
Ananta

[ananta, 2013-10-24 22:59:50]

https://codereview.chromium.org/41193002/diff/1/sandbox/win/src/sync_policy.cc
File sandbox/win/src/sync_policy.cc (right):

https://codereview.chromium.org/41193002/diff/1/sandbox/win/src/sync_policy.c...
sandbox/win/src/sync_policy.cc:40: HANDLE symbolic_link_directory = NULL;
On 2013/10/24 22:18:29, rvargas wrote:
> nit: move to right before use

Done.

https://codereview.chromium.org/41193002/diff/1/sandbox/win/src/sync_policy.c...
sandbox/win/src/sync_policy.cc:51: NOTREACHED() << "Failed to open symbolic link
directory. Error: "
On 2013/10/24 22:18:29, rvargas wrote:
> nit: remove | dlog

Done.

https://codereview.chromium.org/41193002/diff/1/sandbox/win/src/sync_policy.c...
sandbox/win/src/sync_policy.cc:65: NOTREACHED() << "Failed to open symbolic link
Error: " << status;
On 2013/10/24 22:18:29, rvargas wrote:
> nit: remove
Replaced with DLOG

https://codereview.chromium.org/41193002/diff/1/sandbox/win/src/sync_policy.c...
sandbox/win/src/sync_policy.cc:73: if (!target_length) {
On 2013/10/24 22:18:29, rvargas wrote:
> status != STATUS_BUFFER_OVERFLOW ?

Done.

https://codereview.chromium.org/41193002/diff/1/sandbox/win/src/sync_policy.c...
sandbox/win/src/sync_policy.cc:73: if (!target_length) {
On 2013/10/24 22:18:29, rvargas wrote:
> status != STATUS_BUFFER_OVERFLOW ?

Replaced with status != STATUS_BUFFER_TOO_SMALL

https://codereview.chromium.org/41193002/diff/1/sandbox/win/src/sync_policy.c...
sandbox/win/src/sync_policy.cc:75: NOTREACHED() << "Failed to get length for
symbolic link target. Error: "
On 2013/10/24 22:18:29, rvargas wrote:
> nit: remove
Replaced with DLOG

https://codereview.chromium.org/41193002/diff/1/sandbox/win/src/sync_policy.c...
sandbox/win/src/sync_policy.cc:88: NOTREACHED() << "Failed to resolve symbolic
link. Error: " << status;
On 2013/10/24 22:18:29, rvargas wrote:
> nit: remove

Replaced with DLOG

https://codereview.chromium.org/41193002/diff/1/sandbox/win/src/sync_policy.c...
sandbox/win/src/sync_policy.cc:115: NOTREACHED() << "Failed to resolve
BaseNamedObjects path. Error: "
On 2013/10/24 22:18:29, rvargas wrote:
> nit: remove
Replaced with DLOG

[rvargas (doing something else), 2013-10-24 23:48:06]

To be honest, I don't think we should have all those DLOGs. It is nice to be
able to know when something fails, and while we figure out what works on all
bots they are quite useful, but after that it feels like leaving development
traces in the code... logs add up to the size of the compiled code and the speed
of running chrome.

So we can leave them, and even convert them to LOGs, but we should remove most
of them in the not-so-distant future.

[cpu_(ooo_6.6-7.5), 2013-10-25 16:56:27]

lgtm

[commit-bot: I haz the power, 2013-10-25 16:58:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ananta@chromium.org/41193002/290001

[commit-bot: I haz the power, 2013-10-25 17:17:08]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[ananta, 2013-10-25 18:46:21]

Committed patchset #3 manually as r231063 (presubmit successful).