Hosted/Bookmark apps now show the location bar for http connections.

HostedAppTest now use https for all tests other than http-specific ones
(otherwise, all tests would show a location bar, and we wouldn't be
properly testing any of the other behaviour).

Bug: 811158
Change-Id: I0fe220055fb6574c3950f65971e06feadaeabcd9
Reviewed-on: https://chromium-review.googlesource.com/913191Reviewed-by: Ben Wells <benwells@chromium.org>
Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Commit-Queue: Matt Giuca <mgiuca@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537588}

chrome/browser/ui/extensions/hosted_app_browser_controller.cc

@@ -33,10 +33,12 @@ namespace extensions {
 
 namespace {
 
-bool IsSameOriginOrMoreSecure(const GURL& app_url, const GURL& page_url) {
+// 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
+// long as |page_url| is https.
+bool IsSameOriginAndSecure(const GURL& app_url, const GURL& page_url) {
   const std::string www("www.");
-  return (app_url.scheme_piece() == page_url.scheme_piece() ||
-          page_url.scheme_piece() == url::kHttpsScheme) &&
+  return page_url.scheme_piece() == url::kHttpsScheme &&
          (app_url.host_piece() == page_url.host_piece() ||
           www + app_url.host() == page_url.host_piece()) &&
          app_url.port() == page_url.port();
@@ -133,10 +135,9 @@ bool HostedAppBrowserController::ShouldShowLocationBar() const {
   }
 
   GURL launch_url = AppLaunchInfo::GetLaunchWebURL(extension);
-  return !(IsSameOriginOrMoreSecure(launch_url,
-                                    web_contents->GetVisibleURL()) &&
-           IsSameOriginOrMoreSecure(launch_url,
-                                    web_contents->GetLastCommittedURL()));
+  return !IsSameOriginAndSecure(launch_url, web_contents->GetVisibleURL()) ||
+         !IsSameOriginAndSecure(launch_url,
+                                web_contents->GetLastCommittedURL());
 }
 
 void HostedAppBrowserController::UpdateLocationBarVisibility(

chrome/browser/ui/extensions/hosted_app_browsertest.cc

@@ -248,20 +248,20 @@ IN_PROC_BROWSER_TEST_P(HostedAppTest, CtrlClickLink) {
 
 // Check that the location bar is shown correctly.
 IN_PROC_BROWSER_TEST_P(HostedAppTest, ShouldShowLocationBar) {
-  SetupApp("app");
+  SetupApp("https_app");
 
   // Navigate to the app's launch page; the location bar should be hidden.
-  NavigateAndCheckForLocationBar(
-      app_browser_, "http://www.example.com/empty.html", false);
+  NavigateAndCheckForLocationBar(app_browser_,
+                                 "https://www.example.com/empty.html", false);
 
   // Navigate to another page on the same origin; the location bar should still
   // hidden.
-  NavigateAndCheckForLocationBar(
-      app_browser_, "http://www.example.com/blah", false);
+  NavigateAndCheckForLocationBar(app_browser_, "https://www.example.com/blah",
+                                 false);
 
   // Navigate to different origin; the location bar should now be visible.
-  NavigateAndCheckForLocationBar(
-      app_browser_, "http://www.foo.com/blah", true);
+  NavigateAndCheckForLocationBar(app_browser_, "https://www.foo.com/blah",
+                                 true);
 }
 
 // Check that the location bar is shown correctly for HTTP apps when they
@@ -269,12 +269,13 @@ IN_PROC_BROWSER_TEST_P(HostedAppTest, ShouldShowLocationBar) {
 IN_PROC_BROWSER_TEST_P(HostedAppTest, ShouldShowLocationBarForHTTPApp) {
   SetupApp("app");
 
-  // Navigate to the app's launch page; the location bar should be hidden.
-  NavigateAndCheckForLocationBar(
-      app_browser_, "http://www.example.com/empty.html", false);
+  // Navigate to the app's launch page; the location bar should be visible, even
+  // though it exactly matches the site, because it is not secure.
+  NavigateAndCheckForLocationBar(app_browser_,
+                                 "http://www.example.com/empty.html", true);
 
   // Navigate to the https version of the site; the location bar should
-  // be hidden.
+  // be hidden, as it is a more secure version of the site.
   NavigateAndCheckForLocationBar(
       app_browser_, "https://www.example.com/blah", false);
 }
@@ -289,8 +290,7 @@ IN_PROC_BROWSER_TEST_P(HostedAppTest, ShouldShowLocationBarForHTTPSApp) {
       app_browser_, "https://www.example.com/empty.html", false);
 
   // Navigate to the http version of the site; the location bar should
-  // be visible for the https version as it is now on a less secure version
-  // of its host.
+  // be visible for the https version as it is not secure.
   NavigateAndCheckForLocationBar(
       app_browser_, "http://www.example.com/blah", true);
 }
@@ -298,20 +298,20 @@ IN_PROC_BROWSER_TEST_P(HostedAppTest, ShouldShowLocationBarForHTTPSApp) {
 // 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) {
-  SetupApp("app_no_www");
+  SetupApp("https_app_no_www");
 
   // Navigate to the app's launch page; the location bar should be hidden.
-  NavigateAndCheckForLocationBar(
-      app_browser_, "http://example.com/empty.html", false);
+  NavigateAndCheckForLocationBar(app_browser_, "https://example.com/empty.html",
+                                 false);
 
   // Navigate to the app's launch page with the 'www.' prefis; the location bar
   // should be hidden.
-  NavigateAndCheckForLocationBar(
-      app_browser_, "http://www.example.com/empty.html", false);
+  NavigateAndCheckForLocationBar(app_browser_,
+                                 "https://www.example.com/empty.html", false);
 
   // Navigate to different origin; the location bar should now be visible.
-  NavigateAndCheckForLocationBar(
-      app_browser_, "http://www.foo.com/blah", true);
+  NavigateAndCheckForLocationBar(app_browser_, "https://www.foo.com/blah",
+                                 true);
 }
 
 // Check that a subframe on a regular web page can navigate to a URL that

chrome/test/data/extensions/app_no_www/manifest.json → chrome/test/data/extensions/https_app_no_www/manifest.json

@@ -4,7 +4,7 @@
   "manifest_version": 2,
   "app": {
     "launch": {
-      "web_url": "http://example.com/"
+      "web_url": "https://example.com/"
     }
   }
 }