"Redirecting URLs to Packaged Apps" implementation: revised

apps/url_redirector.cc

+// Copyright 2013 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 "apps/url_redirector.h"
+
+#include "apps/launcher.h"
+#include "base/command_line.h"
+#include "base/logging.h"
+#include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/extensions/extension_system.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/common/extensions/api/url_handlers/url_handlers_parser.h"
+#include "chrome/common/extensions/extension_messages.h"
+#include "chrome/common/extensions/extension_set.h"
+#include "extensions/common/switches.h"
+#include "ipc/ipc_message.h"
+#include "ipc/ipc_message_macros.h"
+
+DEFINE_WEB_CONTENTS_USER_DATA_KEY(apps::UrlRedirector);
+
+namespace apps {
+
+UrlRedirector::UrlRedirector(content::WebContents* web_contents)
+    : content::WebContentsObserver(web_contents),
+      profile_(Profile::FromBrowserContext(web_contents->GetBrowserContext())) {
+  // We don't want to ever send visited URLs to apps in incognito. Technically,
+  // apps are not supported in incognito, but explicitly check for that.
+  // See b/240879, which tracks incognito support for v2 apps.
+  CHECK(!profile_->IsOffTheRecord());
+}
+
+UrlRedirector::~UrlRedirector() {
+}
+
+// static
+bool UrlRedirector::MaybeLaunchAppWithUrl(

[benwells, 2013/09/03 01:10:56]

This is called from browser_navigator, and maybe other places. it is static so
the CHECK above isn't guaranteed to catch all uses.

What checks are in place to make sure it isn't called with an incognito profile?
Maybe it should just return false if it is?

[sergeygs, 2013/09/03 06:15:24]

Done, return false in incognito now. This is just an extra precaution, since
there can be no apps in incognito currently, hence nothing to send the URL to in
any case.

+    Profile* profile,
+    const GURL& url,
+    const GURL& referrer_url) {
+  // This should never happen unless the switch is specified; if it's not,
+  // originators of the invocation chain should not call us
+  // (ChromeContentRenderClient::HandleNavigation, chrome::Navigate).
+  // TODO(sergeygs): Remove this and update includes once url_handlers are
+  // moved out of experimental.
+  CHECK(CommandLine::ForCurrentProcess()->HasSwitch(
+          extensions::switches::kEnableExperimentalExtensionApis));
+
+  ExtensionService* service =
+      extensions::ExtensionSystem::Get(profile)->extension_service();
+  if (!service)
+    return false;
+
+  for (ExtensionSet::const_iterator iter = service->extensions()->begin();
+       iter != service->extensions()->end();
+       ++iter) {
+    const extensions::UrlHandlerInfo* handler =
+        extensions::UrlHandlers::FindMatchingUrlHandler(*iter, url);
+    if (handler) {
+      LaunchPlatformAppWithUrl(profile, *iter, handler->id, url, referrer_url);
+      return true;
+    }
+  }
+
+  return false;
+}
+
+bool UrlRedirector::OnMessageReceived(const IPC::Message& message) {
+  bool handled = true;
+  IPC_BEGIN_MESSAGE_MAP(UrlRedirector, message)
+    IPC_MESSAGE_HANDLER(ExtensionHostMsg_RedirectUrl,
+                        LaunchAppWithUrl)
+    IPC_MESSAGE_UNHANDLED(handled = false)
+  IPC_END_MESSAGE_MAP()
+  return handled;
+}
+
+void UrlRedirector::LaunchAppWithUrl(const GURL& url,
+                                     const GURL& referrer_url) const {
+  MaybeLaunchAppWithUrl(profile_, url, referrer_url);

[benwells, 2013/09/03 01:10:56]

Just going by the names, I'm assuming if LaunchAppWithUrl is called, it is not a
Maybe. Can you dcheck that? i.e.
bool launched = Maybe....
DCHECK(launched)

[sergeygs, 2013/09/03 06:15:24]

Done. Indeed, the sender should send the IPC only after making sure an app to
handle the URL exists. I was just a little unsure that the app could not
possibly disappear between sending and receiving the IPC, so left a little
backdoor here. Do you think that's not possible (app disappearing)?

+}
+
+}  // namespace apps