InstantExtended: Committed NTP

chrome/browser/ui/browser_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/browser_instant_controller.h"
 
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/themes/theme_service.h"
 #include "chrome/browser/themes/theme_service_factory.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/omnibox/location_bar.h"
 #include "chrome/browser/ui/omnibox/omnibox_view.h"
 #include "chrome/browser/ui/search/search.h"
 #include "chrome/browser/ui/search/search_tab_helper.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/webui/ntp/app_launcher_handler.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/pref_names.h"
+#include "chrome/common/url_constants.h"
 #include "content/public/browser/notification_service.h"
 #include "grit/theme_resources.h"
 #include "ui/gfx/color_utils.h"
 #include "ui/gfx/sys_color_change_listener.h"
 
 namespace {
 const char* GetInstantPrefName(Profile* profile) {
   return chrome::search::IsInstantExtendedAPIEnabled(profile) ?
       prefs::kInstantExtendedEnabled : prefs::kInstantEnabled;
 }
 }
 
 namespace chrome {
 
 ////////////////////////////////////////////////////////////////////////////////
 // BrowserInstantController, public:
 
 BrowserInstantController::BrowserInstantController(Browser* browser)
     : browser_(browser),
       instant_(ALLOW_THIS_IN_INITIALIZER_LIST(this),
                chrome::search::IsInstantExtendedAPIEnabled(browser->profile())),

[sreeram, 2013/02/07 21:17:50]

Replace browser_->profile() with just profile(), throughout this file.

[samarth, 2013/02/07 22:11:55]

Done.

       instant_unload_handler_(browser),
       initialized_theme_info_(false),
       theme_area_height_(0) {
   profile_pref_registrar_.Init(browser_->profile()->GetPrefs());
   profile_pref_registrar_.Add(
       GetInstantPrefName(browser_->profile()),
       base::Bind(&BrowserInstantController::ResetInstant,
                  base::Unretained(this)));
   profile_pref_registrar_.Add(
       prefs::kSearchSuggestEnabled,
@@ skipping 38 matching lines @@
     case search::INSTANT_FORCE_OFF:
       instant_extended_value = false;
       break;
   }
 
   prefs->RegisterBooleanPref(prefs::kInstantExtendedEnabled,
                              instant_extended_value,
                              PrefServiceSyncable::SYNCABLE_PREF);
 }
 
+bool BrowserInstantController::MaybeSwapInInstantNTPContents(
+    const GURL& url,
+    content::WebContents* source_contents,
+    content::WebContents** target_contents) {
+  if (url != GURL(chrome::kChromeUINewTabURL))
+    return false;
+
+  scoped_ptr<content::WebContents> instant_ntp = instant_.ReleaseNTPContents();
+  if (!instant_ntp)
+    return false;
+
+  *target_contents = instant_ntp.get();
+  instant_ntp->GetController().PruneAllButActive();
+  if (source_contents) {
+    instant_ntp->GetController().CopyStateFromAndPrune(
+        &source_contents->GetController());
+    ReplaceWebContentsAt(
+        browser_->tab_strip_model()->GetIndexOfWebContents(source_contents),
+        instant_ntp.Pass());
+  } else {
+    // If |source_contents| is NULL, then the caller is responsible for
+    // inserting instant_ntp into the tabstrip and will take ownership.
+    ignore_result(instant_ntp.release());
+  }
+  return true;
+}
+
 bool BrowserInstantController::OpenInstant(WindowOpenDisposition disposition) {
   // Unsupported dispositions.
   if (disposition == NEW_BACKGROUND_TAB || disposition == NEW_WINDOW)
     return false;
 
   // The omnibox currently doesn't use other dispositions, so we don't attempt
   // to handle them. If you hit this DCHECK file a bug and I'll (sky) add
   // support for the new disposition.
   DCHECK(disposition == CURRENT_TAB ||
          disposition == NEW_FOREGROUND_TAB) << disposition;
 
   return instant_.CommitIfPossible(disposition == CURRENT_TAB ?
       INSTANT_COMMIT_PRESSED_ENTER : INSTANT_COMMIT_PRESSED_ALT_ENTER);
 }
 
-void BrowserInstantController::CommitInstant(content::WebContents* preview,
-                                             bool in_new_tab) {
+Profile* BrowserInstantController::profile() const {
+  return browser_->profile();
+}
+
+void BrowserInstantController::CommitInstant(
+    scoped_ptr<content::WebContents> preview,
+    bool in_new_tab) {
   if (in_new_tab) {
     // TabStripModel takes ownership of |preview|.
-    browser_->tab_strip_model()->AddWebContents(preview, -1,
+    browser_->tab_strip_model()->AddWebContents(preview.release(), -1,
         instant_.last_transition_type(), TabStripModel::ADD_ACTIVE);
   } else {
     int index = browser_->tab_strip_model()->active_index();
-    DCHECK_NE(TabStripModel::kNoTab, index);
-    content::WebContents* active_tab =
-        browser_->tab_strip_model()->GetWebContentsAt(index);
-    // TabStripModel takes ownership of |preview|.
-    browser_->tab_strip_model()->ReplaceWebContentsAt(index, preview);
-    // InstantUnloadHandler takes ownership of |active_tab|.
-    instant_unload_handler_.RunUnloadListenersOrDestroy(active_tab, index);
-
-    GURL url = preview->GetURL();
+    const GURL& url = preview->GetURL();

[sreeram, 2013/02/07 21:17:50]

GURL url = preview->GetURL();

Because, given preview.Pass() on the next line, it isn't obvious that
preview->GetURL() will continue to be valid. We know it's valid for now, but
better to be safe about these things (maybe the tab strip model will do
something to invalidate |preview| in the future). For example, see
https://code.google.com/p/chromium/issues/detail?id=158056#c4

[samarth, 2013/02/07 22:11:55]

Done.

+    ReplaceWebContentsAt(index, preview.Pass());

[sreeram, 2013/02/07 21:17:50]

Nit: ReplaceWebContentsAt(browser_->tab_strip_model()->active_index(),
preview.Pass())

To match the style of line 118.

[samarth, 2013/02/07 22:11:55]

Done.

     DCHECK(browser_->profile()->GetExtensionService());
     if (browser_->profile()->GetExtensionService()->IsInstalledApp(url)) {
       AppLauncherHandler::RecordAppLaunchType(
           extension_misc::APP_LAUNCH_OMNIBOX_INSTANT);
     }

[sreeram, 2013/02/07 21:17:50]

Move these lines (159-163) to outside the "if" statement, since it feels like it
(recording metrics about whether an Instant-committed URL is an app) should be
done for both in_new_tab vs not.

[samarth, 2013/02/07 22:11:55]

Moved it to the top (I assume that is OK?). That way, I don't have to worry
about saving GetURL to a temporary in the first place.

[sreeram, 2013/02/07 22:18:15]

Yup, moving to the top is totally fine.

   }
 }
 
+void BrowserInstantController::ReplaceWebContentsAt(
+    int index,
+    scoped_ptr<content::WebContents> new_contents) {
+  DCHECK_NE(TabStripModel::kNoTab, index);
+  content::WebContents* old_contents =
+      browser_->tab_strip_model()->GetWebContentsAt(index);
+  // TabStripModel takes ownership of |new_contents|.
+  browser_->tab_strip_model()->ReplaceWebContentsAt(
+      index, new_contents.release());
+  // TODO(samarth): use scoped_ptr instead of comments to document ownership
+  // transfer.
+  // InstantUnloadHandler takes ownership of |old_contents|.
+  instant_unload_handler_.RunUnloadListenersOrDestroy(old_contents, index);
+}
+
 void BrowserInstantController::SetInstantSuggestion(
     const InstantSuggestion& suggestion) {
   browser_->window()->GetLocationBar()->SetInstantSuggestion(suggestion);
 }
 
 gfx::Rect BrowserInstantController::GetInstantBounds() {
   return browser_->window()->GetInstantBounds();
 }
 
 void BrowserInstantController::InstantPreviewFocused() {
@@ skipping 171 matching lines @@
   // part of the image overlay should draw, 'cos the origin is top-left.
   if (!browser_->search_model()->mode().is_ntp() ||
       theme_info_.theme_id.empty() ||
       theme_info_.image_vertical_alignment == THEME_BKGRND_IMAGE_ALIGN_TOP) {
     return;
   }
   instant_.ThemeAreaHeightChanged(theme_area_height_);
 }
 
 }  // namespace chrome