Move download messages from Renderer to Frame filter.

content/renderer/render_view_impl.cc

Index: content/renderer/render_view_impl.cc
diff --git a/content/renderer/render_view_impl.cc b/content/renderer/render_view_impl.cc
index e66153301e131007be323524a5fb471c5d6fb51e..97f2d8fe5f061f4ec60b64cf2d896a5f5ddd0b47 100644
--- a/content/renderer/render_view_impl.cc
+++ b/content/renderer/render_view_impl.cc
@@ -1690,9 +1690,11 @@ void RenderViewImpl::printPage(WebLocalFrame* frame) {
 void RenderViewImpl::saveImageFromDataURL(const blink::WebString& data_url) {
   // Note: We should basically send GURL but we use size-limited string instead
   // in order to send a larger data url to save a image for <canvas> or <img>.
-  if (data_url.length() < kMaxLengthOfDataURLString)
-    Send(new ViewHostMsg_SaveImageFromDataURL(
-        GetRoutingID(), GetMainRenderFrame()->GetRoutingID(), data_url.utf8()));
+  if (data_url.length() < kMaxLengthOfDataURLString) {
+    Send(new FrameHostMsg_SaveImageFromDataURL(
+         GetRoutingID(), GetMainRenderFrame()->GetRoutingID(),

[Charlie Reis, 2016/03/29 04:41:49]

This looks problematic-- we probably shouldn't be assuming that
GetMainRenderFrame() exists.  (It won't in OOPIFs.)  I would kind of expect it
to crash in that case, or if not return the wrong routing ID.

Does the CL fix Rob's repro steps as is?  Maybe we can come back to this.

[brettw, 2016/03/29 05:44:39]

It seems to fix the repro steps. I looked through the code a bit when I started
changing things to make sense of the IDs, and it looks like these are given to
the network system, I assume for prioritization and such. It doesn't look like
we do things like post back to the object (which is what would normally happen)
and the network stack may be tolerant of such errors. I'll check more tomorrow.

[Charlie Reis, 2016/03/29 18:17:13]

Ok.  CC'ing rdsmith (who added the RFH ID, I think) and csharrison (who has been
refactoring the net stack for RFHs) in case they know more.

I'm ok landing this with a TODO if it's a step in the right direction.  

Still, I'd be curious to know why we aren't crashing here, since
GetMainRenderFrame() returns null in an OOPIF process.  I guess we never get
this call in an OOPIF, due to the bug I mentioned above (line 1421)?

[asanka, 2016/03/29 20:56:05]

AFAICT the reason why it's not crashing is:

* None of the test cases in the linked issue trip this code path. In order to
get here, you'll need to create a canvas element and download it via the "Save
image as..." context menu item. For it to be sensitive to OOPIF, the canvas
element needs to be in an OOPIF.

* Invoking "Save image as" on such a canvas element causes the SaveImageAt IPC
to be sent from the main frame's RenderViewHostImpl. The corresponding
RenderView does in fact have a valid main_render_frame_.

* The RenderView that receives the SaveImageAt IPC doesn't have an image at the
location, since there's an iframe there instead. Therefore nothing happens.
Whether or not the main_render_frame_ is valid appears to be moot.

I'm guessing this code path doesn't have test coverage? It's broken ToT.

Also, the code above is trying to dispatch a FrameHostMsg to a RenderViewImpl,
which isn't quite relevant since the code runs on the wrong process.

+         data_url.utf8()));
+  }
 }
 
 bool RenderViewImpl::enumerateChosenDirectory(