"Redirecting URLs to Packaged Apps" implementation: revised

chrome/browser/ui/browser_navigator.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/browser_navigator.h"
 
 #include <algorithm>
 
 #include "base/command_line.h"
 #include "base/prefs/pref_service.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
+#include "chrome/browser/apps/app_url_redirector.h"
 #include "chrome/browser/browser_about_handler.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/extensions/extension_system.h"
 #include "chrome/browser/extensions/tab_helper.h"
 #include "chrome/browser/google/google_url_tracker.h"
 #include "chrome/browser/prefs/incognito_mode_prefs.h"
 #include "chrome/browser/prerender/prerender_manager.h"
 #include "chrome/browser/prerender/prerender_manager_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/tab_contents/tab_util.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_instant_controller.h"
 #include "chrome/browser/ui/browser_tab_contents.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/host_desktop.h"
 #include "chrome/browser/ui/omnibox/location_bar.h"
 #include "chrome/browser/ui/singleton_tabs.h"
 #include "chrome/browser/ui/status_bubble.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/web_applications/web_app.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "content/public/browser/browser_url_handler.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_view.h"
+#include "extensions/common/switches.h"

[not at google - send to devlin, 2013/09/04 15:37:21]

unused?

[sergeygs, 2013/09/05 09:19:03]

Done.

 
 #if defined(USE_AURA)
 #include "ui/aura/window.h"
 #endif
 
 using content::GlobalRequestID;
 using content::NavigationController;
 using content::WebContents;
 
 class BrowserNavigatorWebContentsAdoption {
@@ skipping 416 matching lines @@
 
 void Navigate(NavigateParams* params) {
   Browser* source_browser = params->browser;
   if (source_browser)
     params->initiating_profile = source_browser->profile();
   DCHECK(params->initiating_profile);
 
   if (!AdjustNavigateParamsForURL(params))
     return;
 
-  ExtensionService* service = params->initiating_profile->GetExtensionService();
-  if (service)
-    service->ShouldBlockUrlInBrowserTab(&params->url);
+  // See if the extension service wants to block this navigation or some
+  // platform app wants to intercept it.
+  extensions::ExtensionSystem* system =

[benwells, 2013/09/04 23:54:20]

It's unfortunate this code has to be here. This function is already complicated
and now it has another special case. Is there no other, more systematic way to
do this?

If this is here why do we need the other code to redirect to platform apps?

[sergeygs, 2013/09/05 09:19:03]

To answer the last question: because this deals only with top-level,
browser-initiated navigations. If you click a link which doesn't have
target='_blank' in a tab, WebKit does most of the job. And if you don't stop it
early enough, it will navigate.

But long story short: this is now obsoleted by the latest patch.

+      extensions::ExtensionSystem::Get(params->initiating_profile);
+  if (system) {
+    ExtensionService* service = system->extension_service();
+    if (service && service->ShouldBlockUrlInBrowserTab(&params->url)) {
+      // Proceed. The call just rewrites the URL to the error page: we still
+      // have to display it.
+    } else {
+      if (AppUrlRedirector::MaybeLaunchAppWithUrl(
+              params->initiating_profile,

[not at google - send to devlin, 2013/09/04 15:37:21]

Just grab the AppUrlRedirector from the initiating profile (using that
profile-keyed-service stuff) and call MaybeLaunchAppWithUrl on it. Making
MaybeLaunchAppWithUrl static is awkward and leads to unnecessary checks and
comments such as the one in that file.

However, you could add a static Get method to AppUrlRedirector if you wish
(similar to how ExtensionSystem has a Get method).

[benwells, 2013/09/04 23:54:20]

At the moment the redirector is a tab helper (or whatever they are called now),
not a PKS.

[not at google - send to devlin, 2013/09/05 00:01:52]

Yes, realised this later after chatting to Sergey. I suggested that these
actually be profile-scoped and that it should be responsible for listening to
WebContents creation and destruction (via WebContents::AddCreatedCallback). That
keeps the fact that these are Tab-scoped as an implementation detail, which is
what they should be.

[sergeygs, 2013/09/05 09:19:03]

Obsolete.

+              params->url,
+              params->referrer.url)) {
+        // An app has intercepted the navigation: cancel it here.
+        return;
+      }
+    }
+  }
 
   // The browser window may want to adjust the disposition.
   if (params->disposition == NEW_POPUP &&
       source_browser &&
       source_browser->window()) {
     params->disposition =
         source_browser->window()->GetDispositionForPopupBounds(
             params->window_bounds);
   }
 
@@ skipping 194 matching lines @@
   bool reverse_on_redirect = false;
   content::BrowserURLHandler::GetInstance()->RewriteURLIfNecessary(
       &rewritten_url, browser_context, &reverse_on_redirect);
 
   // Some URLs are mapped to uber subpages. Do not allow them in incognito.
   return !(rewritten_url.scheme() == chrome::kChromeUIScheme &&
            rewritten_url.host() == chrome::kChromeUIUberHost);
 }
 
 }  // namespace chrome