Clean up names and remove unnecessary parameter

content/renderer/render_widget.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 "content/renderer/render_widget.h"
 
 #include <memory>
 #include <utility>
 
 #include "base/auto_reset.h"
@@ skipping 343 matching lines @@
       owner_delegate_(nullptr),
       next_paint_flags_(0),
       auto_resize_mode_(false),
       need_update_rect_for_auto_resize_(false),
       did_show_(false),
       is_hidden_(hidden),
       compositor_never_visible_(never_visible),
       is_fullscreen_granted_(false),
       display_mode_(blink::WebDisplayModeUndefined),
       ime_event_guard_(nullptr),
+      ime_requested_update_(false),
       closing_(false),
       host_closing_(false),
       is_swapped_out_(swapped_out),
       for_oopif_(false),
       text_input_type_(ui::TEXT_INPUT_TYPE_NONE),
       text_input_mode_(ui::TEXT_INPUT_MODE_DEFAULT),
       text_input_flags_(0),
       can_compose_inline_(true),
       composition_range_(gfx::Range::InvalidRange()),
       popup_type_(popup_type),
@@ skipping 543 matching lines @@
 void RenderWidget::UpdateVisualState() {
   GetWebWidget()->updateAllLifecyclePhases();
 }
 
 void RenderWidget::WillBeginCompositorFrame() {
   TRACE_EVENT0("gpu", "RenderWidget::willBeginCompositorFrame");
 
   // The UpdateTextInputState can result in further layout and possibly
   // enable GPU acceleration so they need to be called before any painting
   // is done.
-  UpdateTextInputState(ShowIme::HIDE_IME, ChangeSource::FROM_NON_IME);
+  UpdateTextInputState(ShowIme::DO_NOT_SHOW);
   UpdateSelectionBounds();
 
   for (auto& observer : render_frame_proxies_)
     observer.WillBeginCompositorFrame();
 }
 
 std::unique_ptr<cc::SwapPromise> RenderWidget::RequestCopyOfOutputForLayoutTest(
     std::unique_ptr<cc::CopyOutputRequest> request) {
   return RenderThreadImpl::current()->RequestCopyOfOutputForLayoutTest(
       routing_id_, std::move(request));
@@ skipping 71 matching lines @@
   }
 }
 
 void RenderWidget::SetInputHandler(RenderWidgetInputHandler* input_handler) {
   // Nothing to do here. RenderWidget created the |input_handler| and will take
   // ownership of it. We just verify here that we don't already have an input
   // handler.
   DCHECK(!input_handler_);
 }
 
