refactoring mouse lock to support pepper and WebKit

content/renderer/mouse_lock_dispatcher.h

+// Copyright (c) 2011 The Chromium Authors. All rights reserved.

[yzshen1, 2012/01/16 07:50:35]

2012. (Yes, apocalypse is very close now. :))

[scheib, 2012/01/17 22:05:36]

Done.

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef MOUSE_LOCK_DISPATCHER_H_

[yzshen1, 2012/01/16 07:50:35]

You should list the full path.

[scheib, 2012/01/17 22:05:36]

Done.

+#define MOUSE_LOCK_DISPATCHER_H_
+#pragma once
+
+#include "content/public/renderer/render_view_observer.h"
+
+class RenderViewImpl;
+class WebMouseEvent;

[yzshen1, 2012/01/16 07:50:35]

It is in WebKit namespace.

[scheib, 2012/01/17 22:05:36]

Done.

+
+namespace WebKit {
+class WebWidget;
+}  // namespace WebKit
+
+namespace webkit{
+namespace ppapi {
+class PluginInstance;
+}  // namespace ppapi
+}  // namespace webkit
+
+// MouseLockDispatcherDispatcher is owned by RenderViewImpl.

[yzshen1, 2012/01/16 07:50:35]

One redundant Dispatcher.

[scheib, 2012/01/17 22:05:36]

Done.

+class MouseLockDispatcher : public content::RenderViewObserver {
+ public:
+  explicit MouseLockDispatcher(RenderViewImpl* render_view_impl);
+  virtual ~MouseLockDispatcher();
+
+  bool LockMouse(WebKit::WebWidget* webwidget,

[yzshen1, 2012/01/16 07:50:35]

It is better to have the same interface for webwidget/pinstance to either
inherit or compose.
So we don't need to handle the two explicitly.

Is there any difficulty to do that?

[scheib, 2012/01/17 22:05:36]

I started that way, and showed darin. He did not like the idea of defining that
interface in the WebKit repository and then also using it for pepper. Because
the interface is so minor, and there are only two users of it, we agreed
together that a 'raw access' special casing the two was preferable.

[yzshen1, 2012/01/17 23:39:25]

Okay, if that has been carefully considered.

Please note that even if you don't define that interface in WebKit, you still
can use an adapter to wrap the webwidget, and expose an interface that is shared
by PluginInstance.
That seems better to me. But I will let you decide. :)

If you decide to leave it unchanged, please comment somewhere why you do it this
way. So that others can understand the decision easily.

[scheib, 2012/01/18 17:54:58]

We did consider an adapter as well, but that would require adding a pure
interface, an adapter, wrapping in the adapter, vs a few lines where we deal
with two pointers directly. We sided on the direct implementation, pros: clarity
of operation, still encapsulated in the dispatcher, not more lines of code and
files and indirection.

I've added comments describing the decision.

+                 webkit::ppapi::PluginInstance* pinstance);
+  void UnlockMouse(WebKit::WebWidget* webwidget,
+                   webkit::ppapi::PluginInstance* pinstance);
+  bool IsPointerLockedTo(WebKit::WebWidget* webwidget);
+  bool IsMouseLockedTo(webkit::ppapi::PluginInstance* pinstance);
+  bool WillHandleMouseEvent(const WebKit::WebMouseEvent& event);
+
+ private:
+  // RenderView::Observer implementation.
+  virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE;
+
+  // IPC handlers.
+  void OnLockMouseACK(bool succeeded);
+  void OnMouseLockLost();
+
+

[yzshen1, 2012/01/16 07:50:35]

One empty line is enough.

[scheib, 2012/01/17 22:05:36]

Done.

+  bool MouseLockedOrPendingAction() const {
+    return mouse_locked_ || pending_lock_request_ || pending_unlock_request_;
+  }
+
+  RenderViewImpl* render_view_impl_;
+
+  bool mouse_locked_;
+  // If both |pending_lock_request_| and |pending_unlock_request_| are true,
+  // it means a lock request was sent before an unlock request and we haven't
+  // received responses for them.
+  // The logic in LockMouse() makes sure that a lock request won't be sent when
+  // there is a pending unlock request.
+  bool pending_lock_request_;
+  bool pending_unlock_request_;
+
+  // |mouse_lock_webwidget_owner_| and |mouse_lock_pinstance_owner_|
+  // are not owned by this class.
+  // If they are non-NULL it doesn't indicate that the
+  // mouse has been locked. It is possible that a request to lock the mouse has
+  // been sent, but the response hasn't arrived yet.
+  WebKit::WebWidget* mouse_lock_webwidget_owner_;
+  webkit::ppapi::PluginInstance* mouse_lock_pinstance_owner_;
+
+  DISALLOW_COPY_AND_ASSIGN(MouseLockDispatcher);

[yzshen1, 2012/01/16 07:50:35]

You should include .h for DISALLOW_COPY_AND_ASSIGN.

[scheib, 2012/01/17 22:05:36]

Done.

+};
+
+#endif  // MOUSE_LOCK_DISPATCHER_H_