Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(355)

Unified Diff: content/renderer/render_widget.cc

Issue 2596193002: Clean up names and remove unnecessary parameter (Closed)
Patch Set: use correct patch Created 3 years, 12 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « content/renderer/render_widget.h ('k') | content/test/layouttest_support.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: content/renderer/render_widget.cc
diff --git a/content/renderer/render_widget.cc b/content/renderer/render_widget.cc
index a09a0f216a4b47be7a3c3ca28901268f53b5cd19..e7e28d5597254f66b63ec05e153fafd181bbf532 100644
--- a/content/renderer/render_widget.cc
+++ b/content/renderer/render_widget.cc
@@ -361,6 +361,7 @@ RenderWidget::RenderWidget(int32_t widget_routing_id,
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),
@@ -924,7 +925,7 @@ void 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_)
@@ -1016,10 +1017,13 @@ void RenderWidget::SetInputHandler(RenderWidgetInputHandler* 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 e
Changwan Ryu 2017/01/04 06:43:46 Sounds like a good idea. Changed it to UpdateTextI
+ 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) {
@@ -1042,8 +1046,7 @@ void RenderWidget::UpdateTextInputState(ShowIme show_ime,
// 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) {
@@ -1059,7 +1062,7 @@ void RenderWidget::UpdateTextInputState(ShowIme show_ime,
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(
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));
@@ -1713,7 +1716,7 @@ void RenderWidget::OnDragSourceSystemDragEnded() {
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
@@ -1786,7 +1789,8 @@ void RenderWidget::convertWindowToViewport(blink::WebFloatRect* rect) {
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
@@ -1860,15 +1864,7 @@ void RenderWidget::OnImeEventGuardStart(ImeEventGuard* 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;
@@ -1878,9 +1874,8 @@ void RenderWidget::OnImeEventGuardFinish(ImeEventGuard* guard) {
// 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(AN
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
}
@@ -2095,13 +2090,13 @@ void RenderWidget::didHandleGestureEvent(
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.
« no previous file with comments | « content/renderer/render_widget.h ('k') | content/test/layouttest_support.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698