Implementing TabManager extensions API Discard Function.

chrome/browser/extensions/api/tabs/tabs_test.cc

Index: chrome/browser/extensions/api/tabs/tabs_test.cc
diff --git a/chrome/browser/extensions/api/tabs/tabs_test.cc b/chrome/browser/extensions/api/tabs/tabs_test.cc
index 1bc1eac1ae248a56c861da8de03788acb98ff51c..f6b6a602997f8ef2013716b7681bfb905ec7a304 100644
--- a/chrome/browser/extensions/api/tabs/tabs_test.cc
+++ b/chrome/browser/extensions/api/tabs/tabs_test.cc
@@ -37,6 +37,7 @@
 #include "chrome/test/base/ui_test_utils.h"
 #include "components/prefs/pref_service.h"
 #include "content/public/browser/browser_context.h"
+#include "content/public/browser/notification_service.h"
 #include "content/public/browser/storage_partition.h"
 #include "content/public/common/page_zoom.h"
 #include "content/public/common/url_constants.h"
@@ -1439,6 +1440,123 @@ IN_PROC_BROWSER_TEST_F(ExtensionTabsTest, DiscardedProperty) {
   }
 }
 
+// Tests chrome.tabs.discard(tabId).
+IN_PROC_BROWSER_TEST_F(ExtensionTabsTest, DiscardWithId) {
+  // Create an aditional tab.
+  ui_test_utils::NavigateToURLWithDisposition(
+      browser(), GURL(url::kAboutBlankURL), NEW_BACKGROUND_TAB,
+      ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
+  content::WebContents* web_contents =
+      browser()->tab_strip_model()->GetWebContentsAt(1);
+
+  // Set up the function with an extension.
+  scoped_refptr<const Extension> extension = test_util::CreateEmptyExtension();
+  scoped_refptr<TabsDiscardFunction> discard(new TabsDiscardFunction());
+  discard->set_extension(extension.get());
+
+  // Run function passing the tab id as argument.
+  int tab_id = ExtensionTabUtil::GetTabId(web_contents);
+  std::unique_ptr<base::DictionaryValue> result(
+      utils::ToDictionary(utils::RunFunctionAndReturnSingleResult(
+          discard.get(), base::StringPrintf("[%u]", tab_id), browser())));
+
+  // Confirms that TabManager sees the tab as discarded.
+  memory::TabManager* tab_manager = g_browser_process->GetTabManager();
+  web_contents = browser()->tab_strip_model()->GetWebContentsAt(1);
+  EXPECT_TRUE(tab_manager->IsTabDiscarded(web_contents));
+
+  // Make sure the returned tab is the one discarded and
+  // its discarded state is correct.
+  tab_id = ExtensionTabUtil::GetTabId(web_contents);
+  EXPECT_EQ(tab_id, api_test_utils::GetInteger(result.get(), "id"));
+  EXPECT_TRUE(api_test_utils::GetBoolean(result.get(), "discarded"));
+}
+
+// Tests chrome.tabs.discard(invalidId).
+IN_PROC_BROWSER_TEST_F(ExtensionTabsTest, DiscardWithInvalidId) {
+  // Create an aditional tab.
+  ui_test_utils::NavigateToURLWithDisposition(
+      browser(), GURL(url::kAboutBlankURL), NEW_BACKGROUND_TAB,
+      ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
+
+  // Set up the function with an extension.
+  scoped_refptr<const Extension> extension = test_util::CreateEmptyExtension();
+  scoped_refptr<TabsDiscardFunction> discard(new TabsDiscardFunction());
+  discard->set_extension(extension.get());
+
+  // Run function passing an invalid id as argument.
+  int tab_invalid_id = ExtensionTabUtil::GetTabId(
+                           browser()->tab_strip_model()->GetWebContentsAt(1)) +
+                       2;

[Devlin, 2016/07/25 15:48:55]

This seems fragile.  We should assert that there isn't a tab with this id.

[Anderson Silva, 2016/07/25 17:42:00]

Done.

+  std::string error = utils::RunFunctionAndReturnError(
+      discard.get(), base::StringPrintf("[%u]", tab_invalid_id), browser());
+
+  // Discarded state should still be false as no tab was discarded.
+  EXPECT_FALSE(g_browser_process->GetTabManager()->IsTabDiscarded(
+      browser()->tab_strip_model()->GetWebContentsAt(1)));
+
+  // Check error message.
+  EXPECT_TRUE(base::MatchPattern(error, keys::kTabNotFoundError));
+}
+
+// Tests chrome.tabs.discard().
+IN_PROC_BROWSER_TEST_F(ExtensionTabsTest, DiscardWithoutId) {
+  // Create an aditional tab.
+  ui_test_utils::NavigateToURLWithDisposition(
+      browser(), GURL(url::kAboutBlankURL), NEW_BACKGROUND_TAB,
+      ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
+  content::WebContents* web_contents =
+      browser()->tab_strip_model()->GetWebContentsAt(1);
+
+  // Set up the function with an extension.
+  scoped_refptr<const Extension> extension = test_util::CreateEmptyExtension();
+  scoped_refptr<TabsDiscardFunction> discard(new TabsDiscardFunction());
+  discard->set_extension(extension.get());
+
+  // Disable protection time to discard the tab without passing id.
+  memory::TabManager* tab_manager = g_browser_process->GetTabManager();
+  tab_manager->set_minimum_protection_time_for_tests(
+      base::TimeDelta::FromSeconds(0));
+
+  // Run without passing an id.
+  std::unique_ptr<base::DictionaryValue> result(utils::ToDictionary(
+      utils::RunFunctionAndReturnSingleResult(discard.get(), "[]", browser())));
+
+  // Confirms that TabManager sees the tab as discarded.
+  web_contents = browser()->tab_strip_model()->GetWebContentsAt(1);
+  EXPECT_TRUE(tab_manager->IsTabDiscarded(web_contents));
+
+  // Make sure the returned tab is the one discarded and
+  // its discarded state is correct.
+  EXPECT_EQ(ExtensionTabUtil::GetTabId(web_contents),
+            api_test_utils::GetInteger(result.get(), "id"));
+  EXPECT_TRUE(api_test_utils::GetBoolean(result.get(), "discarded"));
+}
+
+// Tests chrome.tabs.discard() without disabling protection time.
+IN_PROC_BROWSER_TEST_F(ExtensionTabsTest, DiscardNoTabProtection) {
+  // Create an aditional tab.
+  ui_test_utils::NavigateToURLWithDisposition(
+      browser(), GURL(url::kAboutBlankURL), NEW_BACKGROUND_TAB,
+      ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
+
+  // Set up the function with an extension.
+  scoped_refptr<const Extension> extension = test_util::CreateEmptyExtension();
+  scoped_refptr<TabsDiscardFunction> discard(new TabsDiscardFunction());
+  discard->set_extension(extension.get());
+
+  // Run without passing an id. In this case the chrome.tabs.discard()
+  // function execution exits with no errors but the tab couldn't be

[Devlin, 2016/07/25 15:48:55]

This still seems like it *should* be an error to me.  If I'm an extension
developer, what am I supposed to do with this information?  I have no idea why
the tab didn't correctly discard, only that it didn't.

[Anderson Silva, 2016/07/25 17:42:00]

Yes, this is by design. It's more of a "best effort", as we'll try to discard
when asked to, but we don't guarantee that it will succeed.

Depending on whether a tab is supplied or not:
- If not tab is supplied,  this will fail if there are no discardable tabs
currently. This depends on multiple signals (active, playing audio...) that we
are not exposing.
- If a tab is supplied, discarding will fail if the tab is active or if it's
already discarded.

I changed the description of the tabId parameter to better reflect this.

Let me know what you think.

[Devlin, 2016/07/25 18:05:44]

I'm fine with the best-effort approach and with not always being able to discard
a tab, but I think we should let the developer know why something failed.  Was
nothing discarded because there were no discardable tabs?  Because it was too
soon to discard the specified tab?  Because the tab was not discardable?  These
are things that developers may need to rely on, and right now we're being
totally opaque about them.  Is there a reason not to provide the reason that the
call failed?

+  // discarded because of protection time.
+  // Then, the result should be undefined.
+  std::unique_ptr<base::Value> result(
+      utils::RunFunctionAndReturnSingleResult(discard.get(), "[]", browser()));
+
+  EXPECT_FALSE(g_browser_process->GetTabManager()->IsTabDiscarded(
+      browser()->tab_strip_model()->GetWebContentsAt(1)));
+  EXPECT_FALSE(result);
+}
+
 // Tester class for the tabs.zoom* api functions.
 class ExtensionTabsZoomTest : public ExtensionTabsTest {
  public: