Cleanup in InstantTab and InstantController

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 <stddef.h>
 #include <utility>
 
 #include "base/location.h"
 #include "base/strings/stringprintf.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/search/instant_service.h"
 #include "chrome/browser/search/instant_service_factory.h"
 #include "chrome/browser/search/search.h"
 #include "chrome/browser/ui/browser_instant_controller.h"
 #include "chrome/browser/ui/search/instant_tab.h"
 #include "chrome/browser/ui/search/search_tab_helper.h"
 #include "components/sessions/core/serialized_navigation_entry.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/web_contents.h"
 #include "url/gurl.h"
 
-// Macro used for logging debug events. |message| should be a std::string.
-#define LOG_INSTANT_DEBUG_EVENT(controller, message) \
-    controller->LogDebugEvent(message)
-
 namespace {
 
 bool IsContentsFrom(const InstantTab* page,
                     const content::WebContents* contents) {
   return page && (page->web_contents() == contents);
 }
 
 // Adds a transient NavigationEntry to the supplied |contents|'s
 // NavigationController if the page's URL has not already been updated with the
 // supplied |search_terms|. Sets the |search_terms| on the transient entry for
@@ skipping 23 matching lines @@
 
 InstantController::InstantController(BrowserInstantController* browser)
     : browser_(browser) {
 }
 
 InstantController::~InstantController() {
 }
 
 bool InstantController::SubmitQuery(const base::string16& search_terms,
                                     const EmbeddedSearchRequestParams& params) {
-  if (instant_tab_ && instant_tab_->supports_instant() &&
-      search_mode_.is_origin_search()) {
-    // Use |instant_tab_| to run the query if we're already on a search results
-    // page. (NOTE: in particular, we do not send the query to NTPs.)
-    SearchTabHelper::FromWebContents(instant_tab_->web_contents())->Submit(
-        search_terms, params);
-    instant_tab_->web_contents()->Focus();
-    EnsureSearchTermsAreSet(instant_tab_->web_contents(), search_terms);
-    return true;
-  }
-  return false;
+  if (!instant_tab_)
+    return false;
+
+  SearchTabHelper* search_tab =
+      SearchTabHelper::FromWebContents(instant_tab_->web_contents());

[sfiera, 2016/07/22 12:51:00]

By getting search_tab directly here, you've dropped a check that
instant_tab_->web_contents() != nullptr.

[Marc Treib, 2016/07/22 13:11:17]

Good catch! Added the check back.

+  if (!search_tab->SupportsInstant() || !search_mode_.is_origin_search())
+    return false;
+
+  // Use |instant_tab_| to run the query if we're already on a search results
+  // page. (NOTE: in particular, we do not send the query to NTPs.)
+  search_tab->Submit(search_terms, params);
+  instant_tab_->web_contents()->Focus();
+  EnsureSearchTermsAreSet(instant_tab_->web_contents(), search_terms);
+  return true;
 }
 
 void InstantController::SearchModeChanged(const SearchMode& old_mode,
                                           const SearchMode& new_mode) {
-  LOG_INSTANT_DEBUG_EVENT(this, base::StringPrintf(
+  LogDebugEvent(base::StringPrintf(
       "SearchModeChanged: [origin:mode] %d:%d to %d:%d", old_mode.origin,
       old_mode.mode, new_mode.origin, new_mode.mode));
 
   search_mode_ = new_mode;
   ResetInstantTab();
 }
 
 void InstantController::ActiveTabChanged() {
-  LOG_INSTANT_DEBUG_EVENT(this, "ActiveTabChanged");
+  LogDebugEvent("ActiveTabChanged");
   ResetInstantTab();
 }
 
 void InstantController::LogDebugEvent(const std::string& info) const {
   DVLOG(1) << info;
 
   debug_events_.push_front(std::make_pair(
       base::Time::Now().ToInternalValue(), info));
   static const size_t kMaxDebugEventSize = 2000;
   if (debug_events_.size() > kMaxDebugEventSize)
     debug_events_.pop_back();
 }
 
 void InstantController::ClearDebugEvents() {
   debug_events_.clear();
 }
 
-Profile* InstantController::profile() const {
-  return browser_->profile();
-}
-
-InstantTab* InstantController::instant_tab() const {
-  return instant_tab_.get();
-}
-
 void InstantController::InstantSupportChanged(
     InstantSupportState instant_support) {
   // Handle INSTANT_SUPPORT_YES here because InstantTab is not hooked up to the
   // active tab. Search model changed listener in InstantTab will handle other
   // cases.
   if (instant_support != INSTANT_SUPPORT_YES)
     return;
 
   ResetInstantTab();
 }
 
 void InstantController::InstantSupportDetermined(
     const content::WebContents* contents,
     bool supports_instant) {
-  DCHECK(IsContentsFrom(instant_tab(), contents));
+  DCHECK(IsContentsFrom(instant_tab_.get(), contents));
 
   if (!supports_instant)
     base::ThreadTaskRunnerHandle::Get()->DeleteSoon(FROM_HERE,
                                                     instant_tab_.release());
 
   content::NotificationService::current()->Notify(
       chrome::NOTIFICATION_INSTANT_TAB_SUPPORT_DETERMINED,
       content::Source<InstantController>(this),
       content::NotificationService::NoDetails());
 }
 
 void InstantController::InstantTabAboutToNavigateMainFrame(
     const content::WebContents* contents,
     const GURL& url) {
-  DCHECK(IsContentsFrom(instant_tab(), contents));
+  DCHECK(IsContentsFrom(instant_tab_.get(), contents));
 
   // The Instant tab navigated.  Send it the data it needs to display
   // properly.
   UpdateInfoForInstantTab();
 }
 
 void InstantController::ResetInstantTab() {
   if (!search_mode_.is_origin_default()) {
     content::WebContents* active_tab = browser_->GetActiveWebContents();
     if (!instant_tab_ || active_tab != instant_tab_->web_contents()) {
-      instant_tab_.reset(new InstantTab(this));
-      instant_tab_->Init(active_tab);
+      instant_tab_.reset(new InstantTab(this, active_tab));
       UpdateInfoForInstantTab();
     }
   } else {
     instant_tab_.reset();
   }
 }
 
 void InstantController::UpdateInfoForInstantTab() {
   if (instant_tab_) {
     // Update theme details.
     InstantService* instant_service = GetInstantService();
     if (instant_service) {
       instant_service->UpdateThemeInfo();
       instant_service->UpdateMostVisitedItemsInfo();
     }
   }
 }
 
 InstantService* InstantController::GetInstantService() const {
-  return InstantServiceFactory::GetForProfile(profile());
+  return InstantServiceFactory::GetForProfile(browser_->profile());
 }