Extracting placeholder information from Webkit to Blimp

blimp/engine/session/blimp_engine_session.cc

 // Copyright 2015 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 "blimp/engine/session/blimp_engine_session.h"
 
 #include <string>
 #include <utility>
 
 #include "base/command_line.h"
@@ skipping 22 matching lines @@
 #include "blimp/net/blimp_stats.h"
 #include "blimp/net/blob_channel/blob_channel_sender_impl.h"
 #include "blimp/net/blob_channel/helium_blob_sender_delegate.h"
 #include "blimp/net/browser_connection_handler.h"
 #include "blimp/net/common.h"
 #include "blimp/net/engine_authentication_handler.h"
 #include "blimp/net/engine_connection_manager.h"
 #include "blimp/net/null_blimp_message_processor.h"
 #include "blimp/net/tcp_engine_transport.h"
 #include "blimp/net/thread_pipe_manager.h"
+#include "content/browser/web_contents/web_contents_impl.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/render_widget_host.h"
 #include "content/public/browser/render_widget_host_view.h"
 #include "content/public/browser/web_contents.h"
 #include "device/geolocation/geolocation_delegate.h"
 #include "device/geolocation/geolocation_provider.h"
@@ skipping 190 matching lines @@
       base::MakeUnique<InMemoryBlobCache>(), std::move(helium_blob_delegate));
   blob_channel_sender_weak_factory_ =
       base::MakeUnique<base::WeakPtrFactory<BlobChannelSenderImpl>>(
           blob_channel_sender_.get());
 
   device::GeolocationProvider::SetGeolocationDelegate(
       geolocation_feature_.CreateGeolocationDelegate());
 }
 
 BlimpEngineSession::~BlimpEngineSession() {
-  window_tree_host_->GetInputMethod()->RemoveObserver(this);
-
   // Ensure that all tabs are torn down first, since teardown will
   // trigger RenderViewDeleted callbacks to their observers, which will in turn
   // send messages to net_components_, which is already deleted due to the line
   // below.
   tab_.reset();
 
   // Safely delete network components on the IO thread.
   content::BrowserThread::DeleteSoon(content::BrowserThread::IO, FROM_HERE,
                                      net_components_.release());
 }
@@ skipping 12 matching lines @@
   aura::client::SetFocusClient(window_tree_host_->window(),
                                focus_client_.get());
   aura::client::SetActivationClient(window_tree_host_->window(),
                                     focus_client_.get());
   capture_client_.reset(
       new aura::client::DefaultCaptureClient(window_tree_host_->window()));
 
   window_tree_client_.reset(
       new BlimpWindowTreeClient(window_tree_host_->window()));
 
