Extension tweaks for phantom tabs. I did the following modifications:

. Modified chrome.tabs.executeScript so that it sends error message if
  failure in sending message (which happens with phantom tabs).
. When a tab is made phantom we send TabInsertedAt event.
. Made connecting to a phantom tab send disconnect.
. Disallow changing the url of pinned tabs.
. Disallow closing phantom tabs.
. Detect language fails for phantom tabs.

And I removed the flag for phantom tabs.

BUG=25309
TEST=on windows or chromeos create a window with a couple of tabs, pin
the first, then close it. Selection should change to another tab and
the favicon of the closed tab should remain, but not the tab
border. control-tab should not select the phantom tab, but if you
click on the phantom tab it should become live again and the border
should reappear.

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

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

11 files changed, 67 insertions, 23 deletions

diff --git a/chrome/browser/browser_browsertest.cc b/chrome/browser/browser_browsertest.cc
index 7146210..bcf431e 100644
--- a/chrome/browser/browser_browsertest.cc
+++ b/chrome/browser/browser_browsertest.cc
@@ -323,8 +323,6 @@ IN_PROC_BROWSER_TEST_F(BrowserTest, FaviconOfOnloadRedirectToAnchorPage) {
   EXPECT_EQ(expected_favicon_url.spec(), entry->favicon().url().spec());
 }
 
