Instant: Use only one hidden WebContents per profile.

chrome/browser/instant/instant_web_contents_container.h

+// Copyright 2013 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.
+
+#ifndef CHROME_BROWSER_INSTANT_INSTANT_WEB_CONTENTS_CONTAINER_H_
+#define CHROME_BROWSER_INSTANT_INSTANT_WEB_CONTENTS_CONTAINER_H_
+
+#include "base/basictypes.h"
+#include "base/compiler_specific.h"
+#include "base/memory/scoped_ptr.h"
+#include "chrome/browser/instant/instant_page.h"
+#include "chrome/browser/ui/tab_contents/core_tab_helper_delegate.h"
+#include "content/public/browser/web_contents_delegate.h"
+
+class InstantService;
+
+namespace content {
+class WebContents;
+}
+
+// InstantWebContentsContainer owns a WebContents that's shown as an Instant

[samarth, 2013/03/01 17:59:53]

I'm finding the relationship between InstantLoader, InstantWebContentsContainer
and InstantOverlay to be confusing.

How about something like this?
InstantLoader (or perhaps InstantContentsContainer) is responsible for both for
loading the instant contents and is also the web contents delegate.  When a
pre-loaded page becomes an overlay, InstantService just hands off ownership of
its instance of InstantLoader to InstantOverlay, and loads up a new one.

If we want somewhat different behavior for the page while it's a preloaded page
and while it's an overlay, we can codify that as
InstantContentsContainer::Delegate--initially it's InstantService, but when you
hand it off to InstantOverlay, InstantOverlay becomes the delegate.

[sreeram, 2013/03/07 18:18:46]

As discussed offline, I've kept the current structure, but renamed InstantLoader
to InstantPreloader to make the distinction clearer (thanks Shishir for the
name!).

[samarth, 2013/03/11 19:03:24]

Sorry, I'm looking at this again and this still doesn't feel right to me.  For
instance, we have this structure where InstantOverlay <-
InstantWebContentsContainer <- WebContentsDelegate (<- = inherits from) and
methods from WebContentsDelegate are being overridden in both places. This makes
it hard to see exactly what things get overriden.

In our offline discussion, you said the main reason for having
InstantWebContentsContainer is to prevent code duplication. AFAICT, the only
non-trivial pieces of InstantWebContentsContainer that are used by both
InstantPreloader and InstantOverlay are OpenURLFromTab and
SetUp/TearDownContents. We could easily move the logic from those into helpers
that InstantOverlay and InstantPreloader call. And then InstantOverlay and
InstantPreloader could each just separately inherit from
content::WebContentsDelegate. To go from the Preloader to Overlay, you would
just release from the former to give it to the latter.

I really think this will make things clearer and easier to understand, but let's
talk more offline (perhaps with David as well) when you're in.

+// overlay. It can't be instantiated per se, but exists by way of its derived
+// classes (InstantLoader and InstantOverlay). It implements WebContentsDelegate
+// methods that are common to the derived classes. It also owns an InstantPage
+// object that it uses to talk the SearchBox API with its contents.
+class InstantWebContentsContainer : public CoreTabHelperDelegate,
+                                    public content::WebContentsDelegate {
+ public:
+  content::WebContents* contents() const { return contents_.get(); }
+
+ protected:
+  InstantWebContentsContainer(InstantPage::Delegate* delegate,
+                              InstantService* service);
+  virtual ~InstantWebContentsContainer();
+
+  // Overridden from CoreTabHelperDelegate:
+  virtual void SwapTabContents(content::WebContents* old_contents,
+                               content::WebContents* new_contents) OVERRIDE;
+
+  // Overriden from content::WebContentsDelegate:
+  virtual content::WebContents* OpenURLFromTab(
+      content::WebContents* source,
+      const content::OpenURLParams& params) OVERRIDE;
+  virtual bool ShouldFocusPageAfterCrash() OVERRIDE;
+  virtual bool CanDownload(content::RenderViewHost* render_view_host,
+                           int request_id,
+                           const std::string& request_method) OVERRIDE;
+  virtual bool OnGoToEntryOffset(int offset) OVERRIDE;
+
+  virtual void SetUpContents();
+  virtual void TearDownContents();
+
+  scoped_ptr<content::WebContents> contents_;
+  InstantPage page_;
+
+  DISALLOW_COPY_AND_ASSIGN(InstantWebContentsContainer);
+};
+
+#endif  // CHROME_BROWSER_INSTANT_INSTANT_WEB_CONTENTS_CONTAINER_H_