Mac:Dont always enable findbar buttons when restoring focus with search string

Avoids enabling findbar buttons when there is no search match.

TEST=On mac: Search for something not in page, verify next/prev buttons
disabled due to no results, focus away from findbar, do ctrl-f and verify
buttons remain disabled.

BUG=634289

Review-Url: https://codereview.chromium.org/2928333002
Cr-Commit-Position: refs/heads/master@{#492002}

chrome/browser/ui/cocoa/find_bar/find_bar_browsertest.mm

@@ -3,9 +3,12 @@
 // found in the LICENSE file.
 
 #include "base/mac/foundation_util.h"
+#include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_window.h"
 #import "chrome/browser/ui/cocoa/browser_window_controller.h"
+#include "chrome/browser/ui/cocoa/find_bar/find_bar_bridge.h"
+#include "chrome/browser/ui/cocoa/find_bar/find_bar_cocoa_controller.h"
 #include "chrome/browser/ui/cocoa/find_bar/find_bar_text_field.h"
 #import "chrome/browser/ui/cocoa/test/run_loop_testing.h"
 #include "chrome/browser/ui/exclusive_access/fullscreen_controller_test.h"
@@ -14,17 +17,69 @@
 #include "chrome/browser/ui/location_bar/location_bar.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/base/ui_test_utils.h"
 #include "ui/base/test/ui_controls.h"
 #import "ui/events/test/cocoa_test_event_utils.h"
 
+// Expose private variables to make testing easier.
+@implementation FindBarCocoaController (Testing)
+- (NSButton*)nextButton {
+  return nextButton_;
+}
+
+- (NSButton*)previousButton {
+  return previousButton_;
+}
+@end
+
+using content::WebContents;
+using base::ASCIIToUTF16;
+
 namespace {
 
+const char kSimplePage[] = "simple.html";
+
 bool FindBarHasFocus(Browser* browser) {
   NSWindow* window = browser->window()->GetNativeWindow();
   NSText* text_view = base::mac::ObjCCast<NSText>([window firstResponder]);
   return [[text_view delegate] isKindOfClass:[FindBarTextField class]];
 }
 
+GURL GetTestURL(const std::string& filename) {
+  return ui_test_utils::GetTestUrl(base::FilePath().AppendASCII("find_in_page"),
+                                   base::FilePath().AppendASCII(filename));
+}
+
+FindBarCocoaController* GetFindBarCocoaController(Browser* browser) {
+  FindBarBridge* bridge =
+      static_cast<FindBarBridge*>(browser->GetFindBarController()->find_bar());
+  return bridge->find_bar_cocoa_controller();
+}
+
+NSButton* GetFindBarControllerNextButton(Browser* browser) {
+  FindBarCocoaController* cocoacontroller = GetFindBarCocoaController(browser);
+  return [cocoacontroller nextButton];
+}
+
+NSButton* GetFindBarControllerPreviousButton(Browser* browser) {
+  FindBarCocoaController* cocoacontroller = GetFindBarCocoaController(browser);
+  return [cocoacontroller previousButton];
+}
+
+int GetFindInContentsMatchCount(WebContents* contents,
+                                const base::string16& search_string) {
+  return ui_test_utils::FindInPage(contents, search_string, true, false, NULL,
+                                   NULL);
+}
+
+void SimulateKeyPress(NSWindow* window, ui::KeyboardCode key) {
+  base::RunLoop run_loop;
+  ui_controls::EnableUIControls();
+  ui_controls::SendKeyPressNotifyWhenDone(window, key, false, false, false,
+                                          false, run_loop.QuitClosure());
+  run_loop.Run();
+}
+
 }  // namespace
 
 typedef InProcessBrowserTest FindBarBrowserTest;
@@ -52,6 +107,99 @@ IN_PROC_BROWSER_TEST_F(FindBarBrowserTest, FocusOnTabSwitch) {
   EXPECT_FALSE(FindBarHasFocus(browser()));
 }
 
+IN_PROC_BROWSER_TEST_F(FindBarBrowserTest,
+                       NextPreviousButtonsDisabledAfterFocusChange) {
+  ui_test_utils::NavigateToURL(browser(), GetTestURL(kSimplePage));
+  browser()->GetFindBarController()->Show();
+  browser()->GetFindBarController()->find_bar()->SetFocusAndSelection();
+  EXPECT_TRUE(FindBarHasFocus(browser()));
+
+  // Simulate key press for character not in page contents.
+  SimulateKeyPress(browser()->window()->GetNativeWindow(), ui::VKEY_Z);
+  WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents();
+  EXPECT_EQ(GetFindInContentsMatchCount(contents, ASCIIToUTF16("Z")), 0);
+
+  // Buttons should be disabled as there were no results.
+  NSButton* nextButton = GetFindBarControllerNextButton(browser());
+  NSButton* previousButton = GetFindBarControllerPreviousButton(browser());
+  EXPECT_FALSE([nextButton isEnabled]);
+  EXPECT_FALSE([previousButton isEnabled]);
+
+  // Move focus to the location bar then back to the find bar.
+  browser()->window()->GetLocationBar()->FocusLocation(true);
+  EXPECT_FALSE(FindBarHasFocus(browser()));
+  browser()->GetFindBarController()->find_bar()->SetFocusAndSelection();
+  EXPECT_TRUE(FindBarHasFocus(browser()));
+
+  // Buttons should remain disabled.
+  EXPECT_FALSE([nextButton isEnabled]);
+  EXPECT_FALSE([previousButton isEnabled]);
+}
+
+IN_PROC_BROWSER_TEST_F(FindBarBrowserTest,
+                       NextPreviousButtonsEnabledAfterFocusChange) {
+  ui_test_utils::NavigateToURL(browser(), GetTestURL(kSimplePage));
+  browser()->GetFindBarController()->Show();
+  browser()->GetFindBarController()->find_bar()->SetFocusAndSelection();
+  EXPECT_TRUE(FindBarHasFocus(browser()));
+
+  // Simulate key press for character in page contents.
+  SimulateKeyPress(browser()->window()->GetNativeWindow(), ui::VKEY_A);
+  WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents();
+  EXPECT_GT(GetFindInContentsMatchCount(contents, ASCIIToUTF16("A")), 0);
+
+  // Buttons should be enabled because we had results.
+  NSButton* nextButton = GetFindBarControllerNextButton(browser());
+  NSButton* previousButton = GetFindBarControllerPreviousButton(browser());
+  EXPECT_TRUE([nextButton isEnabled]);
+  EXPECT_TRUE([previousButton isEnabled]);
+
+  // Move focus to the location bar then back to the find bar.
+  browser()->window()->GetLocationBar()->FocusLocation(true);
+  EXPECT_FALSE(FindBarHasFocus(browser()));
+  browser()->GetFindBarController()->find_bar()->SetFocusAndSelection();
+  EXPECT_TRUE(FindBarHasFocus(browser()));
+
+  // Buttons should remain enabled.
+  EXPECT_TRUE([nextButton isEnabled]);
+  EXPECT_TRUE([previousButton isEnabled]);
+}
+
+IN_PROC_BROWSER_TEST_F(FindBarBrowserTest,
+                       NextPreviousButtonsEnabledAfterCloseAndTabSwitch) {
+  AddTabAtIndex(1, GURL("about:blank"), ui::PAGE_TRANSITION_LINK);
+  ui_test_utils::NavigateToURL(browser(), GetTestURL(kSimplePage));
+  browser()->GetFindBarController()->Show();
+  browser()->GetFindBarController()->find_bar()->SetFocusAndSelection();
+  EXPECT_TRUE(FindBarHasFocus(browser()));
+
+  // Simulate key press for character in page contents.
+  SimulateKeyPress(browser()->window()->GetNativeWindow(), ui::VKEY_A);
+  WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents();
+  EXPECT_GT(GetFindInContentsMatchCount(contents, ASCIIToUTF16("A")), 0);
+
+  // Close find bar, switch to another tab then switch back and reopen find bar.
+  FindBarCocoaController* controller = GetFindBarCocoaController(browser());
+  [controller close:nil];
+  browser()->tab_strip_model()->ActivateTabAt(1, true);
+  browser()->tab_strip_model()->ActivateTabAt(0, true);
+  browser()->GetFindBarController()->Show();
+  browser()->GetFindBarController()->find_bar()->SetFocusAndSelection();
+
+  NSButton* nextButton = GetFindBarControllerNextButton(browser());
+  NSButton* previousButton = GetFindBarControllerPreviousButton(browser());
+  EXPECT_TRUE([nextButton isEnabled]);
+  EXPECT_TRUE([previousButton isEnabled]);
+
+  // Switch to another tab again and reopen find bar.
+  [controller close:nil];
+  browser()->tab_strip_model()->ActivateTabAt(1, true);
+  browser()->GetFindBarController()->Show();
+  browser()->GetFindBarController()->find_bar()->SetFocusAndSelection();
+  EXPECT_TRUE([nextButton isEnabled]);
+  EXPECT_TRUE([previousButton isEnabled]);
+}
+
 IN_PROC_BROWSER_TEST_F(FindBarBrowserTest, EscapeKey) {
   // Enter fullscreen.
   std::unique_ptr<FullscreenNotificationObserver> waiter(
@@ -73,11 +221,7 @@ IN_PROC_BROWSER_TEST_F(FindBarBrowserTest, EscapeKey) {
   EXPECT_TRUE(FindBarHasFocus(browser()));
 
   // Simulate a key press with the ESC key.
-  base::RunLoop run_loop;
-  ui_controls::EnableUIControls();
-  ui_controls::SendKeyPressNotifyWhenDone(window, ui::VKEY_ESCAPE, false, false,
-                                          false, false, run_loop.QuitClosure());
-  run_loop.Run();
+  SimulateKeyPress(window, ui::VKEY_ESCAPE);
 
   // Check that the browser is still in fullscreen and that the find bar has
   // lost focus.

chrome/browser/ui/cocoa/find_bar/find_bar_cocoa_controller.mm

@@ -41,6 +41,7 @@ const float kFindBarCloseDuration = 0.15;
 const float kFindBarMoveDuration = 0.15;
 const float kRightEdgeOffset = 25;
 const int kMaxCharacters = 4000;
+const int kUndefinedResultCount = -1;
 
 @interface FindBarCocoaController (PrivateMethods) <NSAnimationDelegate>
 // Returns the appropriate frame for a hidden find bar.
@@ -75,6 +76,10 @@ const int kMaxCharacters = 4000;
 - (void)clearFindResultsForCurrentBrowser;
 
 - (BrowserWindowController*)browserWindowController;
+
+// Returns the number of matches from the last find results of the active
+// web contents. Returns kUndefinedResultCount if unable to determine the count.
+- (int)lastNumberOfMatchesForActiveWebContents;
 @end
 
 @implementation FindBarCocoaController
@@ -362,9 +367,8 @@ const int kMaxCharacters = 4000;
 
 - (void)setFocusAndSelection {
   [[findText_ window] makeFirstResponder:findText_];
+  BOOL buttonsEnabled = ([self lastNumberOfMatchesForActiveWebContents] != 0);
 
-  // Enable the buttons if the find text is non-empty.
-  BOOL buttonsEnabled = ([[findText_ stringValue] length] > 0) ? YES : NO;
   [previousButton_ setEnabled:buttonsEnabled];
   [nextButton_ setEnabled:buttonsEnabled];
 }
@@ -668,4 +672,13 @@ const int kMaxCharacters = 4000;
       browserWindowControllerForWindow:browser_->window()->GetNativeWindow()];
 }
 
+- (int)lastNumberOfMatchesForActiveWebContents {
+  if (!browser_)
+    return kUndefinedResultCount;
+
+  content::WebContents* contents =
+      findBarBridge_->GetFindBarController()->web_contents();
+  FindTabHelper* findTabHelper = FindTabHelper::FromWebContents(contents);
+  return findTabHelper->find_result().number_of_matches();
+}
 @end