Bookmark apps created from extension pages no longer show location bar.

Fixed a regression introduced by r537588 which showed a location bar on
all non-https pages. Extension pages are secure, but have scheme
chrome-extension, so did not pass this test. Now chrome-extension pages
are special-cased to be allowed without a location bar.

Bug: 828233
Change-Id: Ic792007ddb8d8d29ab0dc0b82500e4833f6f2c7c
Reviewed-on: https://chromium-review.googlesource.com/991932
Commit-Queue: Matt Giuca <mgiuca@chromium.org>
Reviewed-by: Ben Wells <benwells@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548372}

chrome/browser/ui/extensions/hosted_app_browser_controller.cc

@@ -27,6 +27,7 @@
 #include "content/public/common/renderer_preferences.h"
 #include "content/public/common/web_preferences.h"
 #include "extensions/browser/extension_registry.h"
+#include "extensions/common/constants.h"
 #include "extensions/common/extension.h"
 #include "ui/gfx/favicon_size.h"
 #include "ui/gfx/image/image_skia.h"
@@ -36,12 +37,27 @@ namespace extensions {
 
 namespace {
 
-// Returns true if |page_url| is both secure (https) and on the same origin as
-// |app_url|. Note that even if |app_url| is http, this still returns true as
+// Returns the scheme that page URLs should be, in order to be considered
+// "secure", for an app URL of scheme |scheme|.
+//
+// All pages (even if the app was created with scheme "http") are expected to
+// have scheme "https", since "http" is not secure. As a special exception,
+// pages for "chrome-extension" apps are expected to have the same scheme (since
+// that scheme is secure).
+base::StringPiece ExpectedSchemeForApp(base::StringPiece scheme) {
+  if (scheme == kExtensionScheme)
+    return scheme;
+
+  return url::kHttpsScheme;
+}
+
+// Returns true if |page_url| is both secure (not http) and on the same origin
+// as |app_url|. Note that even if |app_url| is http, this still returns true as
 // long as |page_url| is https.
 bool IsSameOriginAndSecure(const GURL& app_url, const GURL& page_url) {
   const std::string www("www.");
-  return page_url.scheme_piece() == url::kHttpsScheme &&
+  return ExpectedSchemeForApp(app_url.scheme_piece()) ==
+             page_url.scheme_piece() &&
          (app_url.host_piece() == page_url.host_piece() ||
           www + app_url.host() == page_url.host_piece()) &&
          app_url.port() == page_url.port();

chrome/browser/ui/extensions/hosted_app_browsertest.cc

@@ -517,6 +517,47 @@ IN_PROC_BROWSER_TEST_P(HostedAppTest, MAYBE_ShouldShowLocationBarForHTTPSApp) {
       app_browser_, "http://www.example.com/blah", true);
 }
 
+// Check that location bar is not shown for apps hosted within extensions pages.
+// This simulates a case where the user has manually navigated to a page hosted
+// within an extension, then added it as a bookmark app.
+// Regression test for https://crbug.com/828233.
+IN_PROC_BROWSER_TEST_P(HostedAppTest, ShouldShowLocationBarForExtensionPage) {
+  // Test only applies for bookmark apps.
+  if (app_type() != AppType::BOOKMARK_APP)
+    return;
+
+  // Note: This involves the creation of *two* extensions: The first is a
+  // regular (non-app) extension with a popup page. The second is a bookmark app
+  // created from the popup page URL (allowing the extension's popup page to be
+  // loaded in a window).
+
+  // Install the extension that has the popup page.
+  ASSERT_TRUE(LoadExtension(
+      test_data_dir_.AppendASCII("ui").AppendASCII("browser_action_popup")));
+  base::RunLoop().RunUntilIdle();  // Ensure the extension is fully loaded.
+
+  // Install the bookmark app that links to the extension's popup page.
+  GURL popup_url("chrome-extension://" + last_loaded_extension_id() +
+                 "/popup.html");
+  // TODO(mgiuca): Abstract this logic to share code with InstallPWA (which does
+  // almost the same thing, but also sets a scope).
+  WebApplicationInfo web_app_info;
+  web_app_info.app_url = popup_url;
+  app_ = InstallBookmarkApp(web_app_info);
+
+  ui_test_utils::UrlLoadObserver url_observer(
+      popup_url, content::NotificationService::AllSources());
+  app_browser_ = LaunchAppBrowser(app_);
+  url_observer.Wait();
+
+  CHECK(app_browser_);
+  CHECK(app_browser_ != browser());
+
+  // Navigate to the app's launch page; the location bar should not be visible,
+  // because extensions pages are secure.
+  NavigateAndCheckForLocationBar(app_browser_, popup_url.spec(), false);
+}
+
 // Check that the location bar is shown correctly for apps that specify start
 // URLs without the 'www.' prefix.
 IN_PROC_BROWSER_TEST_P(HostedAppTest, ShouldShowLocationBarForAppWithoutWWW) {