-void RenderWidget::UpdateTextInputState(ShowIme show_ime,
-                                        ChangeSource change_source) {
+void RenderWidget::UpdateTextInputState(ShowIme show_ime) {
   TRACE_EVENT0("renderer", "RenderWidget::UpdateTextInputState");
+  bool update_requested = ime_requested_update_;

[aelias_OOO_until_Jul13, 2017/01/04 00:59:51]

I don't think we should use a member variable to effectively pass an argument. 
If you want to clean up the majority of the callsites, I suggest doing the
following instead:

[private] void RenderWidget::SendTextInputState(ShowIme show_ime, bool
reply_to_request) {
  ...
}

void RenderWidget::UpdateTextInputState(ShowIme show_ime) {
  SendTextInputState(show_ime, false);
}

[Changwan Ryu, 2017/01/04 06:43:46]

Sounds like a good idea. Changed it to UpdateTextInputStateInternal().

+  ime_requested_update_ = false;
+
   if (ime_event_guard_) {
+    DCHECK(!update_requested);
     // show_ime should still be effective even if it was set inside the IME
     // event guard.
     if (show_ime == ShowIme::IF_NEEDED) {
       ime_event_guard_->set_show_ime(true);
     }
     return;
   }
 
   ui::TextInputType new_type = GetTextInputType();
   if (IsDateTimeInput(new_type))
     return;  // Not considered as a text input field in WebKit/Chromium.
 
   blink::WebTextInputInfo new_info;
   if (GetWebWidget())
     new_info = GetWebWidget()->textInputInfo();
   const ui::TextInputMode new_mode =
       ConvertWebTextInputMode(new_info.inputMode);
 
   bool new_can_compose_inline = CanComposeInline();
 
   // Only sends text input params if they are changed or if the ime should be
   // shown.
-  if (show_ime == ShowIme::IF_NEEDED ||
-      change_source == ChangeSource::FROM_IME ||
+  if (show_ime == ShowIme::IF_NEEDED || update_requested ||
       text_input_type_ != new_type || text_input_mode_ != new_mode ||
       text_input_info_ != new_info ||
       can_compose_inline_ != new_can_compose_inline) {
     TextInputState params;
     params.type = new_type;
     params.mode = new_mode;
     params.flags = new_info.flags;
     params.value = new_info.value.utf8();
     params.selection_start = new_info.selectionStart;
     params.selection_end = new_info.selectionEnd;
     params.composition_start = new_info.compositionStart;
     params.composition_end = new_info.compositionEnd;
     params.can_compose_inline = new_can_compose_inline;
     params.show_ime_if_needed = (show_ime == ShowIme::IF_NEEDED);
 #if defined(OS_ANDROID) || defined(USE_AURA)

[aelias_OOO_until_Jul13, 2017/01/04 00:59:50]

Please remove "#if defined(OS_ANDROID) || defined(USE_AURA)" as it should be
no-op on other platforms anyway.

[Changwan Ryu, 2017/01/04 06:43:46]

Done.

-    params.is_non_ime_change = (change_source == ChangeSource::FROM_NON_IME);
+    params.update_requested = update_requested;

[aelias_OOO_until_Jul13, 2017/01/04 00:59:50]

I'd prefer name "params.reply_to_request"

[Changwan Ryu, 2017/01/04 06:43:46]

Changed.

 #endif
     Send(new ViewHostMsg_TextInputStateChanged(routing_id(), params));
 
     text_input_info_ = new_info;
     text_input_type_ = new_type;
     text_input_mode_ = new_mode;
     can_compose_inline_ = new_can_compose_inline;
     text_input_flags_ = new_info.flags;
   }
 }
@@ skipping 632 matching lines @@
 }
 
 void RenderWidget::OnDragSourceSystemDragEnded() {
   if (!GetWebWidget())
     return;
 
   static_cast<WebFrameWidget*>(GetWebWidget())->dragSourceSystemDragEnded();
 }
 
 void RenderWidget::showImeIfNeeded() {
 #if defined(OS_ANDROID) || defined(USE_AURA)

[aelias_OOO_until_Jul13, 2017/01/04 00:59:51]

Can this #if be removed as well?

[Changwan Ryu, 2017/01/04 06:43:46]

Done. MacOSX does not seem to use it.

-  UpdateTextInputState(ShowIme::IF_NEEDED, ChangeSource::FROM_NON_IME);
+  UpdateTextInputState(ShowIme::IF_NEEDED);
 #endif
 
 // TODO(rouslan): Fix ChromeOS and Windows 8 behavior of autofill popup with
 // virtual keyboard.
 #if !defined(OS_ANDROID)
   FocusChangeComplete();
 #endif
 }
 
 ui::TextInputType RenderWidget::GetTextInputType() {
@@ skipping 52 matching lines @@
     rect->y *= GetOriginalDeviceScaleFactor();
     rect->width *= GetOriginalDeviceScaleFactor();
     rect->height *= GetOriginalDeviceScaleFactor();
   }
 }
 
 #if defined(OS_ANDROID)
 void RenderWidget::OnRequestTextInputStateUpdate() {
   DCHECK(!ime_event_guard_);
   UpdateSelectionBounds();
-  UpdateTextInputState(ShowIme::HIDE_IME, ChangeSource::FROM_IME);
+  ime_requested_update_ = true;
+  UpdateTextInputState(ShowIme::DO_NOT_SHOW);
 }
 #endif
 
 void RenderWidget::OnRequestCompositionUpdate(bool immediate_request,
                                               bool monitor_request) {
   monitor_composition_info_ = monitor_request;
   if (!immediate_request)
     return;
   UpdateCompositionInfo(true /* immediate request */);
 }
