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

base/message_pump_win.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/message_pump_win.h"
 
 #include <math.h>
 
 #include "base/debug/trace_event.h"
 #include "base/message_loop.h"
@@ skipping 457 matching lines @@
 
 void MessagePumpForIO::ScheduleDelayedWork(const TimeTicks& delayed_work_time) {
   // We know that we can't be blocked right now since this method can only be
   // called on the same thread as Run, so we only need to update our record of
   // how long to sleep when we do sleep.
   delayed_work_time_ = 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:
 
 void MessagePumpForIO::DoRunLoop() {
   for (;;) {
     // If we do any work, we may create more messages etc., and more work may
     // possibly be waiting in another task group.  When we (for example)
     // WaitForIOCompletion(), there is a good chance there are still more
     // messages waiting.  On the other hand, when any of these methods return
     // having done no work, then it is pretty unlikely that calling them
@@ skipping 46 matching lines @@
   IOItem item;
   if (completed_io_.empty() || !MatchCompletedIOItem(filter, &item)) {
     // We have to ask the system for another IO completion.
     if (!GetIOItem(timeout, &item))
       return false;
 
     if (ProcessInternalIOItem(item))
       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);
       DidProcessIOEvent();
     }
   } else {
     // The handler must be gone by now, just cleanup the mess.
     delete item.context;
   }
   return true;
 }
 
 // Asks the OS for another IO completion result.
 bool MessagePumpForIO::GetIOItem(DWORD timeout, IOItem* item) {
   memset(item, 0, sizeof(*item));
   ULONG_PTR key = NULL;
   OVERLAPPED* overlapped = NULL;
   if (!GetQueuedCompletionStatus(port_.Get(), &item->bytes_transfered, &key,
                                  &overlapped, timeout)) {
     if (!overlapped)
       return false;  // Nothing in the queue.
     item->error = GetLastError();
     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;
 }
 
 bool MessagePumpForIO::ProcessInternalIOItem(const IOItem& item) {
   if (this == reinterpret_cast<MessagePumpForIO*>(item.context) &&
       this == reinterpret_cast<MessagePumpForIO*>(item.handler)) {
     // This is our internal completion.
     DCHECK(!item.bytes_transfered);
     InterlockedExchange(&have_work_, 0);
@@ skipping 25 matching lines @@
 }
 
 void MessagePumpForIO::WillProcessIOEvent() {
   FOR_EACH_OBSERVER(IOObserver, io_observers_, WillProcessIOEvent());
 }
 
 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