Disable create application shortcuts for internal pages

- In fact, we only allow scheme file, ftp, http and https to have
  shortcuts;
- Removed GetFavIcon().isNull() for checking since we are using url
  scheme now;
- Add a CommandCreateAppShortcut to BrowserTest per Ben's comments;

BUG=26743
TEST=Verify create application shortcuts menu item is disabled for all
internal pages including but not limited to new tab page, history,
downloads, view-source etc.

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

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

4 files changed, 81 insertions, 3 deletions

diff --git a/chrome/browser/browser.cc b/chrome/browser/browser.cc
index 80fe2429..c13c6c9 100644
--- a/chrome/browser/browser.cc
+++ b/chrome/browser/browser.cc
@@ -51,6 +51,7 @@
 #include "chrome/browser/tab_contents/tab_contents.h"
 #include "chrome/browser/tab_contents/tab_contents_view.h"
 #include "chrome/browser/window_sizer.h"
+#include "chrome/browser/web_applications/web_app.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/extension.h"
@@ -1156,7 +1157,7 @@ void Browser::OpenCreateShortcutsDialog() {
   UserMetrics::RecordAction("CreateShortcut", profile_);
 #if defined(OS_WIN) || defined(OS_LINUX)
   TabContents* current_tab = GetSelectedTabContents();
-  DCHECK(current_tab && current_tab->FavIconIsValid()) <<
+  DCHECK(current_tab && web_app::IsValidUrl(current_tab->GetURL())) <<
       "Menu item should be disabled.";
 
   NavigationEntry* entry = current_tab->controller().GetLastCommittedEntry();
@@ -2544,7 +2545,7 @@ void Browser::UpdateCommandsForTabState() {
 
   // Show various bits of UI
   command_updater_.UpdateCommandEnabled(IDC_CREATE_SHORTCUTS,
-      !current_tab->GetFavIcon().isNull());
+      web_app::IsValidUrl(current_tab->GetURL()));
 }
 
 void Browser::UpdateStopGoState(bool is_loading, bool force) {
@@ -2658,7 +2659,7 @@ void Browser::ProcessPendingUIUpdates() {
       if (flags & (TabContents::INVALIDATE_TAB |
                    TabContents::INVALIDATE_TITLE)) {
         command_updater_.UpdateCommandEnabled(IDC_CREATE_SHORTCUTS,
-            !contents->GetFavIcon().isNull());
+            web_app::IsValidUrl(contents->GetURL()));
         window_->UpdateTitleBar();
       }
     }
diff --git a/chrome/browser/browser_browsertest.cc b/chrome/browser/browser_browsertest.cc
index 762be21..2ea76c2 100644
--- a/chrome/browser/browser_browsertest.cc
+++ b/chrome/browser/browser_browsertest.cc
@@ -6,11 +6,13 @@
 
 #include "app/l10n_util.h"
 #include "base/sys_info.h"
+#include "chrome/app/chrome_dll_resource.h"
 #include "chrome/browser/app_modal_dialog.h"
 #include "chrome/browser/browser.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/renderer_host/render_process_host.h"
 #include "chrome/browser/tab_contents/tab_contents.h"
+#include "chrome/common/url_constants.h"
 #include "chrome/common/page_transition_types.h"
 #include "chrome/test/in_process_browser_test.h"
 #include "chrome/test/ui_test_utils.h"
@@ -196,3 +198,58 @@ IN_PROC_BROWSER_TEST_F(BrowserTest, RenderIdleTime) {
     EXPECT_TRUE(browser_td >= renderer_td);
   }
 }
