Refactor omnibox focus API into a single method.

chrome/browser/ui/search/instant_controller.cc

 // Copyright 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.
 
 #include "chrome/browser/ui/search/instant_controller.h"
 
 #include "base/metrics/histogram.h"
 #include "base/string_util.h"
 #include "base/stringprintf.h"
 #include "base/utf_string_conversions.h"
@@ skipping 1168 matching lines @@
     ShowOverlay(100, INSTANT_SIZE_PERCENT);
 }
 
 void InstantController::ShowInstantOverlay(const content::WebContents* contents,
                                            int height,
                                            InstantSizeUnits units) {
   if (extended_enabled_ && IsContentsFrom(overlay(), contents))
     ShowOverlay(height, units);
 }
 
-void InstantController::FocusOmnibox(const content::WebContents* contents) {
-  if (!extended_enabled_)
+void InstantController::FocusOmnibox(const content::WebContents* contents,
+                                     OmniboxFocusState state) {
+  if (!extended_enabled_ ||
+      !instant_tab_ ||
+      !IsContentsFrom(instant_tab(), contents))
     return;
 
-  DCHECK(IsContentsFrom(instant_tab(), contents));
-  browser_->FocusOmnibox(true);
-}
-
-void InstantController::StartCapturingKeyStrokes(
-    const content::WebContents* contents) {
-  if (!extended_enabled_)
-    return;
-
-  DCHECK(IsContentsFrom(instant_tab(), contents));
-  browser_->FocusOmnibox(false);
-}
-
-void InstantController::StopCapturingKeyStrokes(
-    content::WebContents* contents) {
-  // Nothing to do if omnibox doesn't have invisible focus.
-  if (!extended_enabled_ || omnibox_focus_state_ != OMNIBOX_FOCUS_INVISIBLE)
-    return;
-
-  DCHECK(IsContentsFrom(instant_tab(), contents));
-  contents->GetView()->Focus();
+  switch (state) {
+    case OMNIBOX_FOCUS_VISIBLE:
+      if (instant_tab_->ShouldProcessFocusOmnibox())

[sreeram, 2013/04/01 16:04:38]

These Should*() checks are not needed here. So, this should just read something
like this:

switch (state) {
  case OMNIBOX_FOCUS_VISIBLE:
    browser_->FocusOmnibox(true);
    break;
  case OMNIBOX_FOCUS_INVISIBLE:
    browser_->FocusOmnibox(false);
    break;
  case OMNIBOX_FOCUS_NONE:
    if (omnibox_focus_state_ == OMNIBOX_FOCUS_INVISIBLE)
      contents->GetView()->Focus();
    break;
}

Leave out the "default:" clause. The idea is that, if the switch doesn't handle
all the values of the enum, the compiler will complain (which is better than a
run-time failure in NOTREACHED).

[kmadhusu, 2013/04/02 02:06:50]

Done.

+        browser_->FocusOmnibox(true);
+      break;
+    case OMNIBOX_FOCUS_INVISIBLE:
+      if (instant_tab_->ShouldProcessStartCapturingKeyStrokes())
+        browser_->FocusOmnibox(false);
+      break;
+    case OMNIBOX_FOCUS_NONE:
+      if (instant_tab_->ShouldProcessStopCapturingKeyStrokes() &&
+          omnibox_focus_state_ != OMNIBOX_FOCUS_INVISIBLE)
+        contents->GetView()->Focus();
+      break;
+    default:
+      NOTREACHED();
+  }
 }
 
 void InstantController::NavigateToURL(const content::WebContents* contents,
                                       const GURL& url,
                                       content::PageTransition transition,
                                       WindowOpenDisposition disposition) {
   LOG_INSTANT_DEBUG_EVENT(this, base::StringPrintf(
       "NavigateToURL: url='%s'", url.spec().c_str()));
 
   // TODO(samarth): handle case where contents are no longer "active" (e.g. user
@@ skipping 399 matching lines @@
       // for instance, if the user types 'i' and the suggestion is 'INSTANT',
       // suggest 'nstant'. Otherwise, the user text really isn't a prefix, so
       // suggest nothing.
       // TODO(samarth|jered): revisit this logic. http://crbug.com/196572.
       return true;
     }
   }
 
   return false;
 }