Pass content-type resources to web intents. Goes through download, then invokes the p…

chrome/browser/download/chrome_download_manager_delegate.cc

Index: chrome/browser/download/chrome_download_manager_delegate.cc
diff --git a/chrome/browser/download/chrome_download_manager_delegate.cc b/chrome/browser/download/chrome_download_manager_delegate.cc
index d9436600b45a4eb9e1fb897612ed368216095e33..6e95171de355a181d5fafbcadfc28e5db6e40162 100644
--- a/chrome/browser/download/chrome_download_manager_delegate.cc
+++ b/chrome/browser/download/chrome_download_manager_delegate.cc
@@ -12,6 +12,7 @@
 #include "base/rand_util.h"
 #include "base/stringprintf.h"
 #include "base/time.h"
+#include "base/utf_string_conversions.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/download/download_crx_util.h"
 #include "chrome/browser/download/download_extensions.h"
@@ -33,12 +34,18 @@
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/extensions/user_script.h"
 #include "chrome/common/pref_names.h"
+#include "content/browser/intents/internal_web_intents_dispatcher.h"
 #include "content/public/browser/download_item.h"
 #include "content/public/browser/download_manager.h"
 #include "content/public/browser/notification_source.h"
+#include "content/public/browser/resource_context.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/browser/web_intents_dispatcher.h"
 #include "grit/generated_resources.h"
 #include "ui/base/l10n/l10n_util.h"
+#include "webkit/blob/blob_data.h"
+#include "webkit/blob/blob_storage_controller.h"
+#include "webkit/glue/web_intent_data.h"
 
 using content::BrowserThread;
 using content::DownloadId;
@@ -72,6 +79,7 @@ ChromeDownloadManagerDelegate::ChromeDownloadManagerDelegate(Profile* profile)
 ChromeDownloadManagerDelegate::~ChromeDownloadManagerDelegate() {
 }
 
