Respect the order of input messages from browser to renderer

content/renderer/render_frame_impl.cc

 // Copyright 2013 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 "content/renderer/render_frame_impl.h"
 
 #include <map>
 #include <string>
 #include <utility>
 #include <vector>
@@ skipping 78 matching lines @@
 #include "content/renderer/context_menu_params_builder.h"
 #include "content/renderer/devtools/devtools_agent.h"
 #include "content/renderer/dom_automation_controller.h"
 #include "content/renderer/effective_connection_type_helper.h"
 #include "content/renderer/external_popup_menu.h"
 #include "content/renderer/gpu/gpu_benchmarking_extension.h"
 #include "content/renderer/history_controller.h"
 #include "content/renderer/history_serialization.h"
 #include "content/renderer/image_downloader/image_downloader_impl.h"
 #include "content/renderer/ime_event_guard.h"
+#include "content/renderer/input/input_handler_manager.h"
 #include "content/renderer/internal_document_state_data.h"
 #include "content/renderer/manifest/manifest_manager.h"
 #include "content/renderer/media/audio_device_factory.h"
 #include "content/renderer/media/media_permission_dispatcher.h"
 #include "content/renderer/media/media_stream_dispatcher.h"
 #include "content/renderer/media/media_stream_renderer_factory_impl.h"
 #include "content/renderer/media/midi_dispatcher.h"
 #include "content/renderer/media/render_media_log.h"
 #include "content/renderer/media/renderer_webmediaplayer_delegate.h"
 #include "content/renderer/media/user_media_client_impl.h"
@@ skipping 1105 matching lines @@
     // DevToolsAgent is a RenderFrameObserver, and will destruct itself
     // when |this| is deleted.
     devtools_agent_ = new DevToolsAgent(this);
   }
 
   RegisterMojoServices();
 
   // We delay calling this until we have the WebFrame so that any observer or
   // embedder can call GetWebFrame on any RenderFrame.
   GetContentClient()->renderer()->RenderFrameCreated(this);
+
+  // TODO(changwan): apply this logic to RenderWidget once we add input handler

[no sievers, 2016/06/01 22:14:47]

nit: i'd remove this TODO here since IIUC you intend to leave the code here as
is

[Changwan Ryu, 2016/06/03 06:48:29]

Removed.

+  // in RenderWidget, too.
+  if (render_view_ && render_view_->HasAddedInputHandler()) {

[no sievers, 2016/06/01 22:14:47]

This looks fragile. Do you expect HasAddedInputHandler() to be always true?
It just seems like call sequences could change somewhere, and your change would
silently become nonfunctional.

[Changwan Ryu, 2016/06/02 10:49:06]

Two hidden assumptions here:
1) initializeLayerTreeView() is part of RenderViewImpl::Initialize().
2) RenderViewImpl::Initialize() is created before RenderFrameImpl::Initialize().

I think as long as those two are met, RegisterRoutingID() should be called. I am
not sure if those assumptions are easily breakable.

But there is a chance that this fix can break in the future because there is no
rigorous test around it. I'll try a bit more but I found it challenging to add a
test that instantiates a real RenderThreadImpl object and test it against
RenderViewImpl / RenderFrameImpl / RenderWidget while pausing and resuming the
impl thread.

[Changwan Ryu, 2016/06/03 06:48:29]

Removed render_view check and changed to DCHECK(). Let me know if there is more
to do to.

+    RenderThreadImpl* render_thread = RenderThreadImpl::current();
+    // render_thread may be NULL in tests.
+    InputHandlerManager* input_handler_manager =
+        render_thread ? render_thread->input_handler_manager() : nullptr;
+    if (input_handler_manager)
+      input_handler_manager->RegisterRoutingID(GetRoutingID());

[no sievers, 2016/06/01 22:14:47]

How does it get unregistered?

[Changwan Ryu, 2016/06/03 06:48:29]

Now it's unregistered in ~RenderFrameImpl().

