Delays find-in-page until IME composition is committed.

chrome/browser/ui/views/find_bar_view.cc

Index: chrome/browser/ui/views/find_bar_view.cc
diff --git a/chrome/browser/ui/views/find_bar_view.cc b/chrome/browser/ui/views/find_bar_view.cc
index dcc61fc7cf2b9c500e9492db40dac393802e4336..e0d3966ec76565f41ba26dbfe3a93e72efe37420 100644
--- a/chrome/browser/ui/views/find_bar_view.cc
+++ b/chrome/browser/ui/views/find_bar_view.cc
@@ -395,8 +395,34 @@ void FindBarView::ButtonPressed(
 ////////////////////////////////////////////////////////////////////////////////
 // FindBarView, views::TextfieldController implementation:
 
-void FindBarView::ContentsChanged(views::Textfield* sender,
-                                  const string16& new_contents) {
+bool FindBarView::HandleKeyEvent(views::Textfield* sender,
+                                 const ui::KeyEvent& key_event) {
+  // If the dialog is not visible, there is no reason to process keyboard input.
+  if (!host()->IsVisible())
+    return false;
+
+  if (find_bar_host()->MaybeForwardKeyEventToWebpage(key_event))
+    return true;  // Handled, we are done!
+
+  if (key_event.key_code() == ui::VKEY_RETURN) {
+    // Pressing Return/Enter starts the search (unless text box is empty).
+    string16 find_string = find_text_->text();
+    if (!find_string.empty()) {
+      FindBarController* controller = find_bar_host()->GetFindBarController();
+      FindTabHelper* find_tab_helper =
+          FindTabHelper::FromWebContents(controller->web_contents());
+      // Search forwards for enter, backwards for shift-enter.
+      find_tab_helper->StartFinding(find_string,
+                                    !key_event.IsShiftDown(),
+                                    false);  // Not case sensitive.
+    }
+    return true;
+  }
+
+  return false;
+}
+
+void FindBarView::OnAfterUserAction(views::Textfield* sender) {
   FindBarController* controller = find_bar_host()->GetFindBarController();
   DCHECK(controller);
   content::WebContents* web_contents = controller->web_contents();
@@ -407,12 +433,21 @@ void FindBarView::ContentsChanged(views::Textfield* sender,
     return;
   FindTabHelper* find_tab_helper = FindTabHelper::FromWebContents(web_contents);
 
+  // The composition text wouldn't be what the user is really looking for.
+  // We delay the search until the user commits the composition text.
+  if (sender->IsIMEComposing() ||
+      sender->text() == last_searched_text_)

[msw, 2013/06/01 01:58:22]

Does this actually get hit when you cancel a composition? If there's not
evidence of an actual bug like that, then I'd remove it. Paste worked without
this check and without OnAfterPaste, afaik.


If something like this is really needed, can you avoid OnAfterPaste? Try either
keeping the ContentsChanged override to trigger search (since the workaround is
already in NativeTextfield[Win|Views]::Paste), or overriding
TextfieldController's  ExecuteCommand to trigger search on IDS_APP_PASTE. If you
keep OnAfterPaste, then you can get rid of the NativeTextfield[Win|Views]::Paste
workaround. I guess triggering a search (without an equality check) means
calling SetFindText with the current text or maybe splitting out the bottom of
this function :-/

[Yuki, 2013/06/03 10:07:18]

Yes, this line gets hit when you cancel a composition, you turned a IME on or
off.  So we need this equality check.

I tried avoiding OnAfterPaste, but it didn't work well.
When you type [日本],
    1) the composition becomes "日本",
    2) and then the composition is committed.
ContentsChanged() is called at step 1, but we cannot say it's the text the user
is looking for, because it's not yet committed.
At step 2, ContentsChanged() is NOT called because the text "日本" has not changed
since step 1.
This is why I needed to move the code from ContentsChanged() to
OnAfterUserAction(), where we can tell if the user has committed the composition
or not.

Overriding ExecuteCommand() doesn't seem good to me, because it's not a hook. 
If we handled IDS_APP_PASTE in ExecuteCommand(), we have to implement the actual
paste operation including calling On{Before,After}UserAction(), notifying
changes of text and caret bounds, etc.  It's too much.

Thus, I'd like to introduce OnAfterPaste() to detect a paste operation.
I've removed the duplicated hack for Paste from NativeTextfieldViews.  I
realized that NativeTextfieldWin doesn't support On{Before,After}UserAction()
and I need to keep the old implementation for NativeTextfieldWin.  So I reverted
ContentsChanged() in case that IsViewsTextfieldEnabled() is false.

+    return;
+
+  const string16& search_text = sender->text();
+  last_searched_text_ = search_text;
+
   // When the user changes something in the text box we check the contents and
   // if the textbox contains something we set it as the new search string and
   // initiate search (even though old searches might be in progress).
-  if (!new_contents.empty()) {
+  if (!search_text.empty()) {
     // The last two params here are forward (true) and case sensitive (false).
-    find_tab_helper->StartFinding(new_contents, true, false);
+    find_tab_helper->StartFinding(search_text, true, false);
   } else {
     find_tab_helper->StopFinding(FindBarController::kClearSelectionOnPage);
     UpdateForResult(find_tab_helper->find_result(), string16());
@@ -430,33 +465,6 @@ void FindBarView::ContentsChanged(views::Textfield* sender,
   }
 }
 
-bool FindBarView::HandleKeyEvent(views::Textfield* sender,
-                                 const ui::KeyEvent& key_event) {
-  // If the dialog is not visible, there is no reason to process keyboard input.
-  if (!host()->IsVisible())
-    return false;
-
-  if (find_bar_host()->MaybeForwardKeyEventToWebpage(key_event))
-    return true;  // Handled, we are done!
-
-  if (key_event.key_code() == ui::VKEY_RETURN) {
-    // Pressing Return/Enter starts the search (unless text box is empty).
-    string16 find_string = find_text_->text();
-    if (!find_string.empty()) {
-      FindBarController* controller = find_bar_host()->GetFindBarController();
-      FindTabHelper* find_tab_helper =
-          FindTabHelper::FromWebContents(controller->web_contents());
-      // Search forwards for enter, backwards for shift-enter.
-      find_tab_helper->StartFinding(find_string,
-                                    !key_event.IsShiftDown(),
-                                    false);  // Not case sensitive.
-    }
-    return true;
-  }
-
-  return false;
-}
-
 void FindBarView::OnAfterCutOrCopy() {
   Profile* profile = host()->browser_view()->browser()->profile();
   ui::SourceTag source_tag =
@@ -474,6 +482,13 @@ void FindBarView::OnAfterCutOrCopy() {
   }
 }
 
+void FindBarView::OnAfterPaste() {
+  // Clear the last search text so we always search for the user input after
+  // a paste operation, even if the pasted text is the same as before.
+  // See http://crbug.com/79002
+  last_searched_text_.clear();
+}
+
 void FindBarView::UpdateMatchCountAppearance(bool no_match) {
   if (no_match) {
     match_count_text_->SetBackgroundColor(kBackgroundColorNoMatch);