Keep focus on Find-In-Page buttons when using the keyboard to navigate.

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

 // 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.
 
 #import <Cocoa/Cocoa.h>
 
 #include "base/auto_reset.h"
 #include "base/mac/bundle_locations.h"
 #include "base/strings/sys_string_conversions.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/cocoa/browser_window_controller.h"
 #import "chrome/browser/ui/cocoa/find_bar/find_bar_bridge.h"
 #import "chrome/browser/ui/cocoa/find_bar/find_bar_cocoa_controller.h"
 #import "chrome/browser/ui/cocoa/find_bar/find_bar_text_field.h"
 #import "chrome/browser/ui/cocoa/find_bar/find_bar_text_field_cell.h"
 #import "chrome/browser/ui/cocoa/image_button_cell.h"
 #import "chrome/browser/ui/cocoa/tabs/tab_strip_controller.h"
+#import "chrome/browser/ui/cocoa/view_id_util.h"
 #include "chrome/browser/ui/find_bar/find_bar_controller.h"
 #include "chrome/browser/ui/find_bar/find_tab_helper.h"
 #include "chrome/grit/generated_resources.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/render_widget_host.h"
 #include "content/public/browser/web_contents.h"
 #include "grit/components_strings.h"
 #include "grit/theme_resources.h"
 #import "third_party/google_toolbox_for_mac/src/AppKit/GTMNSAnimation+Duration.h"
 #import "ui/base/cocoa/find_pasteboard.h"
@@ skipping 61 matching lines @@
   }
   return self;
 }
 
 - (void)dealloc {
   [self browserWillBeDestroyed];
   [super dealloc];
 }
 
 - (void)browserWillBeDestroyed {
+  view_id_util::UnsetID(previousButton_);
+  view_id_util::UnsetID(nextButton_);
+  view_id_util::UnsetID(closeButton_);

[tapted, 2016/02/10 00:56:01]

I don't think this will fix the problem.

I think the issue is that by the time we get here, Cocoa has unloaded the view
controller and its owned views, then set the IBOutlets to nil. Cocoa doesn't
have a reliable signal when its views go away since they're reference counted
(The 10.10SDK has -[NSViewController viewWillDisappear] but that's not available
on all the OSes we support and might not be what we need in any case).

My recommendation is to just add these view IDs to the big 'ol `if` condition in
view_id_util_browsetest.mm:


      // Mac implementation does not support following ids yet.
      // TODO(palmer): crbug.com/536257: Enable VIEW_ID_LOCATION_ICON.
      if (i == VIEW_ID_STAR_BUTTON ||
          i == VIEW_ID_CONTENTS_SPLIT ||
          i == VIEW_ID_BROWSER_ACTION ||
          i == VIEW_ID_FEEDBACK_BUTTON ||
          i == VIEW_ID_SCRIPT_BUBBLE ||
          i == VIEW_ID_SAVE_CREDIT_CARD_BUTTON ||
          i == VIEW_ID_TRANSLATE_BUTTON ||
          i == VIEW_ID_LOCATION_ICON) {
        continue;
      }

Since find_bar_views_interactive_uitest.cc won't be testing the Cocoa stuff
anyway, so they're not really needed execpt ot make the ViewIDTest.Basic test
pass as far as I can tell.

[Mario Pistrich, 2016/02/10 01:14:29]

Thanks for clarifying, I've added them to the if so that they are skipped.

+
   // All animations should have been explicitly stopped before a tab is closed.
   DCHECK(!showHideAnimation_.get());
   DCHECK(!moveAnimation_.get());
   [[NSNotificationCenter defaultCenter] removeObserver:self];
   browser_ = nullptr;
 }
 
 - (void)setFindBarBridge:(FindBarBridge*)findBarBridge {
   DCHECK(!findBarBridge_);  // should only be called once.
   findBarBridge_ = findBarBridge;
@@ skipping 18 matching lines @@
 
   [closeButton_ setTitle:l10n_util::GetNSString(IDS_ACCNAME_CLOSE)];
   [previousButton_ setTitle:l10n_util::GetNSString(IDS_ACCNAME_PREVIOUS)];
   [nextButton_ setTitle:l10n_util::GetNSString(IDS_ACCNAME_NEXT)];
 
   [findBarView_ setFrame:[self hiddenFindBarFrame]];
   defaultWidth_ = NSWidth([findBarView_ frame]);
   [[self view] setHidden:YES];
 
   [self prepopulateText:[[FindPasteboard sharedInstance] findText]];
+
+  // Set ViewIDs for the buttons which don't have their dedicated class.
+  // ViewID of |findText_| is handled by itself.
+  view_id_util::SetID(previousButton_, VIEW_ID_FIND_IN_PAGE_PREVIOUS_BUTTON);
+  view_id_util::SetID(nextButton_, VIEW_ID_FIND_IN_PAGE_NEXT_BUTTON);
+  view_id_util::SetID(closeButton_, VIEW_ID_FIND_IN_PAGE_CLOSE_BUTTON);
 }
 
 - (IBAction)close:(id)sender {
   if (findBarBridge_) {
     findBarBridge_->GetFindBarController()->EndFindSession(
         FindBarController::kKeepSelectionOnPage,
         FindBarController::kKeepResultsInFindBox);
   }
 }
 
@@ skipping 490 matching lines @@
 }
 
 - (BrowserWindowController*)browserWindowController {
   if (!browser_)
     return nil;
   return [BrowserWindowController
       browserWindowControllerForWindow:browser_->window()->GetNativeWindow()];
 }
 
 @end