[Chromoting] Adding uiAccess='true' to the remoting_me2me_host.exe's manifest as it is required to …

base/message_pump_win.cc

Index: base/message_pump_win.cc
diff --git a/base/message_pump_win.cc b/base/message_pump_win.cc
index 2b2a10e07534afcf7c7dd8de14567bd4c794d5cd..835a6bd9d3b7cf7392f4175a612b29d3943267fe 100644
--- a/base/message_pump_win.cc
+++ b/base/message_pump_win.cc
@@ -475,11 +475,26 @@ void MessagePumpForIO::ScheduleDelayedWork(const TimeTicks& delayed_work_time) {
 
 void MessagePumpForIO::RegisterIOHandler(HANDLE file_handle,
                                          IOHandler* handler) {
-  ULONG_PTR key = reinterpret_cast<ULONG_PTR>(handler);
+  ULONG_PTR key = HandlerToKey(handler, true);
   HANDLE port = CreateIoCompletionPort(file_handle, port_, key, 1);
   DPCHECK(port);
 }
 
+bool MessagePumpForIO::RegisterJobObject(HANDLE job_handle,
+                                         IOHandler* handler) {
+  // Job object notifications use the OVERLAPPED pointer to carry the message
+  // data. Mark the completion key correspondingly, so we will not try to
+  // convert OVERLAPPED* to IOContext*.
+  ULONG_PTR key = HandlerToKey(handler, false);
+  JOBOBJECT_ASSOCIATE_COMPLETION_PORT info;
+  info.CompletionKey = reinterpret_cast<void*>(key);
+  info.CompletionPort = port_;
+  return SetInformationJobObject(job_handle,
+                                 JobObjectAssociateCompletionPortInformation,
+                                 &info,
+                                 sizeof(info)) != FALSE;
+}
+
 //-----------------------------------------------------------------------------
 // MessagePumpForIO private:
 
@@ -546,12 +561,13 @@ bool MessagePumpForIO::WaitForIOCompletion(DWORD timeout, IOHandler* filter) {
       return true;
   }
 
-  if (item.context->handler) {
+  if (!item.has_valid_io_context || item.context->handler) {

[rvargas (doing something else), 2012/08/13 18:54:12]

This deserves a comment given that the first condition is redundant and we are
"just" avoiding touching invalid memory with the second condition.

[alexeypa (please no reviews), 2012/08/13 20:02:23]

Done, though both conditions are not redundant. Or I'm interpreting your comment
incorrectly.

[rvargas (doing something else), 2012/08/13 21:20:10]

Thanks. What I meant is that (ignoring dereferencing an invalid pointer),
handler cannot be NULL when has_valid_io_context is false (as in, the caller
cannot clear up the handler after it was registered, for job related
operations), so the job case would always flow into the if clause 'cause handler
would be true.

Of course, then we have the whole issue about not being able to see the handler
at all (because there's no storage for it in the first place), but then the code
doesn't follow the common pattern of "if (a.valid && a.use)"... so the comment
helps.

Another way to look at it: The if is basically saying "was this item cancelled
or do I have to process it". The condition of job || handler is correct. I just
wanted a comment pointing out why |handler| alone was not enough/possible.

     if (filter && item.handler != filter) {
       // Save this item for later
       completed_io_.push_back(item);
     } else {
-      DCHECK_EQ(item.context->handler, item.handler);
+      DCHECK(!item.has_valid_io_context ||
+             (item.context->handler == item.handler));
       WillProcessIOEvent();
       item.handler->OnIOCompleted(item.context, item.bytes_transfered,
                                   item.error);
@@ -577,7 +593,7 @@ bool MessagePumpForIO::GetIOItem(DWORD timeout, IOItem* item) {
     item->bytes_transfered = 0;
   }
 
-  item->handler = reinterpret_cast<IOHandler*>(key);
+  item->handler = KeyToHandler(key, &item->has_valid_io_context);
   item->context = reinterpret_cast<IOContext*>(overlapped);
   return true;
 }
@@ -623,4 +639,27 @@ void MessagePumpForIO::DidProcessIOEvent() {
   FOR_EACH_OBSERVER(IOObserver, io_observers_, DidProcessIOEvent());
 }
 
+// static
+ULONG_PTR MessagePumpForIO::HandlerToKey(IOHandler* handler,
+                                         bool has_valid_io_context) {
+  ULONG_PTR key = reinterpret_cast<ULONG_PTR>(handler);
+
+  // |IOHandler| has virtual methods, so it is at least pointer-size aligned.
+  DCHECK((key & 1) == 0);
+
+  // Mark the completion key as context-less.
+  if (!has_valid_io_context) {
+    key = key | 1;

[rvargas (doing something else), 2012/08/13 18:54:12]

ouch

[alexeypa (please no reviews), 2012/08/13 20:02:23]

I think introducing a constant instead on the flag will make the code less
compact and less readable.

[rvargas (doing something else), 2012/08/13 21:20:10]

yeah... I was just showing pain, not suggesting that it should be changed.

+  }

[rvargas (doing something else), 2012/08/13 18:54:12]

nit: single line if without {}

[alexeypa (please no reviews), 2012/08/13 20:02:23]

Done.

+  return key;
+}
+
+// static
+MessagePumpForIO::IOHandler* MessagePumpForIO::KeyToHandler(
+    ULONG_PTR key,
+    bool* has_valid_io_context_out) {

[rvargas (doing something else), 2012/08/13 18:54:12]

nit: naming an out argument _out is highly atypical. Avoid it.

[alexeypa (please no reviews), 2012/08/13 20:02:23]

Done.

+  *has_valid_io_context_out = !(key & 1);
+  return reinterpret_cast<IOHandler*>(key & ~static_cast<ULONG_PTR>(1));
+}
+
 }  // namespace base