+
+// Test IDC_CREATE_SHORTCUTS command is enabled for url scheme file, ftp, http
+// and https and disabled for chrome://, about:// etc.
+IN_PROC_BROWSER_TEST_F(BrowserTest, CommandCreateAppShortcut) {
+  static const wchar_t kDocRoot[] = L"chrome/test/data";
+
+  CommandUpdater* command_updater = browser()->command_updater();
+
+  // Urls that are okay to have shortcuts.
+  GURL file_url(ui_test_utils::GetTestUrl(L".", L"empty.html"));
+  ASSERT_TRUE(file_url.SchemeIs(chrome::kFileScheme));
+  ui_test_utils::NavigateToURL(browser(), file_url);
+  EXPECT_TRUE(command_updater->IsCommandEnabled(IDC_CREATE_SHORTCUTS));
+
+  scoped_refptr<FTPTestServer> ftp_server(
+        FTPTestServer::CreateServer(kDocRoot));
+  ASSERT_TRUE(NULL != ftp_server.get());
+  GURL ftp_url(ftp_server->TestServerPage(""));
+  ASSERT_TRUE(ftp_url.SchemeIs(chrome::kFtpScheme));
+  ui_test_utils::NavigateToURL(browser(), ftp_url);
+  EXPECT_TRUE(command_updater->IsCommandEnabled(IDC_CREATE_SHORTCUTS));
+
+  scoped_refptr<HTTPTestServer> http_server(
+        HTTPTestServer::CreateServer(kDocRoot, NULL));
+  ASSERT_TRUE(NULL != http_server.get());
+  GURL http_url(http_server->TestServerPage(""));
+  ASSERT_TRUE(http_url.SchemeIs(chrome::kHttpScheme));
+  ui_test_utils::NavigateToURL(browser(), http_url);
+  EXPECT_TRUE(command_updater->IsCommandEnabled(IDC_CREATE_SHORTCUTS));
+
+  scoped_refptr<HTTPSTestServer> https_server(
+        HTTPSTestServer::CreateGoodServer(kDocRoot));
+  ASSERT_TRUE(NULL != https_server.get());
+  GURL https_url(https_server->TestServerPage("/"));
+  ASSERT_TRUE(https_url.SchemeIs(chrome::kHttpsScheme));
+  ui_test_utils::NavigateToURL(browser(), https_url);
+  EXPECT_TRUE(command_updater->IsCommandEnabled(IDC_CREATE_SHORTCUTS));
+
+  // Urls that should not have shortcuts.
+  GURL new_tab_url(chrome::kChromeUINewTabURL);
+  ui_test_utils::NavigateToURL(browser(), new_tab_url);
+  EXPECT_FALSE(command_updater->IsCommandEnabled(IDC_CREATE_SHORTCUTS));
+
+  GURL history_url(chrome::kChromeUIHistoryURL);
+  ui_test_utils::NavigateToURL(browser(), history_url);
+  EXPECT_FALSE(command_updater->IsCommandEnabled(IDC_CREATE_SHORTCUTS));
+
+  GURL downloads_url(chrome::kChromeUIDownloadsURL);
+  ui_test_utils::NavigateToURL(browser(), downloads_url);
+  EXPECT_FALSE(command_updater->IsCommandEnabled(IDC_CREATE_SHORTCUTS));
+
+  GURL blank_url(chrome::kAboutBlankURL);
+  ui_test_utils::NavigateToURL(browser(), blank_url);
+  EXPECT_FALSE(command_updater->IsCommandEnabled(IDC_CREATE_SHORTCUTS));
+}
diff --git a/chrome/browser/web_applications/web_app.cc b/chrome/browser/web_applications/web_app.cc
index 1c55c4b..ebf8387 100644
--- a/chrome/browser/web_applications/web_app.cc
+++ b/chrome/browser/web_applications/web_app.cc
@@ -12,6 +12,7 @@
 #include "chrome/browser/browser_process.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_plugin_util.h"
+#include "chrome/common/url_constants.h"
 
 #if defined(OS_WIN)
 #include "app/gfx/icon_util.h"
@@ -298,4 +299,20 @@ void CreateShortcut(
       new CreateShortcutTask(data_dir, shortcut_info, callback));
 }
 
+bool IsValidUrl(const GURL& url) {
+  static const char* const kValidUrlSchemes[] = {
+      chrome::kFileScheme,
+      chrome::kFtpScheme,
+      chrome::kHttpScheme,
+      chrome::kHttpsScheme,
+  };
+
+  for (size_t i = 0; i < arraysize(kValidUrlSchemes); ++i) {
+    if (url.SchemeIs(kValidUrlSchemes[i]))
+      return true;
+  }
+
+  return false;
+}
+
 };  // namespace web_app
diff --git a/chrome/browser/web_applications/web_app.h b/chrome/browser/web_applications/web_app.h
index 3e3c9b0..f6c9612 100644
--- a/chrome/browser/web_applications/web_app.h
+++ b/chrome/browser/web_applications/web_app.h
@@ -25,6 +25,9 @@ void CreateShortcut(
     const ShellIntegration::ShortcutInfo& shortcut_info,
     CreateShortcutCallback* callback);
 
+// Returns true if given url is a valid web app url.
+bool IsValidUrl(const GURL& url);
+
 };  // namespace web_app
 
 #endif  // CHROME_BROWSER_WEB_APPLICATIONS_WEB_APP_H_