Allow URL requests for object/embed tags to be intercepted as streams.

content/browser/loader/buffered_resource_handler.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 "content/browser/loader/buffered_resource_handler.h"
 
 #include <vector>
 
 #include "base/bind.h"
 #include "base/logging.h"
@@ skipping 294 matching lines @@
   const std::string& mime_type = response_->head.mime_type;
 
   if (net::IsSupportedCertificateMimeType(mime_type)) {
     // Install certificate file.
     info->set_is_download(true);
     scoped_ptr<ResourceHandler> handler(
         new CertificateResourceHandler(request()));
     return UseAlternateNextHandler(handler.Pass(), std::string());
   }
 
+  // Allow requests for object/embed tags to be intercepted as streams.
+  if (info->GetResourceType() == content::RESOURCE_TYPE_OBJECT) {

[Zachary Kuznia, 2014/11/13 23:18:08]

Why isn't this caught by line 333?

[raymes, 2014/11/13 23:42:10]

Because in these cases |allow_download| is always false. The reason is that
allow_download is only ever true if the resource types are frames (see
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/lo...).

This makes sense because if the request isn't coming from a frame we don't want
the browser to download it.

[Zachary Kuznia, 2014/11/13 23:48:42]

It feels like we should share this call with the other case, instead of copying
it.  However, that might made the code harder to read...  Can you think of a way
to re-arrange this so that MaybeInterceptAsStream has only one call site in this
function?

[raymes, 2014/11/14 20:55:29]

I did try to share the code in a few different ways but none of them seemed to
make it clearer. If we have a function, it needs to have a return value which is
a bool indicating whether the stream was intercepted and an out param which is a
bool which indicates the return value which felt a bit awkward. If you prefer
this, let me know and I can make the change :)

[Zachary Kuznia, 2014/11/14 22:01:45]

Fair enough, I thought that might be the case.

+    DCHECK(!info->allow_download());
+    std::string payload;
+    scoped_ptr<ResourceHandler> handler(
+        host_->MaybeInterceptAsStream(request(), response_.get(), &payload));
+    return UseAlternateNextHandler(handler.Pass(), payload);

[Zachary Kuznia, 2014/11/13 23:18:08]

Is there a reason not to test that handler is set?

[raymes, 2014/11/13 23:42:10]

There is one other place where requests are made on behalf of objects which is
for pnacl pexes:
https://code.google.com/p/chromium/codesearch#chromium/src/components/nacl/re...

In those cases the request might not be intercepted as a stream.

[Zachary Kuznia, 2014/11/13 23:48:42]

In that case, shouldn't we maintain the current behavior of returning true, like
like 325?

[raymes, 2014/11/14 20:55:29]

Yes, good catch! I added a test for this.

+  }
+
   if (!info->allow_download())
     return true;
 
   bool must_download = MustDownload();
   if (!must_download) {
     if (net::IsSupportedMimeType(mime_type))
       return true;
 
     std::string payload;
     scoped_ptr<ResourceHandler> handler(
@@ skipping 169 matching lines @@
   request()->LogUnblocked();
   bool defer = false;
   if (!ProcessResponse(&defer)) {
     controller()->Cancel();
   } else if (!defer) {
     controller()->Resume();
   }
 }
 
 }  // namespace content