"Redirecting URLs to Packaged Apps" implementation: revised

chrome/renderer/extensions/dispatcher.cc

Index: chrome/renderer/extensions/dispatcher.cc
diff --git a/chrome/renderer/extensions/dispatcher.cc b/chrome/renderer/extensions/dispatcher.cc
index 7ed9437eb12539c5f71807233a45a6d0d2980ca6..5149446ac6935feef61b4666100a18d3cb457f69 100644
--- a/chrome/renderer/extensions/dispatcher.cc
+++ b/chrome/renderer/extensions/dispatcher.cc
@@ -19,6 +19,7 @@
 #include "chrome/common/chrome_version_info.h"
 #include "chrome/common/extensions/api/extension_api.h"
 #include "chrome/common/extensions/api/runtime.h"
+#include "chrome/common/extensions/api/url_handlers/url_handlers_parser.h"
 #include "chrome/common/extensions/background_info.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_constants.h"
@@ -94,6 +95,7 @@
 #include "third_party/WebKit/public/web/WebRuntimeFeatures.h"
 #include "third_party/WebKit/public/web/WebScopedUserGesture.h"
 #include "third_party/WebKit/public/web/WebSecurityPolicy.h"
+#include "third_party/WebKit/public/web/WebUserGestureIndicator.h"
 #include "third_party/WebKit/public/web/WebView.h"
 #include "ui/base/layout.h"
 #include "ui/base/resource/resource_bundle.h"
@@ -1568,4 +1570,74 @@ void Dispatcher::InvokeModuleSystemMethod(
   }
 }
 
+bool Dispatcher::MaybeRedirectUrlToApp(

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

It doesn't feel good to have this code here. Did you log a bug / add a TODO to
move it to chrome/browser/apps? Feels like we need an apps_dispatcher in chrome.

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

Obsolete.

+    WebKit::WebFrame* frame,
+    const WebKit::WebURLRequest& request,
+    WebKit::WebNavigationType type,
+    WebKit::WebNavigationPolicy default_policy) {
+  // Limit redirection to top-level frames only.
+  if (frame != frame->top())
+    return false;
+
+  // Exclude Ignore and Download.
+  const bool policy_ok =
+      (default_policy == WebKit::WebNavigationPolicyCurrentTab) ||
+      (default_policy == WebKit::WebNavigationPolicyNewBackgroundTab) ||
+      (default_policy == WebKit::WebNavigationPolicyNewForegroundTab) ||
+      (default_policy == WebKit::WebNavigationPolicyNewWindow) ||
+      (default_policy == WebKit::WebNavigationPolicyNewPopup);
+  if (!policy_ok)
+    return false;
+
+  bool navigation_type_ok = false;
+  if (IsWithinPlatformApp()) {

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

I didn't think you could navigate within platform app processes like this. It's
probably a bug if you can?

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

To answer the question: this is ultimately called from WebKit, so those
navigation filters haven't been applied yet when we reach here. But yes, you are
not allowed to navigate via regular link clicks within apps (as the comment
below also mentions), and that case is explicitly handled here by omitting the
WebNavigationTypeLinkClicked case (compare to the 'else' for non-apps). So if a
link is clicked within an app, we return false, and that diagnostic gets printed
as usual. However, you are allowed to click links with target=_blank within
apps,  and those go through here too.

But anyway, this is now obsoleted by the latest patch.

+    // Exclude reloads (shouldn't happen anyway), back/forward, form submits,
+    // and regular links (without target='_blank'). The latter are not supposed
+    // to work in apps at all: an error message is normally printed to console
+    // if one is clicked, so fall back to that default behavior here.
+    navigation_type_ok =
+        (type == WebKit::WebNavigationTypeOther && frame->opener());
+  } else {
+    // Exclude reloads (shouldn't happen anyway), back/forward, form submits.
+    navigation_type_ok =
+        WebKit::WebUserGestureIndicator::isProcessingUserGesture() ||
+        (type == WebKit::WebNavigationTypeOther && frame->opener()) ||
+        (type == WebKit::WebNavigationTypeLinkClicked);
+  }
+  if (!navigation_type_ok)
+    return false;
+
+  const GURL url(request.url());
+  const GURL ref_url(request.httpHeaderField(WebString::fromUTF8("Referer")));
+
+  // Find out if there is an extension that can handle the URL.
+  bool some_extension_can_handle = false;
+  for (ExtensionSet::const_iterator it = extensions_.begin();
+       it != extensions_.end(); ++it) {
+    if (UrlHandlers::CanExtensionHandleUrl(*it, url)) {
+      some_extension_can_handle = true;
+      break;
+    }
+  }
+  if (!some_extension_can_handle)
+    return false;
+
+  // Get the correct RenderView (tab or app window) so that we can send a
+  // routed IPC to the correct listener.
+  content::RenderView* render_view = NULL;
+  if (IsWithinPlatformApp()) {

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

likewise

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

See the answer above.

+    render_view = content::RenderView::FromWebView(frame->opener()->view());
+  } else {
+    render_view = frame->opener() ?
+        content::RenderView::FromWebView(frame->opener()->view()) :
+        content::RenderView::FromWebView(frame->view());

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

does this mean that you redirect the opener frame rather than the opened one?

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

I used the opener frame's routing ID to reach the correct AppUrlRedirector. But
again: obsolete.

+  }
+  if (!render_view)
+    return false;
+
+  return RenderThread::Get()->Send(
+      new ExtensionHostMsg_RedirectUrl(
+          render_view->GetRoutingID(), url, ref_url));
+}
+
 }  // namespace extensions