-  window_tree_host_->GetInputMethod()->AddObserver(this);
-
   window_tree_host_->SetBounds(gfx::Rect(screen_->GetPrimaryDisplay().size()));
 
   RegisterFeatures();
 
   // Initialize must only be posted after the RegisterFeature calls have
   // completed.
   content::BrowserThread::PostTask(
       content::BrowserThread::IO, FROM_HERE,
       base::Bind(&EngineNetworkComponents::Initialize,
                  base::Unretained(net_components_.get()),
@@ skipping 75 matching lines @@
                                       const gfx::Size& size) {
   DVLOG(1) << "Resize to " << size.ToString() << ", " << device_pixel_ratio;
   screen_->UpdateDisplayScaleAndSize(device_pixel_ratio, size);
   window_tree_host_->SetBounds(gfx::Rect(size));
   if (tab_) {
     tab_->Resize(device_pixel_ratio,
                  screen_->GetPrimaryDisplay().bounds().size());
   }
 }
 
-void BlimpEngineSession::OnTextInputTypeChanged(
-    const ui::TextInputClient* client) {}
+void BlimpEngineSession::OnUpdateTextInputStateCalled(

[EhsanK, 2016/09/28 16:14:11]

I guess I see why you had to observe TextInputManager. Basically, you need to
know the RWH which changed its TextInputState. This is a totally different
structure then the rest of InputMethod and TextInputClient that I know of.

Passing some info about RWH to InputMethod is also not a good idea since AFAIK
InputMethod does not really care which RWH is focused or not; as long as there
is a client which consumes the results methods.

But could there be something added to WebContentsDelegate about the change
instead of exposing all this? We are exposing TextInputManager,
RenderWidgetHostViewAura and WebContentsImpl which are all content/ internal.

[shaktisahu, 2016/09/28 20:17:23]

I think I pipe this call to RenderWidgetHostDelegate -> WebContentsDelegate ->
BlimpEngineSession. That makes more sense and in that way we don't have to
expose any of the content internal stuff.

[EhsanK, 2016/10/03 14:53:10]

I too think going through the delegates is better.

+    content::TextInputManager* text_input_manager,
+    content::RenderWidgetHostViewBase* updated_view,
+    bool did_update_state) {
+  const content::TextInputState* state =
+      text_input_manager->GetTextInputState();
 
-void BlimpEngineSession::OnFocus() {}
-
-void BlimpEngineSession::OnBlur() {}
-
-void BlimpEngineSession::OnCaretBoundsChanged(
-    const ui::TextInputClient* client) {}
-
-// Called when either:
-//  - the TextInputClient is changed (e.g. by a change of focus)
-//  - the TextInputType of the TextInputClient changes
-void BlimpEngineSession::OnTextInputStateChanged(
-    const ui::TextInputClient* client) {
-  if (!tab_ || !tab_->web_contents()->GetRenderWidgetHostView())
+  if (!state)
     return;
 
-  ui::TextInputType type =
-      client ? client->GetTextInputType() : ui::TEXT_INPUT_TYPE_NONE;
+  if (state->show_ime_if_needed) {
+    render_widget_feature_.SendShowImeRequest(
+        tab_->tab_id(), updated_view->GetRenderWidgetHost(), state);

[EhsanK, 2016/09/28 16:14:11]

RWHV::GetRenderWidgetHost() may return nullptr. I could not tell if this would
cause issues down the stack. As I see it it will be used for a map to retrieve a
RWHId. I assume the map will not find any id for a nullptr entry?

[shaktisahu, 2016/09/28 20:17:23]

Ya, this is checked. The function will return 0 for nullptr entry and we are
DCHECKing the id for non-zero after that. So indirectly the DCHECK checks
thatthe GetRenderWidgetHost() should return non-null.

[EhsanK, 2016/10/03 14:53:10]

Acknowledged.

+  }
 
-  // TODO(shaktisahu): Propagate the new type to the client.
-  // Hide IME, when text input is out of focus, i.e. if the text input type
-  // changes to ui::TEXT_INPUT_TYPE_NONE. For other text input types,
-  // OnShowImeIfNeeded is used instead to send show IME request to client.
-  if (type == ui::TEXT_INPUT_TYPE_NONE)
+  if (state->type == ui::TEXT_INPUT_TYPE_NONE) {
     render_widget_feature_.SendHideImeRequest(
-        tab_->tab_id(),
-        tab_->web_contents()->GetRenderWidgetHostView()->GetRenderWidgetHost());
-}
-
-void BlimpEngineSession::OnInputMethodDestroyed(
-    const ui::InputMethod* input_method) {}
-
-// Called when a user input should trigger showing the IME.
-void BlimpEngineSession::OnShowImeIfNeeded() {
-  TRACE_EVENT0("blimp", "BlimpEngineSession::OnShowImeIfNeeded");
-  if (!tab_ || !tab_->web_contents()->GetRenderWidgetHostView() ||
-      !window_tree_host_->GetInputMethod()->GetTextInputClient())
-    return;
-
-  render_widget_feature_.SendShowImeRequest(
-      tab_->tab_id(),
-      tab_->web_contents()->GetRenderWidgetHostView()->GetRenderWidgetHost(),
-      window_tree_host_->GetInputMethod()->GetTextInputClient());
+        tab_->tab_id(), updated_view->GetRenderWidgetHost());
+  }
 }
 
 void BlimpEngineSession::ProcessMessage(
     std::unique_ptr<BlimpMessage> message,
     const net::CompletionCallback& callback) {
   TRACE_EVENT1("blimp", "BlimpEngineSession::ProcessMessage", "TabId",
                message->target_tab_id());
   DCHECK(!callback.is_null());
   DCHECK(BlimpMessage::kTabControl == message->feature_case() ||
          BlimpMessage::kNavigation == message->feature_case());
@@ skipping 82 matching lines @@
 }
 
 void BlimpEngineSession::RequestToLockMouse(content::WebContents* web_contents,
                                             bool user_gesture,
                                             bool last_unlocked_by_target) {
   web_contents->GotResponseToLockMouseRequest(true);
 }
 
 void BlimpEngineSession::CloseContents(content::WebContents* source) {
   if (source == tab_->web_contents()) {
+    static_cast<content::WebContentsImpl*>(source)
+        ->GetTextInputManager()
+        ->RemoveObserver(this);
     tab_.reset();
   }
 }
 
 void BlimpEngineSession::ActivateContents(content::WebContents* contents) {
   contents->GetRenderViewHost()->GetWidget()->Focus();
 }
 
 void BlimpEngineSession::ForwardCompositorProto(
     content::RenderWidgetHost* render_widget_host,
@@ skipping 18 matching lines @@
     std::unique_ptr<content::WebContents> new_contents,
     const int target_tab_id) {
   new_contents->SetDelegate(this);
 
   aura::Window* parent = window_tree_host_->window();
   aura::Window* content = new_contents->GetNativeView();
   if (!parent->Contains(content))
     parent->AddChild(content);
   content->Show();
 
+  static_cast<content::WebContentsImpl*>(new_contents.get())
+      ->GetTextInputManager()
+      ->AddObserver(this);
+
   tab_ = base::MakeUnique<Tab>(std::move(new_contents), target_tab_id,
                                &render_widget_feature_,
                                navigation_message_sender_.get());
 }
 
 }  // namespace engine
 }  // namespace blimp