Refactor omnibox focus API into a single method.

chrome/browser/ui/search/instant_controller.cc

Index: chrome/browser/ui/search/instant_controller.cc
diff --git a/chrome/browser/ui/search/instant_controller.cc b/chrome/browser/ui/search/instant_controller.cc
index 43b5951d17e353d5441acf1ddff69ec71710b30c..1e671c7b8126e655615356896b143910e543db10 100644
--- a/chrome/browser/ui/search/instant_controller.cc
+++ b/chrome/browser/ui/search/instant_controller.cc
@@ -1186,31 +1186,30 @@ void InstantController::ShowInstantOverlay(const content::WebContents* contents,
     ShowOverlay(height, units);
 }
 
-void InstantController::FocusOmnibox(const content::WebContents* contents) {
-  if (!extended_enabled_)
-    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)
+void InstantController::FocusOmnibox(const content::WebContents* contents,
+                                     OmniboxFocusState state) {
+  if (!extended_enabled_ ||
+      !instant_tab_ ||
+      !IsContentsFrom(instant_tab(), contents))
     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,