Refactor omnibox focus API into a single method.

chrome/browser/ui/search/instant_page.cc

Index: chrome/browser/ui/search/instant_page.cc
diff --git a/chrome/browser/ui/search/instant_page.cc b/chrome/browser/ui/search/instant_page.cc
index 764a5658b1865df35f4eaadc972a3840a68b2cfd..a94b2444e4399587c552566930186c303a6dd3bc 100644
--- a/chrome/browser/ui/search/instant_page.cc
+++ b/chrome/browser/ui/search/instant_page.cc
@@ -161,10 +161,6 @@ bool InstantPage::OnMessageReceived(const IPC::Message& message) {
     IPC_MESSAGE_HANDLER(ChromeViewHostMsg_ShowInstantOverlay,
                         OnShowInstantOverlay)
     IPC_MESSAGE_HANDLER(ChromeViewHostMsg_FocusOmnibox, OnFocusOmnibox)
-    IPC_MESSAGE_HANDLER(ChromeViewHostMsg_StartCapturingKeyStrokes,
-                        OnStartCapturingKeyStrokes);
-    IPC_MESSAGE_HANDLER(ChromeViewHostMsg_StopCapturingKeyStrokes,
-                        OnStopCapturingKeyStrokes);
     IPC_MESSAGE_HANDLER(ChromeViewHostMsg_SearchBoxNavigate,
                         OnSearchBoxNavigate);
     IPC_MESSAGE_HANDLER(ChromeViewHostMsg_SearchBoxDeleteMostVisitedItem,
@@ -228,27 +224,25 @@ void InstantPage::OnShowInstantOverlay(int page_id,
   }
 }
 
-void InstantPage::OnFocusOmnibox(int page_id) {
+void InstantPage::OnFocusOmnibox(int page_id, OmniboxFocusState state) {
   if (contents()->IsActiveEntry(page_id)) {
     OnInstantSupportDetermined(page_id, true);
-    if (ShouldProcessFocusOmnibox())
-      delegate_->FocusOmnibox(contents());
-  }
-}
-
-void InstantPage::OnStartCapturingKeyStrokes(int page_id) {
-  if (contents()->IsActiveEntry(page_id)) {
-    OnInstantSupportDetermined(page_id, true);
-    if (ShouldProcessStartCapturingKeyStrokes())
-      delegate_->StartCapturingKeyStrokes(contents());
-  }
-}
-
-void InstantPage::OnStopCapturingKeyStrokes(int page_id) {
-  if (contents()->IsActiveEntry(page_id)) {
-    OnInstantSupportDetermined(page_id, true);
-    if (ShouldProcessStopCapturingKeyStrokes())
-      delegate_->StopCapturingKeyStrokes(contents());
+    switch (state) {
+      case OMNIBOX_FOCUS_VISIBLE:
+        if (ShouldProcessFocusOmnibox())
+          delegate_->FocusOmnibox(contents());
+        break;
+      case OMNIBOX_FOCUS_INVISIBLE:
+        if (ShouldProcessStartCapturingKeyStrokes())
+          delegate_->StartCapturingKeyStrokes(contents());
+        break;
+      case OMNIBOX_FOCUS_NONE:
+        if (ShouldProcessStopCapturingKeyStrokes())
+          delegate_->StopCapturingKeyStrokes(contents());

[kmadhusu, 2013/03/26 18:56:17]

From my understanding of the bug report, we want to get rid of
StartCapturingKeyStrokes IPC and StopCapturingKeyStrokes IPC.

I would like to keep InstantPage::Delegate::StartCapturingKeyStrokes() and
InstantPage::Delegate::StopCapturingKeyStorkes() for better understanding of the
code. WDYT?

[sreeram, 2013/03/26 19:01:24]

Let's not. InstantPage should be a simple message relay, and complicated logic
doesn't belong here. So, do these in InstantController instead.

[kmadhusu, 2013/03/26 21:04:00]

Done. Moved this logic to InstantController.

Can you tell me the use case when ShouldProcessStopCapturingKeyStrokes() or
ShouldProcessStartCapturingKeyStrokes() returns false? Is it okay to do these
checks in InstantController?

+        break;
+      default:
+        NOTREACHED();
+    }
   }
 }