Added support of relative mouse motion in Chromoting.

remoting/client/plugin/chromoting_instance.cc

Index: remoting/client/plugin/chromoting_instance.cc
diff --git a/remoting/client/plugin/chromoting_instance.cc b/remoting/client/plugin/chromoting_instance.cc
index 37bceee87bf26631c5e5b5735bff033fb6cd9594..12c9e7a95df125a54b8bfb90049dc3d43de5364e 100644
--- a/remoting/client/plugin/chromoting_instance.cc
+++ b/remoting/client/plugin/chromoting_instance.cc
@@ -4,6 +4,7 @@
 
 #include "remoting/client/plugin/chromoting_instance.h"
 
+#include <algorithm>
 #include <string>
 #include <vector>
 
@@ -55,6 +56,12 @@ namespace {
 // 32-bit BGRA is 4 bytes per pixel.
 const int kBytesPerPixel = 4;
 
+#if defined(ARCH_CPU_LITTLE_ENDIAN)
+const uint32_t kPixelAlphaMask = 0xff000000;
+#else  // !defined(ARCH_CPU_LITTLE_ENDIAN)
+const uint32_t kPixelAlphaMask = 0x000000ff;
+#endif  // !defined(ARCH_CPU_LITTLE_ENDIAN)
+
 // Default DPI to assume for old clients that use notifyClientResolution.
 const int kDefaultDPI = 96;
 
@@ -125,6 +132,11 @@ std::string ConnectionErrorToString(protocol::ErrorCode error) {
   return std::string();
 }
 
+// Returns true |pixel| is not completely transparent.

[Wez, 2013/09/05 20:24:45]

nit: "... true if ..."

[alexeypa (please no reviews), 2013/09/06 20:00:17]

Done.

+bool IsVisiblePixel(uint32_t pixel) {
+  return (pixel & kPixelAlphaMask) != 0;
+}
+
 // This flag blocks LOGs to the UI if we're already in the middle of logging
 // to the UI. This prevents a potential infinite loop if we encounter an error
 // while sending the log message to the UI.
@@ -144,7 +156,7 @@ logging::LogMessageHandlerFunction g_logging_old_handler = NULL;
 const char ChromotingInstance::kApiFeatures[] =
     "highQualityScaling injectKeyEvent sendClipboardItem remapKey trapKey "
     "notifyClientResolution pauseVideo pauseAudio asyncPin thirdPartyAuth "
-    "pinlessAuth extensionMessage";
+    "pinlessAuth extensionMessage allowMouseLock";
 
 const char ChromotingInstance::kRequestedCapabilities[] = "";
 const char ChromotingInstance::kSupportedCapabilities[] = "desktopShape";
@@ -170,14 +182,18 @@ bool ChromotingInstance::ParseAuthMethods(const std::string& auth_methods_str,
 
 ChromotingInstance::ChromotingInstance(PP_Instance pp_instance)
     : pp::Instance(pp_instance),
+      pp::MouseLock(this),
       initialized_(false),
       plugin_task_runner_(new PluginThreadTaskRunner(&plugin_thread_delegate_)),
       context_(plugin_task_runner_.get()),
+      focused_(false),
+      mouse_lock_state_(MouseLockDisabled),
       input_tracker_(&mouse_input_filter_),
       key_mapper_(&input_tracker_),
       normalizing_input_filter_(CreateNormalizingInputFilter(&key_mapper_)),
       input_handler_(normalizing_input_filter_.get()),
       use_async_pin_dialog_(false),
+      callback_factory_(this),
       weak_factory_(this) {
   RequestInputEvents(PP_INPUTEVENT_CLASS_MOUSE | PP_INPUTEVENT_CLASS_WHEEL);
   RequestFilteringInputEvents(PP_INPUTEVENT_CLASS_KEYBOARD);
@@ -304,9 +320,18 @@ void ChromotingInstance::HandleMessage(const pp::Var& message) {
     HandleRequestPairing(*data);
   } else if (method == "extensionMessage") {
     HandleExtensionMessage(*data);
+  } else if (method == "allowMouseLock") {
+    HandleAllowMouseLockMessage();
   }
 }
 
+void ChromotingInstance::DidChangeFocus(bool has_focus) {
+  DCHECK(plugin_task_runner_->BelongsToCurrentThread());
+
+  focused_ = has_focus;
+  RequestMouseLock();

[Wez, 2013/09/05 20:24:45]

nit: It looks strange to call RequestMouseLock() in the case that we're becoming
un-focused.

Does mouse lock fail in that case, BTW?

[alexeypa (please no reviews), 2013/09/06 20:00:17]

RequestMouseLock() checks whether the plugin is focused.
Yes, as far as I can tell.

+}
+
 void ChromotingInstance::DidChangeView(const pp::View& view) {
   DCHECK(plugin_task_runner_->BelongsToCurrentThread());
 
@@ -323,7 +348,17 @@ bool ChromotingInstance::HandleInputEvent(const pp::InputEvent& event) {
   if (!IsConnected())
     return false;
 
-  return input_handler_.HandleInputEvent(event);
+  return input_handler_.HandleInputEvent(event,
+                                         mouse_lock_state_ == MouseLockOn);
+}
+
+void ChromotingInstance::MouseLockLost() {
+  DCHECK(plugin_task_runner_->BelongsToCurrentThread());
+  DCHECK(mouse_lock_state_ == MouseLockOn ||
+         mouse_lock_state_ == MouseLockCancelling);
+
+  mouse_lock_state_ = MouseLockOff;
+  EnableMousePointer();
 }
 
 void ChromotingInstance::SetDesktopSize(const SkISize& size,
@@ -464,6 +499,8 @@ void ChromotingInstance::InjectClipboardEvent(
 
 void ChromotingInstance::SetCursorShape(
     const protocol::CursorShapeInfo& cursor_shape) {
+  COMPILE_ASSERT(sizeof(uint32_t) == kBytesPerPixel, rgba_pixels_are_32bit);
+
   if (!cursor_shape.has_data() ||
       !cursor_shape.has_width() ||
       !cursor_shape.has_height() ||
@@ -500,47 +537,58 @@ void ChromotingInstance::SetCursorShape(
 
   int hotspot_x = cursor_shape.hotspot_x();
   int hotspot_y = cursor_shape.hotspot_y();
-
   int bytes_per_row = width * kBytesPerPixel;
-  const uint8* src_row_data = reinterpret_cast<const uint8*>(
+  int src_stride = width;
+  const uint32_t* src_row_data = reinterpret_cast<const uint32_t*>(
       cursor_shape.data().data());
-  int stride = bytes_per_row;
-
-  // If the cursor exceeds the size permitted by PPAPI then crop it, keeping
-  // the hotspot as close to the center of the new cursor shape as possible.
-  if (height > kMaxCursorHeight) {
-    int y = hotspot_y - (kMaxCursorHeight / 2);
-    y = std::max(y, 0);
-    y = std::min(y, height - kMaxCursorHeight);
-
-    src_row_data += stride * y;
-    height = kMaxCursorHeight;
-    hotspot_y -= y;
-  }
-  if (width > kMaxCursorWidth) {
-    int x = hotspot_x - (kMaxCursorWidth / 2);
-    x = std::max(x, 0);
-    x = std::min(x, height - kMaxCursorWidth);
-
-    src_row_data += x * kBytesPerPixel;
-    width = kMaxCursorWidth;
-    bytes_per_row = width * kBytesPerPixel;
-    hotspot_x -= x;
-  }
+  const uint32_t* src_row_data_end = src_row_data + src_stride * height;
+
+  // See if the cursor image completely consists of transparent pixels.

[Wez, 2013/09/05 20:24:45]

nit: consists completely

[alexeypa (please no reviews), 2013/09/06 20:00:17]

Done.

+  bool mouse_lock_requested = (mouse_lock_state_ != MouseLockDisabled) &&
+      (std::find_if(src_row_data,
+                    src_row_data_end,
+                    &IsVisiblePixel) == src_row_data_end);

[Wez, 2013/09/05 20:24:45]

nit: Move this find_if() into an IsVisibleRow() helper alongside IsVisiblePixel.

[alexeypa (please no reviews), 2013/09/06 20:00:17]

Done.

+  if (mouse_lock_requested) {
+    // Request mouse lock.
+    cursor_image_.reset();
+    RequestMouseLock();
+  } else {
+    // If the cursor exceeds the size permitted by PPAPI then crop it, keeping
+    // the hotspot as close to the center of the new cursor shape as possible.

[Wez, 2013/09/05 20:24:45]

Nit, can we move this crop logic into a CropCursor() helper function without
complicating things too much?

[alexeypa (please no reviews), 2013/09/06 20:00:17]

Not sure. This code updates a bunch of local variables meaning a lot of
parameters passed in and out the function. I don't think it belong to this CL.

+    if (height > kMaxCursorHeight) {
+      int y = hotspot_y - (kMaxCursorHeight / 2);
+      y = std::max(y, 0);
+      y = std::min(y, height - kMaxCursorHeight);
+
+      src_row_data += src_stride * y;
+      height = kMaxCursorHeight;
+      hotspot_y -= y;
+    }
+    if (width > kMaxCursorWidth) {
+      int x = hotspot_x - (kMaxCursorWidth / 2);
+      x = std::max(x, 0);
+      x = std::min(x, height - kMaxCursorWidth);
+
+      src_row_data += x;
+      width = kMaxCursorWidth;
+      bytes_per_row = width * kBytesPerPixel;
+      hotspot_x -= x;
+    }
 
-  pp::ImageData cursor_image(this, PP_IMAGEDATAFORMAT_BGRA_PREMUL,
-                             pp::Size(width, height), false);
+    cursor_image_.reset(new pp::ImageData(this, PP_IMAGEDATAFORMAT_BGRA_PREMUL,
+                                          pp::Size(width, height), false));
+    cursor_hotspot_ = pp::Point(hotspot_x, hotspot_y);
 
-  uint8* dst_row_data = reinterpret_cast<uint8*>(cursor_image.data());
-  for (int row = 0; row < height; row++) {
-    memcpy(dst_row_data, src_row_data, bytes_per_row);
-    src_row_data += stride;
-    dst_row_data += cursor_image.stride();
-  }
+    uint8* dst_row_data = reinterpret_cast<uint8*>(cursor_image_->data());
+    for (int row = 0; row < height; row++) {
+      memcpy(dst_row_data, src_row_data, bytes_per_row);
+      src_row_data += src_stride;
+      dst_row_data += cursor_image_->stride();
+    }
 
-  pp::MouseCursor::SetCursor(this, PP_MOUSECURSOR_TYPE_CUSTOM,
-                             cursor_image,
-                             pp::Point(hotspot_x, hotspot_y));
+    // Cancel mouse lock if any and apply the new cursor image.

[Wez, 2013/09/05 20:24:45]

nit: if any and -> if active, and

[alexeypa (please no reviews), 2013/09/06 20:00:17]

Done.

+    CancelMouseLock();
+  }
 }
 
 void ChromotingInstance::OnFirstFrameReceived() {
@@ -860,6 +908,11 @@ void ChromotingInstance::HandleExtensionMessage(
   host_connection_->host_stub()->DeliverClientMessage(message);
 }
 
+void ChromotingInstance::HandleAllowMouseLockMessage() {
+  DCHECK_EQ(mouse_lock_state_, MouseLockDisabled);
+  mouse_lock_state_ = MouseLockOff;
+}
+
 ChromotingStats* ChromotingInstance::GetStats() {
   if (!client_.get())
     return NULL;
@@ -1036,4 +1089,86 @@ bool ChromotingInstance::IsConnected() {
     (host_connection_->state() == protocol::ConnectionToHost::CONNECTED);
 }
 
+void ChromotingInstance::EnableMousePointer() {

[Wez, 2013/09/05 20:24:45]

This name doesn't feel right; you're setting the cursor shape, so call it
SetMouseCursor() or UpdateMouseCursor()?

[alexeypa (please no reviews), 2013/09/06 20:00:17]

Done.

+  DCHECK(plugin_task_runner_->BelongsToCurrentThread());
+  DCHECK(mouse_lock_state_ == MouseLockDisabled ||
+         mouse_lock_state_ == MouseLockOff);
+
+  if (cursor_image_) {
+    pp::MouseCursor::SetCursor(this, PP_MOUSECURSOR_TYPE_CUSTOM,
+                               *cursor_image_,
+                               cursor_hotspot_);
+  } else {
+    // THe browser cancelled mouse lock but we don't have a cursor to show. Use
+    // the standard arrow pointer instead.

[Wez, 2013/09/05 20:24:45]

Will we also be in this state if the host never sent us a cursor shape at all,
in which case comment could be:

"If there is no cursor shape stored, either because the host never supplied one,
or we were previously in mouse-lock mode, then use a standard arrow pointer."

[alexeypa (please no reviews), 2013/09/06 20:00:17]

Done.

+    pp::MouseCursor::SetCursor(this, PP_MOUSECURSOR_TYPE_POINTER);
+  }
+}
+
+void ChromotingInstance::RequestMouseLock() {
+  DCHECK(plugin_task_runner_->BelongsToCurrentThread());
+
+  // Request mouse lock if the plugin is focused, mouse lock requested by

[Wez, 2013/09/05 20:24:45]

nit: lock if -> lock only if

[alexeypa (please no reviews), 2013/09/06 20:00:17]

Done.

+  // the host and no callback is pending.

[Wez, 2013/09/05 20:24:45]

nit: mouse lock requested by the host -> the host-supplied cursor is empty

[alexeypa (please no reviews), 2013/09/06 20:00:17]

Done.

+  if (focused_ && !cursor_image_ && mouse_lock_state_ == MouseLockOff) {

[Wez, 2013/09/05 20:24:45]

nit: I think this function would be more readable w/ a set of three ifs w/
early-exits.

[alexeypa (please no reviews), 2013/09/06 20:00:17]

Six lines instead of one? I don't think so.

+    pp::CompletionCallback callback =
+        callback_factory_.NewCallback(&ChromotingInstance::OnMouseLocked);
+    int result = pp::MouseLock::LockMouse(callback);
+    DCHECK_EQ(result, PP_OK_COMPLETIONPENDING);
+
+    // Hide the cursor to workaround http://crbug.com/285809.

[Wez, 2013/09/05 20:24:45]

nit: Suggest: "Hide cursor to avoid it becoming a black square (see
crbug.com/285809."

[alexeypa (please no reviews), 2013/09/06 20:00:17]

Done.

+    pp::MouseCursor::SetCursor(this, PP_MOUSECURSOR_TYPE_NONE);
+    mouse_lock_state_ = MouseLockPending;

[Wez, 2013/09/05 20:24:45]

nit: Since the comment "Hide the cursor" doesn't apply to this line, have a
blank line between it and the SetCursor() line :)

[alexeypa (please no reviews), 2013/09/06 20:00:17]

Done.

+  }
+}
+
+void ChromotingInstance::CancelMouseLock() {
+  DCHECK(plugin_task_runner_->BelongsToCurrentThread());
+
+  switch (mouse_lock_state_) {
+    case MouseLockDisabled:
+    case MouseLockOff:
+      EnableMousePointer();
+      break;
+
+    case MouseLockCancelling:
+      break;
+
+    case MouseLockPending:
+      // Let the callback know that the operation should be cancelled.

[Wez, 2013/09/05 20:24:45]

nit: Clarify here that we can't call UnlockMouse() while a LockMouse() request
is pending, if that's the case. Otherwise, just call UnlockMouse here. :)

[alexeypa (please no reviews), 2013/09/06 20:00:17]

Done.

+      mouse_lock_state_ = MouseLockCancelling;
+      break;
+
+    case MouseLockOn:
+      pp::MouseLock::UnlockMouse();
+
+      // Wait until MouseLockLost() is called.

[Wez, 2013/09/05 20:24:45]

nit: Suggest "Note that mouse-lock has been cancelled. We will continue to
receive locked events until MouseLockLost() is called back."

[alexeypa (please no reviews), 2013/09/06 20:00:17]

Done.

+      mouse_lock_state_ = MouseLockCancelling;
+      break;
+
+    default:
+      NOTREACHED();
+  }
+}
+
+void ChromotingInstance::OnMouseLocked(int error) {
+  DCHECK(plugin_task_runner_->BelongsToCurrentThread());
+  DCHECK(mouse_lock_state_ == MouseLockPending ||
+         mouse_lock_state_ == MouseLockCancelling);
+
+  bool cancel = (mouse_lock_state_ == MouseLockCancelling);

[Wez, 2013/09/05 20:24:45]

nit: cancel -> should_cancel?

[alexeypa (please no reviews), 2013/09/06 20:00:17]

Done.

+
+  // See if the operation succeeded.
+  if (error == PP_OK) {
+    mouse_lock_state_ = MouseLockOn;
+  } else {
+    mouse_lock_state_ = MouseLockOff;
+    EnableMousePointer();
+  }
+
+  // Cancel as needed.
+  if (cancel)
+    CancelMouseLock();

[Wez, 2013/09/05 20:24:45]

So we can't dispatch the mouse-lock cancel until a previous mouse-lock request
has completed? That seems an unnecessary restriction in the API. :(

[alexeypa (please no reviews), 2013/09/06 20:00:17]

I don't know. I implemented it this way because the state logic is simpler this
way.

+}
+
 }  // namespace remoting