Wires TabContents to app extensions.

BUG=32845
TEST=none

Review URL: http://codereview.chromium.org/566032

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@38212 0039d316-1c4b-4281-b951-d872f2087c98

11 files changed, 174 insertions, 58 deletions

diff --git a/chrome/browser/browser_browsertest.cc b/chrome/browser/browser_browsertest.cc
index aa048f6..988ab96 100644
--- a/chrome/browser/browser_browsertest.cc
+++ b/chrome/browser/browser_browsertest.cc
@@ -11,8 +11,13 @@
 #include "chrome/browser/browser.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/defaults.h"
+#include "chrome/browser/extensions/extension_browsertest.h"
+#include "chrome/browser/extensions/extensions_service.h"
+#include "chrome/browser/profile.h"
 #include "chrome/browser/renderer_host/render_process_host.h"
 #include "chrome/browser/tab_contents/tab_contents.h"
+#include "chrome/common/chrome_switches.h"
+#include "chrome/common/extensions/extension.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/common/page_transition_types.h"
 #include "chrome/test/in_process_browser_test.h"
@@ -58,38 +63,50 @@ int CountRenderProcessHosts() {
 
 }  // namespace
 
-class BrowserTest : public InProcessBrowserTest {
+class BrowserTest : public ExtensionBrowserTest {
  public:
   // Used by phantom tab tests. Creates two tabs, pins the first and makes it
   // a phantom tab (by closing it).
   void PhantomTabTest() {
-    static const wchar_t kDocRoot[] = L"chrome/test/data";
-    scoped_refptr<HTTPTestServer> server(
-        HTTPTestServer::CreateServer(kDocRoot, NULL));
-    ASSERT_TRUE(NULL != server.get());
+    HTTPTestServer* server = StartHTTPServer();
+    ASSERT_TRUE(server);
     GURL url(server->TestServerPage("empty.html"));
     TabStripModel* model = browser()->tabstrip_model();
 
+    ASSERT_TRUE(LoadExtension(test_data_dir_.AppendASCII("app/")));
+
+    Extension* app_extension = GetExtension();
+
     ui_test_utils::NavigateToURL(browser(), url);
-    model->GetTabContentsAt(0)->set_app(true);
-    model->SetTabPinned(0, true);
 
-    browser()->AddTabWithURL(GURL("about:blank"), GURL(), PageTransition::TYPED,
-                             true, 1, false, NULL);
+    TabContents* app_contents = new TabContents(browser()->profile(), NULL,
+                                                MSG_ROUTING_NONE, NULL);
+    app_contents->SetAppExtension(app_extension);
+
+    model->AddTabContents(app_contents, 0, false, 0, false);
+    model->SetTabPinned(0, true);
     ui_test_utils::NavigateToURL(browser(), url);
 
     // Close the first, which should make it a phantom.
-    TabContents* initial_contents = model->GetTabContentsAt(0);
     model->CloseTabContentsAt(0);
+
     // There should still be two tabs.
     ASSERT_EQ(2, browser()->tab_count());
     // The first tab should be a phantom.
     EXPECT_TRUE(model->IsPhantomTab(0));
     // And the tab contents of the first tab should have changed.
-    EXPECT_TRUE(model->GetTabContentsAt(0) != initial_contents);
+    EXPECT_TRUE(model->GetTabContentsAt(0) != app_contents);
   }
 
+
  protected:
+  virtual void SetUpCommandLine(CommandLine* command_line) {
+    ExtensionBrowserTest::SetUpCommandLine(command_line);
+
+    // Needed for phantom tab tests.
+    command_line->AppendSwitch(switches::kEnableExtensionApps);
+  }
+
   // In RTL locales wrap the page title with RTL embedding characters so that it
   // matches the value returned by GetWindowTitle().
   std::wstring LocaleWindowCaptionFromPageTitle(
@@ -108,6 +125,18 @@ class BrowserTest : public InProcessBrowserTest {
     return page_title;
 #endif
   }
+
+  // Returns the app extension installed by PhantomTabTest.
+  Extension* GetExtension() {
+    const ExtensionList* extensions =
+        browser()->profile()->GetExtensionsService()->extensions();
+    for (size_t i = 0; i < extensions->size(); ++i) {
+      if ((*extensions)[i]->name() == "App Test")
+        return (*extensions)[i];
+    }
+    NOTREACHED();
+    return NULL;
+  }
 };
 
 // Launch the app on a page with no title, check that the app title was set
@@ -324,7 +353,7 @@ IN_PROC_BROWSER_TEST_F(BrowserTest, FaviconOfOnloadRedirectToAnchorPage) {
   EXPECT_EQ(expected_favicon_url.spec(), entry->favicon().url().spec());
 }
 
-// TODO(sky): get these to run a Mac.
+// TODO(sky): get these to run on a Mac.
 #if !defined(OS_MACOSX)
 IN_PROC_BROWSER_TEST_F(BrowserTest, PhantomTab) {
   PhantomTabTest();
@@ -346,6 +375,17 @@ IN_PROC_BROWSER_TEST_F(BrowserTest, RevivePhantomTab) {
   // The first tab should no longer be a phantom.
   EXPECT_FALSE(model->IsPhantomTab(0));
 }
+
+IN_PROC_BROWSER_TEST_F(BrowserTest, AppTabRemovedWhenExtensionUninstalled) {
+  PhantomTabTest();
+
+  Extension* extension = GetExtension();
+  UninstallExtension(extension->id());
+
+  // The uninstall should have removed the tab.
+  ASSERT_EQ(1, browser()->tab_count());
+}
+
 #endif
 
 // Tests that the CLD (Compact Language Detection) works properly.
diff --git a/chrome/browser/extensions/extension_browsertest.cc b/chrome/browser/extensions/extension_browsertest.cc
index f711fbd..252e2e6 100644
--- a/chrome/browser/extensions/extension_browsertest.cc
+++ b/chrome/browser/extensions/extension_browsertest.cc
@@ -30,8 +30,6 @@
 // minimize false positives.
 static const int kTimeoutMs = 60 * 1000;  // 1 minute
 
-// Base class for extension browser tests. Provides utilities for loading,
-// unloading, and installing extensions.
 void ExtensionBrowserTest::SetUpCommandLine(CommandLine* command_line) {
   // This enables DOM automation for tab contentses.
   EnableDOMAutomation();
diff --git a/chrome/browser/extensions/extension_browsertest.h b/chrome/browser/extensions/extension_browsertest.h
index 7c116b5..65fb889 100644
--- a/chrome/browser/extensions/extension_browsertest.h
+++ b/chrome/browser/extensions/extension_browsertest.h
@@ -70,6 +70,8 @@ class ExtensionBrowserTest
 
   bool loaded_;
   bool installed_;
+
+  // test_data/extensions.
   FilePath test_data_dir_;
   std::string last_loaded_extension_id_;
   int extension_installs_observed_;
diff --git a/chrome/browser/tab_contents/tab_contents.cc b/chrome/browser/tab_contents/tab_contents.cc
index 1b9bcf7..aa59881 100644
--- a/chrome/browser/tab_contents/tab_contents.cc
+++ b/chrome/browser/tab_contents/tab_contents.cc
@@ -62,6 +62,7 @@
 #include "chrome/browser/search_engines/template_url_fetcher.h"
 #include "chrome/browser/search_engines/template_url_model.h"
 #include "chrome/common/chrome_switches.h"
+#include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_action.h"
 #include "chrome/common/notification_service.h"
 #include "chrome/common/platform_util.h"
@@ -267,7 +268,7 @@ TabContents::TabContents(Profile* profile,
       is_showing_before_unload_dialog_(false),
       renderer_preferences_(),
       opener_dom_ui_type_(DOMUIFactory::kNoDOMUI),
-      app_(false) {
+      app_extension_(NULL) {
   ClearBlockedContentSettings();
   renderer_preferences_util::UpdateFromSystemSettings(
       &renderer_preferences_, profile);
@@ -453,6 +454,11 @@ PluginInstaller* TabContents::GetPluginInstaller() {
   return plugin_installer_.get();
 }
 
+void TabContents::SetAppExtension(Extension* extension) {
+  DCHECK(!extension || extension->IsApp());
+  app_extension_ = extension;
+}
+
 const GURL& TabContents::GetURL() const {
   // We may not have a navigation entry yet
   NavigationEntry* entry = controller_.GetActiveEntry();
@@ -1224,23 +1230,17 @@ void TabContents::OnCloseStarted() {
 }
 
 TabContents* TabContents::CloneAndMakePhantom() {
-  // TODO(sky): the initial URL, title and what not should come from the app.
-  NavigationEntry* entry = controller().GetActiveEntry();
+  DCHECK(app_extension_);  // Should only be invoked on apps.
 
   TabNavigation tab_nav;
-  if (entry)
-    tab_nav.SetFromNavigationEntry(*entry);
+  tab_nav.set_url(app_extension_->app_launch_url());
   std::vector<TabNavigation> navigations;
   navigations.push_back(tab_nav);
 
   TabContents* new_contents =
       new TabContents(profile(), NULL, MSG_ROUTING_NONE, NULL);
+  new_contents->SetAppExtension(app_extension_);
   new_contents->controller().RestoreFromState(navigations, 0, false);
-  new_contents->app_ = app_;
-  if (entry) {
-    // TODO(sky): this should come from the app.
-    new_contents->controller().GetActiveEntry()->favicon() = entry->favicon();
-  }
 
   return new_contents;
 }
diff --git a/chrome/browser/tab_contents/tab_contents.h b/chrome/browser/tab_contents/tab_contents.h
index d74be111..97a2628 100644
--- a/chrome/browser/tab_contents/tab_contents.h
+++ b/chrome/browser/tab_contents/tab_contents.h
@@ -73,6 +73,7 @@ class AutoFillManager;
 class BlockedPopupContainer;
 class DOMUI;
 class DownloadItem;
+class Extension;
 class FormFieldHistoryManager;
 class LoadNotificationDetails;
 class OmniboxSearchHint;
@@ -120,7 +121,6 @@ class TabContents : public PageNavigator,
 
   // |base_tab_contents| is used if we want to size the new tab contents view
   // based on an existing tab contents view.  This can be NULL if not needed.
-  // TODO(sky): make constructor take Extension.
   TabContents(Profile* profile,
               SiteInstance* site_instance,
               int routing_id,
@@ -184,10 +184,16 @@ class TabContents : public PageNavigator,
     return fav_icon_helper_;
   }
 
-  // TODO(sky): whether a tab is an app should be determined by the constructor,
-  // not a setter, and should likely take the extension.
-  void set_app(bool app) { app_ = app; }
-  bool app() const { return app_; }
+  // Sets the extension denoting this as an app. If |extension| is non-null this
+  // tab becomes an app-tab. TabContents does not listen for unload events for
+  // the extension. It's up to consumers of TabContents to do that.
+  //
+  // NOTE: this should only be manipulated before the tab is added to a browser.
+  // TODO(sky): resolve if this is the right way to identify an app tab. If it
+  // is, than this should be passed in the constructor.
+  void SetAppExtension(Extension* extension);
+  Extension* app_extension() const { return app_extension_; }
+  bool is_app() const { return app_extension_ != NULL; }
 
 #ifdef UNIT_TEST
   // Expose the render manager for testing.
@@ -1184,9 +1190,9 @@ class TabContents : public PageNavigator,
   // profile
   scoped_refptr<URLRequestContextGetter> request_context_;
 
-  // This is temporary until wired to extensions.
-  // TODO(sky): fix this.
-  bool app_;
+  // If non-null this tab is an app tab and this is the extension the tab was
+  // created for.
+  Extension* app_extension_;
 
   // ---------------------------------------------------------------------------
 
diff --git a/chrome/browser/tabs/tab_strip_model.cc b/chrome/browser/tabs/tab_strip_model.cc
index 4e3708c..39dea18 100644
--- a/chrome/browser/tabs/tab_strip_model.cc
+++ b/chrome/browser/tabs/tab_strip_model.cc
@@ -13,6 +13,7 @@
 #include "chrome/browser/bookmarks/bookmark_model.h"
 #include "chrome/browser/browser_shutdown.h"
 #include "chrome/browser/defaults.h"
+#include "chrome/browser/extensions/extensions_service.h"
 #include "chrome/browser/metrics/user_metrics.h"
 #include "chrome/browser/profile.h"
 #include "chrome/browser/renderer_host/render_process_host.h"
@@ -23,6 +24,7 @@
 #include "chrome/browser/tab_contents/tab_contents_delegate.h"
 #include "chrome/browser/tab_contents/tab_contents_view.h"
 #include "chrome/common/chrome_switches.h"
+#include "chrome/common/extensions/extension.h"
 #include "chrome/common/notification_service.h"
 #include "chrome/common/url_constants.h"
 
@@ -56,6 +58,9 @@ TabStripModel::TabStripModel(TabStripModelDelegate* delegate, Profile* profile)
   registrar_.Add(this,
       NotificationType::TAB_CONTENTS_DESTROYED,
       NotificationService::AllSources());
+  registrar_.Add(this,
+                 NotificationType::EXTENSION_UNLOADED,
+                 Source<Profile>(profile_));
   order_controller_ = new TabStripModelOrderController(this);
 }
 
@@ -104,7 +109,7 @@ void TabStripModel::InsertTabContentsAt(int index,
                                         bool inherit_group) {
   // Make sure the index maintains that all app tab occurs before non-app tabs.
   int first_non_app = IndexOfFirstNonAppTab();
-  if (contents->app())
+  if (contents->is_app())
     index = std::min(first_non_app, index);
   else
     index = std::max(first_non_app, index);
@@ -418,7 +423,7 @@ bool TabStripModel::IsTabPinned(int index) const {
 }
 
 bool TabStripModel::IsAppTab(int index) const {
-  return GetTabContentsAt(index)->app();
+  return GetTabContentsAt(index)->is_app();
 }
 
 bool TabStripModel::IsPhantomTab(int index) const {
@@ -432,7 +437,7 @@ bool TabStripModel::IsTabBlocked(int index) const {
 
 int TabStripModel::IndexOfFirstNonAppTab() const {
   for (size_t i = 0; i < contents_data_.size(); ++i) {
-    if (!contents_data_[i]->contents->app())
+    if (!contents_data_[i]->contents->is_app())
       return static_cast<int>(i);
   }
   // No app tabs.
@@ -670,22 +675,41 @@ std::vector<int> TabStripModel::GetIndexesOpenedBy(int index) const {
 void TabStripModel::Observe(NotificationType type,
                             const NotificationSource& source,
                             const NotificationDetails& details) {
-  DCHECK(type == NotificationType::TAB_CONTENTS_DESTROYED);
-  // Sometimes, on qemu, it seems like a TabContents object can be destroyed
-  // while we still have a reference to it. We need to break this reference
-  // here so we don't crash later.
-  int index = GetIndexOfTabContents(Source<TabContents>(source).ptr());
-  if (index != TabStripModel::kNoTab) {
-    // Note that we only detach the contents here, not close it - it's already
-    // been closed. We just want to undo our bookkeeping.
-    if (IsAppTab(index) && IsTabPinned(index) && !IsPhantomTab(index) &&
-        !closing_all_) {
-      // We don't actually allow pinned tabs to close. Instead they become
-      // phantom.
-      MakePhantom(index);
-    } else {
-      DetachTabContentsAt(index);
+  switch (type.value) {
+    case NotificationType::TAB_CONTENTS_DESTROYED: {
+      // Sometimes, on qemu, it seems like a TabContents object can be destroyed
+      // while we still have a reference to it. We need to break this reference
+      // here so we don't crash later.
+      int index = GetIndexOfTabContents(Source<TabContents>(source).ptr());
+      if (index != TabStripModel::kNoTab) {
+        // Note that we only detach the contents here, not close it - it's
+        // already been closed. We just want to undo our bookkeeping.
+        if (ShouldMakePhantomOnClose(index)) {
+          // We don't actually allow pinned tabs to close. Instead they become
+          // phantom.
+          MakePhantom(index);
+        } else {
+          DetachTabContentsAt(index);
+        }
+      }
+      break;
+    }
+
+    case NotificationType::EXTENSION_UNLOADED: {
+      Extension* extension = Details<Extension>(details).ptr();
+      // Iterate backwards as we may remove items while iterating.
+      for (int i = count() - 1; i >= 0; i--) {
+        if (GetTabContentsAt(i)->app_extension() == extension) {
+          // The extension an app tab was created from has been nuked. Delete
+          // the TabContents.
+          delete GetTabContentsAt(i);
+        }
+      }
+      break;
     }
+
+    default:
+      NOTREACHED();
   }
 }
 
@@ -828,6 +852,23 @@ int TabStripModel::IndexOfNextNonPhantomTab(int index,
   return start;
 }
 
+bool TabStripModel::ShouldMakePhantomOnClose(int index) {
+  if (IsAppTab(index) && IsTabPinned(index) && !IsPhantomTab(index) &&
+      !closing_all_ && profile()) {
+    ExtensionsService* extension_service = profile()->GetExtensionsService();
+    if (!extension_service)
+      return false;
+
+    Extension* app_extension = GetTabContentsAt(index)->app_extension();
+    DCHECK(app_extension);
+
+    // Only allow the tab to be made phantom if the extension still exists.
+    return extension_service->GetExtensionById(app_extension->id(),
+                                               false) != NULL;
+  }
+  return false;
+}
+
 void TabStripModel::MakePhantom(int index) {
   if (selected_index_ == index) {
     // Change the selection, otherwise we're going to force the phantom tab
diff --git a/chrome/browser/tabs/tab_strip_model.h b/chrome/browser/tabs/tab_strip_model.h
index 24eb20e..305c562 100644
--- a/chrome/browser/tabs/tab_strip_model.h
+++ b/chrome/browser/tabs/tab_strip_model.h
@@ -592,6 +592,10 @@ class TabStripModel : public NotificationObserver {
   // |ignore_index|.
   int IndexOfNextNonPhantomTab(int index, int ignore_index);
 
+  // Returns true if the tab at the specified index should be made phantom when
+  // the tab is closing.
+  bool ShouldMakePhantomOnClose(int index);
+
   // Makes the tab a phantom tab.
   void MakePhantom(int index);
 
diff --git a/chrome/browser/tabs/tab_strip_model_unittest.cc b/chrome/browser/tabs/tab_strip_model_unittest.cc
index 7330a41..f577bc6 100644
--- a/chrome/browser/tabs/tab_strip_model_unittest.cc
+++ b/chrome/browser/tabs/tab_strip_model_unittest.cc
@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "app/system_monitor.h"
+#include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/path_service.h"
 #include "base/string_util.h"
@@ -16,6 +17,7 @@
 #include "chrome/browser/tab_contents/tab_contents.h"
 #include "chrome/browser/tabs/tab_strip_model.h"
 #include "chrome/browser/tabs/tab_strip_model_order_controller.h"
+#include "chrome/common/extensions/extension.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/property_bag.h"
 #include "chrome/common/url_constants.h"
@@ -1347,10 +1349,17 @@ TEST_F(TabStripModelTest, Apps) {
 
   typedef MockTabStripModelObserver::State State;
 
+#if defined(OS_WIN)
+  FilePath path(FILE_PATH_LITERAL("c:\\foo"));
+#elif defined(OS_POSIX)
+  FilePath path(FILE_PATH_LITERAL("/foo"));
+#endif
+  Extension app_extension(path);
+  app_extension.app_launch_url_ = GURL("http://www.google.com");
   TabContents* contents1 = CreateTabContents();
-  contents1->set_app(true);
+  contents1->SetAppExtension(&app_extension);
   TabContents* contents2 = CreateTabContents();
-  contents2->set_app(true);
+  contents2->SetAppExtension(&app_extension);
   TabContents* contents3 = CreateTabContents();
 
   SetID(contents1, 1);
diff --git a/chrome/common/extensions/extension.cc b/chrome/common/extensions/extension.cc
index 8bd7b2f..eef996e 100644
--- a/chrome/common/extensions/extension.cc
+++ b/chrome/common/extensions/extension.cc
@@ -783,10 +783,10 @@ bool Extension::InitFromValue(const DictionaryValue& source, bool require_id,
   if (source.HasKey(keys::kPublicKey)) {
     std::string public_key_bytes;
     if (!source.GetString(keys::kPublicKey, &public_key_) ||
-      !ParsePEMKeyBytes(public_key_, &public_key_bytes) ||
-      !GenerateId(public_key_bytes, &id_)) {
-        *error = errors::kInvalidKey;
-        return false;
+        !ParsePEMKeyBytes(public_key_, &public_key_bytes) ||
+        !GenerateId(public_key_bytes, &id_)) {
+      *error = errors::kInvalidKey;
+      return false;
     }
   } else if (require_id) {
     *error = errors::kInvalidKey;
diff --git a/chrome/common/extensions/extension.h b/chrome/common/extensions/extension.h
index c858534..0e521c4 100644
--- a/chrome/common/extensions/extension.h
+++ b/chrome/common/extensions/extension.h
@@ -423,16 +423,17 @@ class Extension {
 
   // The URL an app should launch to.
   GURL app_launch_url_;
-  
+
   // The type of window to start when the application is launched.
   AppLaunchWindowType app_launch_window_type_;
-  
+
   // Runtime data:
 
   // True if the background page is ready.
   bool background_page_ready_;
 
   FRIEND_TEST(ExtensionTest, LoadPageActionHelper);
+  FRIEND_TEST(TabStripModelTest, Apps);
 
   DISALLOW_COPY_AND_ASSIGN(Extension);
 };
diff --git a/chrome/test/data/extensions/app/manifest.json b/chrome/test/data/extensions/app/manifest.json
new file mode 100644
index 0000000..959e75b
--- /dev/null
+++ b/chrome/test/data/extensions/app/manifest.json
@@ -0,0 +1,15 @@
+{
+  "name": "App Test",
+  "version": "1",
+  "permissions": [
+    "notifications"
+  ],
+  "app": {
+    "extent": [
+      "http://www.example.com/empty.html"
+    ],
+    "launch": {
+      "url": "http://www.example.com/empty.html"
+    }
+  }
+}