@@ skipping 53 matching lines @@
   next_paint_flags_ |= ViewHostMsg_UpdateRect_Flags::IS_REPAINT_ACK;
 }
 
 void RenderWidget::OnImeEventGuardStart(ImeEventGuard* guard) {
   if (!ime_event_guard_)
     ime_event_guard_ = guard;
 }
 
 void RenderWidget::OnImeEventGuardFinish(ImeEventGuard* guard) {
   if (ime_event_guard_ != guard) {
-#if defined(OS_ANDROID)
-    // In case a from-IME event (e.g. touch) ends up in not-from-IME event
-    // (e.g. long press gesture), we want to treat it as not-from-IME event
-    // so that ReplicaInputConnection can make changes to its Editable model.
-    // Therefore, we want to mark this text state update as 'from IME' only
-    // when all the nested events are all originating from IME.
-    ime_event_guard_->set_from_ime(
-        ime_event_guard_->from_ime() && guard->from_ime());
-#endif
+    DCHECK(!ime_event_guard_->update_requested());
     return;
   }
   ime_event_guard_ = nullptr;
 
   // While handling an ime event, text input state and selection bounds updates
   // are ignored. These must explicitly be updated once finished handling the
   // ime event.
   UpdateSelectionBounds();
 #if defined(OS_ANDROID)

[aelias_OOO_until_Jul13, 2017/01/04 00:59:50]

Maybe for another patch, but why is this #if OS(ANDROID)?  Seems weird.

[Changwan Ryu, 2017/01/04 06:43:46]

Filed crbug.com/678163 .

-  UpdateTextInputState(
-      guard->show_ime() ? ShowIme::IF_NEEDED : ShowIme::HIDE_IME,
-      guard->from_ime() ? ChangeSource::FROM_IME : ChangeSource::FROM_NON_IME);
+  UpdateTextInputState(guard->show_ime() ? ShowIme::IF_NEEDED
+                                         : ShowIme::DO_NOT_SHOW);
 #endif
 }
 
 void RenderWidget::GetSelectionBounds(gfx::Rect* focus, gfx::Rect* anchor) {
 #if BUILDFLAG(ENABLE_PLUGINS)
   if (focused_pepper_plugin_) {
     // TODO(kinaba) http://crbug.com/101101
     // Current Pepper IME API does not handle selection bounds. So we simply
     // use the caret position as an empty range for now. It will be updated
     // after Pepper API equips features related to surrounding text retrieval.
@@ skipping 194 matching lines @@
 }
 #endif
 
 void RenderWidget::didHandleGestureEvent(
     const WebGestureEvent& event,
     bool event_cancelled) {
 #if defined(OS_ANDROID) || defined(USE_AURA)
   if (event_cancelled)
     return;
   if (event.type == WebInputEvent::GestureTap) {
-    UpdateTextInputState(ShowIme::IF_NEEDED, ChangeSource::FROM_NON_IME);
+    UpdateTextInputState(ShowIme::IF_NEEDED);
   } else if (event.type == WebInputEvent::GestureLongPress) {
     DCHECK(GetWebWidget());
     if (GetWebWidget()->textInputInfo().value.isEmpty())
-      UpdateTextInputState(ShowIme::HIDE_IME, ChangeSource::FROM_NON_IME);
+      UpdateTextInputState(ShowIme::DO_NOT_SHOW);
     else
-      UpdateTextInputState(ShowIme::IF_NEEDED, ChangeSource::FROM_NON_IME);
+      UpdateTextInputState(ShowIme::IF_NEEDED);
   }
 // TODO(ananta): Piggyback off existing IPCs to communicate this information,
 // crbug/420130.
 #if defined(OS_WIN)
   if (event.type != blink::WebGestureEvent::GestureTap)
     return;
 
   // TODO(estade): hit test the event against focused node to make sure
   // the tap actually hit the focused node.
   blink::WebTextInputType text_input_type = GetWebWidget()->textInputType();
@@ skipping 127 matching lines @@
     // browser side (https://crbug.com/669219).
     // If there is no WebFrameWidget, then there will be no
     // InputMethodControllers for a WebLocalFrame.
     return nullptr;
   }
   return static_cast<blink::WebFrameWidget*>(GetWebWidget())
       ->getActiveWebInputMethodController();
 }
 
 }  // namespace content