+// static
 bool ChromeDownloadManagerDelegate::IsExtensionDownload(
     const DownloadItem* item) {
   if (item->PromptUserForSaveLocation())
@@ -81,6 +89,24 @@ bool ChromeDownloadManagerDelegate::IsExtensionDownload(
       UserScript::IsURLUserScript(item->GetURL(), item->GetMimeType());
 }
 
+// static
+bool ChromeDownloadManagerDelegate::ShouldWebIntentsHandle(

[jam, 2012/03/13 20:05:01]

why is this stuff in chrome vs content? it seems generic to the web platform (as
opposed to the crx stuff, which is chrome specific)

[Greg Billock, 2012/03/13 20:53:56]

It could be defined more generically in DownloadManagerDelegate. My current take
on the policy I think we'll use is that using web intents for some downloaded
content types is a browser-specific policy decision. Tooling this back through
the content API to call TabContentsDelegate::WebIntentDispatch would require
putting some more tooling in the caller of ShouldOpenDownload. It might end up
looking pretty similar, though, i.e.

download_item_impl.cc:770

WebIntentsDispatcher* d = delegate_->ShouldOpenWithWebIntents(this);
if (d != NULL) {
  GetWebContents()->delegate->Dispatch(d);
  Completed();
}

Do you think that's a better insertion point? One thing I'm nervous about is
that these are all synchronous calls, and if we end up wanting a policy
requiring consulting the WebIntentsRegistry, we'll need to make this whole part
of the code use a continuation.

+    const DownloadItem& item) {
+  std::string mime_type = item.GetMimeType();
+  LOG(INFO) << "Testing " << mime_type << " for web intents";
+
+  // TODO: Check for explicit user-caused download?

[michaeln, 2012/03/12 21:16:12]

Should this also check for mimes for which there actually are registered
'intent' handlers? Seems like the "to webintent or not to webintent" decision
making needs to refinement?

[Greg Billock, 2012/03/13 17:56:10]

Yes, it needs refinement. Checking the registry for registered handlers is
probably in the future. The current mission is to support RCH-style pluggability
for a whitelist of types. We may need a policy decision on whether we should
handle those with web intents when we know there are available applications.

In other words, this is more of a dummy proof of concept than what's intended as
the actual policy.

On 2012/03/12 21:16:12, michaeln wrote:

+
+  if (mime_type == "application/rss+xml" ||
+      mime_type == "application/atom+xml" ||
+      mime_type == "application/ms-word" ||
+      mime_type == "appliation/pdf") {
+    return true;
+  }
+
+  return false;
+}
+
 void ChromeDownloadManagerDelegate::SetDownloadManager(DownloadManager* dm) {
   download_manager_ = dm;
   download_history_.reset(new DownloadHistory(profile_));
@@ -198,27 +224,80 @@ bool ChromeDownloadManagerDelegate::ShouldCompleteDownload(DownloadItem* item) {
   return true;
 }
 
+// Helper method for ShouldOpenDownload.
+content::WebIntentsDispatcher* CreateWebIntentsDispatcherFor(

[michaeln, 2012/03/12 21:16:12]

At this point, we've fully downloaded a file to the Downloads folder and now we
want to provide a reference to that file to the 'intent handler' script as a
'blob'... is that right?

Some questions...

* Does/should this file be visible in the downloads tray?

* Should this file be automagically deleted at some point or should it remain in
the users Downloads folder?

Depending on the answers, things would probably be different in how to handle
it. I think these answers to those questions, YES and NO, make for the simplest
implementation with regard to blob handling. In particular the answer to the
second question (dont automagically delete) makes the real difference.

Assuming for the moment we don't automagically delete these files...

I think you just want to send the full file path (not a blob url) to the
WebIntentHost running in the recipient renderer process. And to use the
WebBlob::createFromFile(full_file_path, file_size) method to create the blob,
that method expects a FilePath as input. And that's it... no need to muck with
blob storage controller and such.


If we do want automagic deletion... things would get a little more
complicated... let me know if that's the behavior you really want.

[Greg Billock, 2012/03/13 17:56:10]

Yes, that's right.
I think so, yes. This is to provide an affordance to the user while the file is
downloading (cancel, resume, etc.) and also a fallback handling if they don't
choose to do anything with the file using intents. (They can still have the
downloaded file there.)
 
I'm inclined to leave it in the downloads folder, but this policy, like the
web-intents-triggering one, we should probably have reviewed.

 
Ah! That'd be great! I'll get the requirements process engaged to decide on the
policy, as this would be really straightforward. We'd need some sandboxing
access set, which we discussed, but that'd be the only wrinkle I know of.

+    const DownloadItem& item,
+    webkit_blob::BlobStorageController* blob_controller) {
+  // This blob url format is not the same as that prepared internally in webkit.
+  // It shouldn't matter, however.
+  GURL blob_url(StringPrintf("blob:chrome-internal/%llux",
+      static_cast<long long unsigned int>(base::RandUint64())));
+
+  blob_controller->StartBuildingBlob(blob_url);
+  webkit_blob::BlobData::Item blob_item;
+     // Need to get the item file time. item.GetLastModifiedTime() is a string
+     // and unset. :-(
+  blob_item.SetToFile(item.GetFullPath(),
+                      0,
+                      item.GetReceivedBytes(),
+                      base::Time::Now());

[michaeln, 2012/03/12 21:16:12]

just use base::Time() here to indicate "doesn't matter"

[Greg Billock, 2012/03/13 17:56:10]

Done.

+  blob_controller->AppendBlobDataItem(blob_url, blob_item);
+  blob_controller->FinishBuildingBlob(blob_url, item.GetMimeType());
+
+  webkit_glue::WebIntentData intent_data(
+      ASCIIToUTF16("http://webintents.org/view"),
+      ASCIIToUTF16(item.GetMimeType()),
+      ASCIIToUTF16(item.GetURL().spec()));
+
+  /*
+  // TODO: need to actually serialize this blob url, but can't really
+  // do that in-browser-process. Need to figure out what to do here.
+  // Hidden params in WebIntentData?
+  intent_data.data = ASCIIToUTF16(blob_url.spec());
+
+  // For now, send the url as extra. Need to re-examine this...
+  intent_data.extra_data.put(ASCIIToUTF16("url"),
+                             ASCIIToUTF16(item.GetURL()).spec());
+  */
+
+  return new InternalWebIntentsDispatcher(intent_data);
+}
+
 bool ChromeDownloadManagerDelegate::ShouldOpenDownload(DownloadItem* item) {
-  if (!IsExtensionDownload(item)) {
-    return true;
+  if (IsExtensionDownload(item)) {
+    scoped_refptr<CrxInstaller> crx_installer =
+        download_crx_util::OpenChromeExtension(profile_, *item);
+
+    // CRX_INSTALLER_DONE will fire when the install completes.  Observe()
+    // will call DelayedDownloadOpened() on this item.  If this DownloadItem is
+    // not around when CRX_INSTALLER_DONE fires, Complete() will not be called.
+    registrar_.Add(this,
+                   chrome::NOTIFICATION_CRX_INSTALLER_DONE,
+                   content::Source<CrxInstaller>(crx_installer.get()));
+
+    crx_installers_[crx_installer.get()] = item->GetId();
+    // The status text and percent complete indicator will change now
+    // that we are installing a CRX.  Update observers so that they pick
+    // up the change.
+    item->UpdateObservers();
+    return false;
   }
 
-  scoped_refptr<CrxInstaller> crx_installer =
-      download_crx_util::OpenChromeExtension(profile_, *item);
-
-  // CRX_INSTALLER_DONE will fire when the install completes.  Observe()
-  // will call DelayedDownloadOpened() on this item.  If this DownloadItem is
-  // not around when CRX_INSTALLER_DONE fires, Complete() will not be called.
-  registrar_.Add(this,
-                 chrome::NOTIFICATION_CRX_INSTALLER_DONE,
-                 content::Source<CrxInstaller>(crx_installer.get()));
-
-  crx_installers_[crx_installer.get()] = item->GetId();
-  // The status text and percent complete indicator will change now
-  // that we are installing a CRX.  Update observers so that they pick
-  // up the change.
-  item->UpdateObservers();
-  return false;
+  if (ShouldWebIntentsHandle(*item)) {
+    content::WebContents* web_contents = item->GetWebContents();
+
+    webkit_blob::BlobStorageController* blob_controller =
+        content::ResourceContext::GetBlobStorageController(
+            profile_->GetResourceContext());
+    content::WebIntentsDispatcher* dispatcher =
+        CreateWebIntentsDispatcherFor(*item, blob_controller);
+    web_contents->GetDelegate()->WebIntentDispatch(web_contents, dispatcher);
+    // Update text/completion indicator on shelf.

[michaeln, 2012/03/12 21:16:12]

just curious... what will the shelf indicator say about this file?

[Greg Billock, 2012/03/13 17:56:10]

Not sure yet...

+    item->UpdateObservers();
+    return false;
+  }
+
+  return true;
 }
 
 bool ChromeDownloadManagerDelegate::GenerateFileHash() {