+  }
 }
 
 void RenderFrameImpl::InitializeBlameContext(RenderFrameImpl* parent_frame) {
   DCHECK(!blame_context_);
   blame_context_ = new FrameBlameContext(this, parent_frame);
   blame_context_->Initialize();
 }
 
 void RenderFrameImpl::OnGotZoomLevel(const GURL& url, double zoom_level) {
   // TODO(wjmaclean): We should see if this restriction is really necessary,
@@ skipping 271 matching lines @@
                         OnAdjustSelectionByCharacterOffset)
     IPC_MESSAGE_HANDLER(InputMsg_Unselect, OnUnselect)
     IPC_MESSAGE_HANDLER(InputMsg_MoveRangeSelectionExtent,
                         OnMoveRangeSelectionExtent)
     IPC_MESSAGE_HANDLER(InputMsg_Replace, OnReplace)
     IPC_MESSAGE_HANDLER(InputMsg_ReplaceMisspelling, OnReplaceMisspelling)
     IPC_MESSAGE_HANDLER(InputMsg_ExtendSelectionAndDelete,
                         OnExtendSelectionAndDelete)
     IPC_MESSAGE_HANDLER(InputMsg_SetCompositionFromExistingText,
                         OnSetCompositionFromExistingText)
+    IPC_MESSAGE_HANDLER(InputMsg_SetEditableSelectionOffsets,
+                        OnSetEditableSelectionOffsets)
     IPC_MESSAGE_HANDLER(InputMsg_ExecuteNoValueEditCommand,
                         OnExecuteNoValueEditCommand)
     IPC_MESSAGE_HANDLER(FrameMsg_CSSInsertRequest, OnCSSInsertRequest)
     IPC_MESSAGE_HANDLER(FrameMsg_AddMessageToConsole, OnAddMessageToConsole)
     IPC_MESSAGE_HANDLER(FrameMsg_JavaScriptExecuteRequest,
                         OnJavaScriptExecuteRequest)
     IPC_MESSAGE_HANDLER(FrameMsg_JavaScriptExecuteRequestForTests,
                         OnJavaScriptExecuteRequestForTests)
     IPC_MESSAGE_HANDLER(FrameMsg_JavaScriptExecuteRequestInIsolatedWorld,
                         OnJavaScriptExecuteRequestInIsolatedWorld)
     IPC_MESSAGE_HANDLER(FrameMsg_VisualStateRequest,
                         OnVisualStateRequest)
-    IPC_MESSAGE_HANDLER(FrameMsg_SetEditableSelectionOffsets,
-                        OnSetEditableSelectionOffsets)
     IPC_MESSAGE_HANDLER(FrameMsg_Reload, OnReload)
     IPC_MESSAGE_HANDLER(FrameMsg_ReloadLoFiImages, OnReloadLoFiImages)
     IPC_MESSAGE_HANDLER(FrameMsg_TextSurroundingSelectionRequest,
                         OnTextSurroundingSelectionRequest)
     IPC_MESSAGE_HANDLER(FrameMsg_SetAccessibilityMode,
                         OnSetAccessibilityMode)
     IPC_MESSAGE_HANDLER(AccessibilityMsg_SnapshotTree,
                         OnSnapshotAccessibilityTree)
     IPC_MESSAGE_HANDLER(FrameMsg_UpdateOpener, OnUpdateOpener)
     IPC_MESSAGE_HANDLER(FrameMsg_CommitNavigation, OnCommitNavigation)
@@ skipping 4677 matching lines @@
   // event target. Potentially a Pepper plugin will receive the event.
   // In order to tell whether a plugin gets the last mouse event and which it
   // is, we set |pepper_last_mouse_event_target_| to null here. If a plugin gets
   // the event, it will notify us via DidReceiveMouseEvent() and set itself as
   // |pepper_last_mouse_event_target_|.
   pepper_last_mouse_event_target_ = nullptr;
 #endif
 }
 
 }  // namespace content