-// TODO(sky): enable these once phantom tabs aren't behind a flag.
-/*
 IN_PROC_BROWSER_TEST_F(BrowserTest, PhantomTab) {
   if (!browser_defaults::kPinnedTabsActLikeApps)
     return;
@@ -351,7 +349,6 @@ IN_PROC_BROWSER_TEST_F(BrowserTest, RevivePhantomTab) {
   // The first tab should no longer be a phantom.
   EXPECT_FALSE(model->IsPhantomTab(0));
 }
-*/
 
 // Tests that the CLD (Compact Language Detection) works properly.
 IN_PROC_BROWSER_TEST_F(BrowserTest, PageLanguageDetection) {
diff --git a/chrome/browser/extensions/execute_code_in_tab_function.cc b/chrome/browser/extensions/execute_code_in_tab_function.cc
index 56f3353..46660cd 100644
--- a/chrome/browser/extensions/execute_code_in_tab_function.cc
+++ b/chrome/browser/extensions/execute_code_in_tab_function.cc
@@ -95,7 +95,8 @@ bool ExecuteCodeInTabFunction::RunImpl() {
   }
 
   if (!code_string.empty()) {
-    Execute(code_string);
+    if (!Execute(code_string))
+      return false;
     return true;
   }
 
@@ -135,19 +136,19 @@ void ExecuteCodeInTabFunction::DidLoadFile(bool success,
   Release();  // Balance the AddRef taken in RunImpl
 }
 
-void ExecuteCodeInTabFunction::Execute(const std::string& code_string) {
+bool ExecuteCodeInTabFunction::Execute(const std::string& code_string) {
   TabContents* contents = NULL;
   Browser* browser = NULL;
   if (!ExtensionTabUtil::GetTabById(execute_tab_id_, profile(), &browser, NULL,
                                     &contents, NULL) && contents && browser) {
     SendResponse(false);
-    return;
+    return false;
   }
 
   Extension* extension = GetExtension();
   if (!extension) {
     SendResponse(false);
-    return;
+    return false;
   }
 
   bool is_js_code = true;
@@ -157,12 +158,16 @@ void ExecuteCodeInTabFunction::Execute(const std::string& code_string) {
   } else if (function_name != TabsExecuteScriptFunction::function_name()) {
     DCHECK(false);
   }
+  if (!contents->ExecuteCode(request_id(), extension->id(),
+                             extension->host_permissions(), is_js_code,
+                             code_string, all_frames_)) {
+    SendResponse(false);
+    return false;
+  }
   registrar_.Add(this, NotificationType::TAB_CODE_EXECUTED,
                  NotificationService::AllSources());
   AddRef();  // balanced in Observe()
-  contents->ExecuteCode(request_id(), extension->id(),
-                        extension->host_permissions(), is_js_code, code_string,
-                        all_frames_);
+  return true;
 }
 
 void ExecuteCodeInTabFunction::Observe(NotificationType type,
diff --git a/chrome/browser/extensions/execute_code_in_tab_function.h b/chrome/browser/extensions/execute_code_in_tab_function.h
index bd765f5..8da8b75 100644
--- a/chrome/browser/extensions/execute_code_in_tab_function.h
+++ b/chrome/browser/extensions/execute_code_in_tab_function.h
@@ -34,8 +34,9 @@ class ExecuteCodeInTabFunction : public AsyncExtensionFunction,
   // arguments has been loaded.
   void DidLoadFile(bool success, const std::string& data);
 
-  // Run in UI thread.  Code string contains the code to be executed.
-  void Execute(const std::string& code_string);
+  // Run in UI thread.  Code string contains the code to be executed. Returns
+  // true on success. If true is returned, this does an AddRef.
+  bool Execute(const std::string& code_string);
 
   NotificationRegistrar registrar_;
 
diff --git a/chrome/browser/extensions/extension_browser_event_router.cc b/chrome/browser/extensions/extension_browser_event_router.cc
index 667c9e3..990a0a1 100644
--- a/chrome/browser/extensions/extension_browser_event_router.cc
+++ b/chrome/browser/extensions/extension_browser_event_router.cc
@@ -399,10 +399,11 @@ void ExtensionBrowserEventRouter::TabChangedAt(TabContents* contents,
 void ExtensionBrowserEventRouter::TabReplacedAt(TabContents* old_contents,
                                                 TabContents* new_contents,
                                                 int index) {
-  // TODO: figure out the right notification to send.
+  // TODO: 32913, consider adding better notification for this event.
+  TabInsertedAt(new_contents, index, false);
 }
 
-void ExtensionBrowserEventRouter::TabStripEmpty() { }
+void ExtensionBrowserEventRouter::TabStripEmpty() {}
 
 void ExtensionBrowserEventRouter::DispatchOldPageActionEvent(
     Profile* profile,
diff --git a/chrome/browser/extensions/extension_message_service.cc b/chrome/browser/extensions/extension_message_service.cc
index 5aaeb73..9339196 100644
--- a/chrome/browser/extensions/extension_message_service.cc
+++ b/chrome/browser/extensions/extension_message_service.cc
@@ -280,7 +280,7 @@ void ExtensionMessageService::OpenChannelToTabOnUIThread(
   if (!source)
     return;
 
-  TabContents* contents;
+  TabContents* contents = NULL;
   MessagePort receiver;
   receiver.debug_info = 2;
   if (ExtensionTabUtil::GetTabById(tab_id, source->profile(),
@@ -289,6 +289,15 @@ void ExtensionMessageService::OpenChannelToTabOnUIThread(
     receiver.routing_id = contents->render_view_host()->routing_id();
     receiver.debug_info = 3;
   }
+
+  if (contents && contents->controller().needs_reload()) {
+    // The tab isn't loaded yet (it may be phantom). Don't attempt to connect.
+    // Treat this as a disconnect.
+    DispatchOnDisconnect(MessagePort(source, MSG_ROUTING_CONTROL),
+                         GET_OPPOSITE_PORT_ID(receiver_port_id));
+    return;
+  }
+
   TabContents* source_contents = tab_util::GetTabContentsByID(
       source_process_id, source_routing_id);
 
@@ -378,6 +387,11 @@ int ExtensionMessageService::OpenSpecialChannelToTab(
   DCHECK_EQ(MessageLoop::current()->type(), MessageLoop::TYPE_UI);
   DCHECK(target_tab_contents);
 
+  if (target_tab_contents->controller().needs_reload()) {
+    // The tab isn't loaded yet (it may be phantom). Don't attempt to connect.
+    return -1;
+  }
+
   int port1_id = -1;
   int port2_id = -1;
   // Create a channel ID for both sides of the channel.
diff --git a/chrome/browser/extensions/extension_tabs_module.cc b/chrome/browser/extensions/extension_tabs_module.cc
index 36cf400..a006a68 100644
--- a/chrome/browser/extensions/extension_tabs_module.cc
+++ b/chrome/browser/extensions/extension_tabs_module.cc
@@ -19,6 +19,7 @@
 #include "chrome/browser/renderer_host/render_view_host_delegate.h"
 #include "chrome/browser/tab_contents/navigation_entry.h"
 #include "chrome/browser/tab_contents/tab_contents.h"
+#include "chrome/browser/tabs/tab_strip_model.h"
 #include "chrome/browser/window_sizer.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_error_utils.h"
@@ -559,6 +560,12 @@ bool UpdateTabFunction::RunImpl() {
       &error_))
     return false;
 
+  if (tab_strip->IsTabPinned(tab_index)) {
+    // Don't allow changing the url of pinned tabs.
+    error_ = keys::kCannotUpdatePinnedTab;
+    return false;
+  }
+
   NavigationController& controller = contents->controller();
 
   // TODO(rafaelw): handle setting remaining tab properties:
@@ -707,6 +714,13 @@ bool RemoveTabFunction::RunImpl() {
   if (!GetTabById(tab_id, profile(), &browser, NULL, &contents, NULL, &error_))
     return false;
 
+  int tab_index = browser->GetIndexOfController(&contents->controller());
+  if (browser->tabstrip_model()->IsPhantomTab(tab_index)) {
+    // Don't allow closing phantom tabs.
+    error_ = keys::kCannotRemovePhantomTab;
+    return false;
+  }
+
   browser->CloseTabContents(contents);
   return true;
 }
@@ -855,6 +869,13 @@ bool DetectTabLanguageFunction::RunImpl() {
       return false;
   }
 
+  if (contents->controller().needs_reload()) {
+    // If the tab hasn't been loaded, such as happens with phantom tabs, don't
+    // wait for the tab to load, instead return.
+    error_ = keys::kCannotDetermineLanguageOfUnloadedTab;
+    return false;
+  }
+
   AddRef();  // Balanced in GotLanguage()
 
   NavigationEntry* entry = contents->controller().GetActiveEntry();
diff --git a/chrome/browser/extensions/extension_tabs_module_constants.cc b/chrome/browser/extensions/extension_tabs_module_constants.cc
index 8e62d68..4232a77 100644
--- a/chrome/browser/extensions/extension_tabs_module_constants.cc
+++ b/chrome/browser/extensions/extension_tabs_module_constants.cc
@@ -55,5 +55,9 @@ const char kNoCodeOrFileToExecuteError[] = "No source code or file specified.";
 const char kMoreThanOneValuesError[] = "Code and file should not be specified "
     "at the same time in the second argument.";
 const char kLoadFileError[] = "Failed to load file: \"*\". ";
+const char kCannotUpdatePinnedTab[] = "Cannot update pinned tabs";
+const char kCannotRemovePhantomTab[] = "Cannot remove phantom tabs";
+const char kCannotDetermineLanguageOfUnloadedTab[] =
+    "Cannot determine language: tab not loaded";
 
 }  // namespace extension_tabs_module_constants
diff --git a/chrome/browser/extensions/extension_tabs_module_constants.h b/chrome/browser/extensions/extension_tabs_module_constants.h
index 3cd8f76..ce340e5 100644
--- a/chrome/browser/extensions/extension_tabs_module_constants.h
+++ b/chrome/browser/extensions/extension_tabs_module_constants.h
@@ -57,6 +57,9 @@ extern const char kSupportedInWindowsOnlyError[];
 extern const char kNoCodeOrFileToExecuteError[];
 extern const char kMoreThanOneValuesError[];
 extern const char kLoadFileError[];
+extern const char kCannotUpdatePinnedTab[];
+extern const char kCannotRemovePhantomTab[];
+extern const char kCannotDetermineLanguageOfUnloadedTab[];
 
 };  // namespace extension_tabs_module_constants
 
diff --git a/chrome/browser/tab_contents/tab_contents.cc b/chrome/browser/tab_contents/tab_contents.cc
index 0ca822b..27c215e 100644
--- a/chrome/browser/tab_contents/tab_contents.cc
+++ b/chrome/browser/tab_contents/tab_contents.cc
@@ -833,11 +833,11 @@ void TabContents::CloseAllSuppressedPopups() {
     blocked_popups_->CloseAll();
 }
 
-void TabContents::ExecuteCode(int request_id, const std::string& extension_id,
+bool TabContents::ExecuteCode(int request_id, const std::string& extension_id,
                               const std::vector<URLPattern>& host_permissions,
                               bool is_js_code, const std::string& code_string,
                               bool all_frames) {
-  render_view_host()->Send(
+  return render_view_host()->Send(
       new ViewMsg_ExecuteCode(render_view_host()->routing_id(),
           ViewMsg_ExecuteCode_Params(request_id, extension_id, host_permissions,
                                      is_js_code, code_string, all_frames)));
diff --git a/chrome/browser/tab_contents/tab_contents.h b/chrome/browser/tab_contents/tab_contents.h
index 0603d6b..2ea5bf5 100644
--- a/chrome/browser/tab_contents/tab_contents.h
+++ b/chrome/browser/tab_contents/tab_contents.h
@@ -367,8 +367,9 @@ class TabContents : public PageNavigator,
   // of unwanted popups.
   void CloseAllSuppressedPopups();
 
-  // Execute code in this tab.
-  void ExecuteCode(int request_id, const std::string& extension_id,
+  // Execute code in this tab. Returns true if the message was successfully
+  // sent.
+  bool ExecuteCode(int request_id, const std::string& extension_id,
                    const std::vector<URLPattern>& host_permissions,
                    bool is_js_code, const std::string& code_string,
                    bool all_frames);
diff --git a/chrome/browser/tabs/tab_strip_model.cc b/chrome/browser/tabs/tab_strip_model.cc
index 39f4b66..409815a 100644
--- a/chrome/browser/tabs/tab_strip_model.cc
+++ b/chrome/browser/tabs/tab_strip_model.cc
@@ -397,10 +397,7 @@ bool TabStripModel::IsTabPinned(int index) const {
 
 bool TabStripModel::IsAppTab(int index) const {
   // TODO (sky): this is temporary and should be integrated with real apps.
-  return CommandLine::ForCurrentProcess()->HasSwitch(
-      switches::kEnablePhantomTabs) &&
-      browser_defaults::kPinnedTabsActLikeApps &&
-      IsTabPinned(index);
+  return browser_defaults::kPinnedTabsActLikeApps && IsTabPinned(index);
 }
 
 bool TabStripModel::IsPhantomTab(int index) const {