Fixed focus requesting for the findbar.

chrome/browser/ui/views/find_bar_view.cc

 // Copyright (c) 2010 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 "chrome/browser/views/find_bar_view.h"
 
 #include <algorithm>
 
 #include "app/l10n_util.h"
 #include "app/resource_bundle.h"
 #include "base/string_number_conversions.h"
 #include "base/string_util.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/find_bar_controller.h"
 #include "chrome/browser/find_bar_state.h"
 #include "chrome/browser/profile.h"
 #include "chrome/browser/tab_contents/tab_contents.h"
 #include "chrome/browser/themes/browser_theme_provider.h"
 #include "chrome/browser/view_ids.h"
 #include "chrome/browser/views/find_bar_host.h"
 #include "chrome/browser/views/frame/browser_view.h"
 #include "gfx/canvas.h"
 #include "grit/generated_resources.h"
 #include "grit/theme_resources.h"
 #include "third_party/skia/include/effects/SkGradientShader.h"
 #include "views/background.h"
 #include "views/controls/button/image_button.h"
 #include "views/controls/label.h"
+#include "views/focus/focus_manager.h"
 #include "views/widget/widget.h"
 
 // The amount of whitespace to have before the find button.
 static const int kWhiteSpaceAfterMatchCountLabel = 1;
 
 // The margins around the search field and the close button.
 static const int kMarginLeftOfCloseButton = 3;
 static const int kMarginRightOfCloseButton = 7;
 static const int kMarginLeftOfFindTextfield = 12;
 
@@ skipping 177 matching lines @@
   }
 
   // The match_count label may have increased/decreased in size so we need to
   // do a layout and repaint the dialog so that the find text field doesn't
   // partially overlap the match-count label when it increases on no matches.
   Layout();
   SchedulePaint();
 }
 
 void FindBarView::SetFocusAndSelection(bool select_all) {
+  DCHECK(find_text_);

[Finnur, 2010/11/15 14:49:12]

This is redundant and just adds cruft to debug builds. The code is going to
crash on the next line anyway, you don't need a DCHECK to tell you that.

[altimofeev, 2010/11/15 15:36:19]

Done.

   find_text_->RequestFocus();
+  // Storing is needed here because view that has focus before wrench menu
+  // activation will be activated just after wrench menu is closed. FindBar
+  // has it's own focus tracker, so it will focus correct view on close.

[Finnur, 2010/11/15 14:49:12]

nit: I count four instances of the word 'the' being missing here (view, wrench
menu, wrench menu, view).

[altimofeev, 2010/11/15 15:36:19]

Thanks, will try to improve my grammar.

+  DCHECK(find_text_->GetFocusManager());

[Finnur, 2010/11/15 14:49:12]

Same here (don't need the DCHECK)

[altimofeev, 2010/11/15 15:36:19]

Done.

+  find_text_->GetFocusManager()->StoreFocusedView();

[Finnur, 2010/11/15 14:49:12]

Make sure the following use case works (I think you haven't broken it, but just
to make sure):

1) With two tabs open, open find window (ctrl-F or wrench)
2) Set focus to the omnibox
3) switch to the other tab
4) switch back.

Focus should not be restored to the Find box. It should be restored to where
focus was when you left the tab.

[altimofeev, 2010/11/15 15:36:19]

I have tried your use case. The focus behaves as it should.

   if (select_all && !find_text_->text().empty())
     find_text_->SelectAll();
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 // FindBarView, views::View overrides:
 
 void FindBarView::Paint(gfx::Canvas* canvas) {
   SkPaint paint;
 
@@ skipping 281 matching lines @@
 
 void FindBarView::OnThemeChanged() {
   ResourceBundle& rb = ResourceBundle::GetSharedInstance();
   if (GetThemeProvider()) {
     close_button_->SetBackground(
         GetThemeProvider()->GetColor(BrowserThemeProvider::COLOR_TAB_TEXT),
         rb.GetBitmapNamed(IDR_CLOSE_BAR),
         rb.GetBitmapNamed(IDR_CLOSE_BAR_MASK));
   }
 }