Allow platform apps to respond to Web Intents

chrome/browser/extensions/web_intent_callbacks.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 "base/bind.h"
+#include "base/stringprintf.h"
+#include "chrome/browser/extensions/web_intent_callbacks.h"
+#include "chrome/browser/profiles/profile_dependency_manager.h"
+#include "chrome/common/extensions/extension.h"
+#include "content/public/browser/web_contents.h"
+#include "content/public/browser/web_contents_observer.h"
+#include "content/public/browser/web_intents_dispatcher.h"
+#include "webkit/glue/web_intent_reply_data.h"

[benwells, 2012/08/13 08:27:29]

Are all these includes needed?

[thorogood, 2012/08/14 02:59:08]

I've removed a couple - thanks.

+
+namespace {
+
+// Get the key for use inside the internal pending WebIntentCallbacks map.
+std::string GetKey(const extensions::Extension* extension, int id) {
+  return StringPrintf("%s/%d", extension->id().c_str(), id);
+}
+
+}  // namespace
+
+namespace extensions {
+
+// SourceObserver is a subclass of WebContentsObserver that is instantiated
+// on RegisterCallback to wait for the source WebContents to be destroyed.
+// If it is destroyed, this automatically clears the callback from pending_.
+class WebIntentCallbacks::SourceObserver : content::WebContentsObserver {
+ public:
+  SourceObserver(content::WebContents* web_contents,
+                       WebIntentCallbacks* callbacks,

[benwells, 2012/08/13 08:27:29]

Indentation

[thorogood, 2012/08/14 02:59:08]

Done.

+                       const std::string key)
+      : content::WebContentsObserver(web_contents),
+        callbacks_(callbacks),
+        key_(key) {}
+  virtual ~SourceObserver() {}
+
+  // Implement WebContentsObserver
+  virtual void WebContentsDestroyed(content::WebContents* web_contents) {

[benwells, 2012/08/13 08:27:29]

Missing OVERRIDE

[thorogood, 2012/08/14 02:59:08]

Done - although I've copied this inner class mostly from
web_intent_picker_controller.cc, which has a class called SourceWindowObserver.
It does not have OVERRIDE.

+    callbacks_->pending_.erase(key_);
+    delete this;
+  }
+
+ private:
+  WebIntentCallbacks* callbacks_;
+  const std::string key_;
+};
+
+WebIntentCallbacks::WebIntentCallbacks() {}

[benwells, 2012/08/13 08:27:29]

Can you initialize last_id_ to zero here?

[thorogood, 2012/08/14 02:59:08]

Done.

+
+WebIntentCallbacks::~WebIntentCallbacks() {}
+
+// static
+WebIntentCallbacks* WebIntentCallbacks::Get(Profile* profile) {
+  return Factory::GetForProfile(profile);
+}
+
+int WebIntentCallbacks::RegisterCallback(
+    const Extension* extension,
+    content::WebIntentsDispatcher* dispatcher,
+    content::WebContents* source) {
+  int id = last_id_++;
+  std::string key = GetKey(extension, id);
+  pending_[key] = dispatcher;
+
+  // TODO(thorogood): We should also listen for extension suspend events, and
+  // clear the relevant callback (i.e., the handler of this intent)
+
+  // TODO(thorogood): Note that if the caller is itself a platform app, then

[benwells, 2012/08/13 08:27:29]

What is the TODO?

Is this behavior comparable to platform apps dispatching intents to normal
pages?

[thorogood, 2012/08/14 02:59:08]

So, I've rewritten the comment to make it clearer. Yes, it's comparable,
although the case you describe currently segfaults (I filed a bug yesterday:
b/141812). I believe it would have the same problem: the scope of the web intent
is bound to the lifetime of the source WebContents, not the source packaged app.

+  // we'll clear the callback when the window goes away: not the whole app.
+
+  if (source) {
+    // Listen to when the source WebContent is destroyed. This object is self-
+    // deleting on this event.
+    new SourceObserver(source, this, key);
+  } else {
+    LOG(WARNING) << "No source WebContents for WebIntent; "

[benwells, 2012/08/13 08:27:29]

Can this happen? If it should not be able to happen maybe we should have
DCHECK(source).

[thorogood, 2012/08/14 02:59:08]

I don't believe so. I originally believed it may occur if we invoked a Web
Intent via the command line on a file, but as you've pointed out that goes
through a different method in app_api.cc. (What about if we invoke it through
the command line without a file?)

I've made your suggestion for now though.

+                 << "unresponded intent will leak";
+  }
+  return id;
+}
+
+content::WebIntentsDispatcher* WebIntentCallbacks::RetrieveCallback(
+    const Extension* extension, int id) {
+  std::string key = GetKey(extension, id);
+
+  if (!pending_.count(key))
+    return NULL;
+
+  content::WebIntentsDispatcher* dispatcher = pending_[key];
+  pending_.erase(key);
+  return dispatcher;
+}
+
+///////////////////////////////////////////////////////////////////////////////
+// Factory boilerplate
+
+// static
+WebIntentCallbacks* WebIntentCallbacks::Factory::GetForProfile(
+    Profile* profile) {
+  return static_cast<WebIntentCallbacks*>(
+      GetInstance()->GetServiceForProfile(profile, true));
+}
+
+WebIntentCallbacks::Factory* WebIntentCallbacks::Factory::GetInstance() {
+  return Singleton<WebIntentCallbacks::Factory>::get();
+}
+
+WebIntentCallbacks::Factory::Factory()
+    : ProfileKeyedServiceFactory("WebIntentCallbacks",
+                                 ProfileDependencyManager::GetInstance()) {
+}
+
+WebIntentCallbacks::Factory::~Factory() {
+}
+
+ProfileKeyedService* WebIntentCallbacks::Factory::BuildServiceInstanceFor(
+    Profile* profile) const {
+  return new WebIntentCallbacks();
+}
+
+}  // namespace extensions