Updates to DownloadUrlParameters in preparation for OOPIF changes

Updates to DownloadUrlParameters in preparation for OOPIF changes

This should have no behavior change except for theoretical crash fix.

Renames DownloadUrlParameters::FromWebContents to CreateForWebContentsMainFrame to be more clear what this actually does. Clean up the constructors. WebContentsImpl::SaveFrameWithHeaders is a bit different. The functionality is the same but I separated out the frame computation code so that in the future we can replace the frame with the right one and the rest of the function will be correct (we won't want to use the main frame in the future).

Add null check for RenderFrameHost in a RenderViewContextMenu that was previously not checking for null. This is a potential crash if the frame disappears when the menu is open.

Make the RenderViewContextMenu construct a DownloadUrlParameters object referencing the frame doing the downloading rather than the main frame of the page.

Rename some child_id variables to process_id which makes more sense to me.

Add some comments clarifying which RenderViewHost should be used for DownloadUrlParameters and ResourceRequestInfo.

Remove "temporary" comment about RenderViewHost from render_frame_host.h. Charlie says this will not go away (this was confusing me and made me thing that the relationship between some of these things is different than it is) but will likely just be renamed in the future).

BUG=596283, 482049
Committed: https://crrev.com/760f7448c5798bfd55734e412f09f562e3a469b2
Cr-Commit-Position: refs/heads/master@{#395412}

[brettw, 2016-05-12 22:04:37]

Description was changed from

==========
Updates to DownloadUrlParameters in preparation for OOPIF changes
==========

to

==========
Updates to DownloadUrlParameters in preparation for OOPIF changes

Renames DownloadUrlParameters::FromWebContents to CreateForWebContentsMainFrame
to be more clear what this actually does. Update existing callers. Convert
webstore_installer.cc to use it (it was doing the equivalent construction
manually).

Add null check for RenderFrameHost in a RenderViewContextMenu that was
previously not checking for null.

Separate out "save link as" into a helper function in the context menu code for
clarity.

Make the RenderViewContextMenu construct a DownloadUrlParameters object
referencing the frame doing the downloading rather than the main frame of the
page.

Make a version of the DownloadUrlParameters constructor that sets the various
IDs to -1.

Rename some child_id variables to process_id which makes more sense to me.

WebContentsImpl::SaveFrameWithHeaders is a bit different. The functionality is
the same but I separated out the frame computation code so that in the future we
can replace the frame with the right one and the rest of the function will be
correct (we won't want to use the main frame in the future).

Add some comments clarifying which RenderViewHost should be used for
DownloadUrlParameters and ResourceRequestInfo.

Remove "temporary" comment about RenderViewHost from render_frame_host.h.
Charlie says this will not go away (this was confusing me and made me thing that
the relationship between some of these things is different than it is) but will
likely just be renamed in the future).
==========

[brettw, 2016-05-12 22:06:04]

Description was changed from

==========
Updates to DownloadUrlParameters in preparation for OOPIF changes

Renames DownloadUrlParameters::FromWebContents to CreateForWebContentsMainFrame
to be more clear what this actually does. Update existing callers. Convert
webstore_installer.cc to use it (it was doing the equivalent construction
manually).

Add null check for RenderFrameHost in a RenderViewContextMenu that was
previously not checking for null.

Separate out "save link as" into a helper function in the context menu code for
clarity.

Make the RenderViewContextMenu construct a DownloadUrlParameters object
referencing the frame doing the downloading rather than the main frame of the
page.

Make a version of the DownloadUrlParameters constructor that sets the various
IDs to -1.

Rename some child_id variables to process_id which makes more sense to me.

WebContentsImpl::SaveFrameWithHeaders is a bit different. The functionality is
the same but I separated out the frame computation code so that in the future we
can replace the frame with the right one and the rest of the function will be
correct (we won't want to use the main frame in the future).

Add some comments clarifying which RenderViewHost should be used for
DownloadUrlParameters and ResourceRequestInfo.

Remove "temporary" comment about RenderViewHost from render_frame_host.h.
Charlie says this will not go away (this was confusing me and made me thing that
the relationship between some of these things is different than it is) but will
likely just be renamed in the future).
==========

to

==========
Updates to DownloadUrlParameters in preparation for OOPIF changes

Renames DownloadUrlParameters::FromWebContents to CreateForWebContentsMainFrame
to be more clear what this actually does. Update existing callers. Convert
webstore_installer.cc to use it (it was doing the equivalent construction
manually).

Add null check for RenderFrameHost in a RenderViewContextMenu that was
previously not checking for null.

Separate out "save link as" into a helper function in the context menu code for
clarity.

Make the RenderViewContextMenu construct a DownloadUrlParameters object
referencing the frame doing the downloading rather than the main frame of the
page.

Make a version of the DownloadUrlParameters constructor that sets the various
IDs to -1.

Rename some child_id variables to process_id which makes more sense to me.

WebContentsImpl::SaveFrameWithHeaders is a bit different. The functionality is
the same but I separated out the frame computation code so that in the future we
can replace the frame with the right one and the rest of the function will be
correct (we won't want to use the main frame in the future).

Add some comments clarifying which RenderViewHost should be used for
DownloadUrlParameters and ResourceRequestInfo.

Remove "temporary" comment about RenderViewHost from render_frame_host.h.
Charlie says this will not go away (this was confusing me and made me thing that
the relationship between some of these things is different than it is) but will
likely just be renamed in the future).

BUG=596283, 482049
==========

[brettw, 2016-05-12 22:07:53]

brettw@chromium.org changed reviewers:
+ asanka@chromium.org, rdsmith@chromium.org

[brettw, 2016-05-12 22:07:54]

I'm spinning this patch out underneath the previous one:
https://codereview.chromium.org/1836973003/
that sets up some of the ID problems exposed by that patch.

[brettw, 2016-05-12 22:07:59]

The CQ bit was checked by brettw@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-12 22:08:12]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1977623002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1977623002/20001

[commit-bot: I haz the power, 2016-05-12 22:10:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-12 22:10:17]

Dry run: Try jobs failed on following builders:
  ios-device on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-gn/bui...)
  ios-simulator on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[Randy Smith (Not in Mondays), 2016-05-12 22:15:02]

I'm presuming Asanka will take this--at least, I won't get to it until
Monday--but I think you need to edit and coalesce the CL description; it looks
like a set of git commit messages.

[brettw, 2016-05-12 23:04:15]

Description was changed from

==========
Updates to DownloadUrlParameters in preparation for OOPIF changes

Renames DownloadUrlParameters::FromWebContents to CreateForWebContentsMainFrame
to be more clear what this actually does. Update existing callers. Convert
webstore_installer.cc to use it (it was doing the equivalent construction
manually).

Add null check for RenderFrameHost in a RenderViewContextMenu that was
previously not checking for null.

Separate out "save link as" into a helper function in the context menu code for
clarity.

Make the RenderViewContextMenu construct a DownloadUrlParameters object
referencing the frame doing the downloading rather than the main frame of the
page.

Make a version of the DownloadUrlParameters constructor that sets the various
IDs to -1.

Rename some child_id variables to process_id which makes more sense to me.

WebContentsImpl::SaveFrameWithHeaders is a bit different. The functionality is
the same but I separated out the frame computation code so that in the future we
can replace the frame with the right one and the rest of the function will be
correct (we won't want to use the main frame in the future).

Add some comments clarifying which RenderViewHost should be used for
DownloadUrlParameters and ResourceRequestInfo.

Remove "temporary" comment about RenderViewHost from render_frame_host.h.
Charlie says this will not go away (this was confusing me and made me thing that
the relationship between some of these things is different than it is) but will
likely just be renamed in the future).

BUG=596283, 482049
==========

to

==========
Updates to DownloadUrlParameters in preparation for OOPIF changes

Renames DownloadUrlParameters::FromWebContents to CreateForWebContentsMainFrame
to be more clear what this actually does. Clean up the constructors.
WebContentsImpl::SaveFrameWithHeaders is a bit different. The functionality is
the same but I separated out the frame computation code so that in the future we
can replace the frame with the right one and the rest of the function will be
correct (we won't want to use the main frame in the future).

Add null check for RenderFrameHost in a RenderViewContextMenu that was
previously not checking for null. This is a potential crash if the frame
disappears when the menu is open.

Make the RenderViewContextMenu construct a DownloadUrlParameters object
referencing the frame doing the downloading rather than the main frame of the
page.

Rename some child_id variables to process_id which makes more sense to me.

Add some comments clarifying which RenderViewHost should be used for
DownloadUrlParameters and ResourceRequestInfo.

Remove "temporary" comment about RenderViewHost from render_frame_host.h.
Charlie says this will not go away (this was confusing me and made me thing that
the relationship between some of these things is different than it is) but will
likely just be renamed in the future).

BUG=596283, 482049
==========

[brettw, 2016-05-12 23:54:33]

The CQ bit was checked by brettw@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-12 23:54:59]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1977623002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1977623002/40001

[brettw, 2016-05-12 23:55:52]

New snap up (merge) with updated description

[commit-bot: I haz the power, 2016-05-13 00:10:40]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-13 00:10:41]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_chromium_gn_compile_rel on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_chro...)

[brettw, 2016-05-13 16:51:43]

Description was changed from

==========
Updates to DownloadUrlParameters in preparation for OOPIF changes

Renames DownloadUrlParameters::FromWebContents to CreateForWebContentsMainFrame
to be more clear what this actually does. Clean up the constructors.
WebContentsImpl::SaveFrameWithHeaders is a bit different. The functionality is
the same but I separated out the frame computation code so that in the future we
can replace the frame with the right one and the rest of the function will be
correct (we won't want to use the main frame in the future).

Add null check for RenderFrameHost in a RenderViewContextMenu that was
previously not checking for null. This is a potential crash if the frame
disappears when the menu is open.

Make the RenderViewContextMenu construct a DownloadUrlParameters object
referencing the frame doing the downloading rather than the main frame of the
page.

Rename some child_id variables to process_id which makes more sense to me.

Add some comments clarifying which RenderViewHost should be used for
DownloadUrlParameters and ResourceRequestInfo.

Remove "temporary" comment about RenderViewHost from render_frame_host.h.
Charlie says this will not go away (this was confusing me and made me thing that
the relationship between some of these things is different than it is) but will
likely just be renamed in the future).

BUG=596283, 482049
==========

to

==========
Updates to DownloadUrlParameters in preparation for OOPIF changes

This should have no behavior change.

Renames DownloadUrlParameters::FromWebContents to CreateForWebContentsMainFrame
to be more clear what this actually does. Clean up the constructors.
WebContentsImpl::SaveFrameWithHeaders is a bit different. The functionality is
the same but I separated out the frame computation code so that in the future we
can replace the frame with the right one and the rest of the function will be
correct (we won't want to use the main frame in the future).

Add null check for RenderFrameHost in a RenderViewContextMenu that was
previously not checking for null. This is a potential crash if the frame
disappears when the menu is open.

Make the RenderViewContextMenu construct a DownloadUrlParameters object
referencing the frame doing the downloading rather than the main frame of the
page.

Rename some child_id variables to process_id which makes more sense to me.

Add some comments clarifying which RenderViewHost should be used for
DownloadUrlParameters and ResourceRequestInfo.

Remove "temporary" comment about RenderViewHost from render_frame_host.h.
Charlie says this will not go away (this was confusing me and made me thing that
the relationship between some of these things is different than it is) but will
likely just be renamed in the future).

BUG=596283, 482049
==========

[brettw, 2016-05-13 16:52:15]

Description was changed from

==========
Updates to DownloadUrlParameters in preparation for OOPIF changes

This should have no behavior change.

Renames DownloadUrlParameters::FromWebContents to CreateForWebContentsMainFrame
to be more clear what this actually does. Clean up the constructors.
WebContentsImpl::SaveFrameWithHeaders is a bit different. The functionality is
the same but I separated out the frame computation code so that in the future we
can replace the frame with the right one and the rest of the function will be
correct (we won't want to use the main frame in the future).

Add null check for RenderFrameHost in a RenderViewContextMenu that was
previously not checking for null. This is a potential crash if the frame
disappears when the menu is open.

Make the RenderViewContextMenu construct a DownloadUrlParameters object
referencing the frame doing the downloading rather than the main frame of the
page.

Rename some child_id variables to process_id which makes more sense to me.

Add some comments clarifying which RenderViewHost should be used for
DownloadUrlParameters and ResourceRequestInfo.

Remove "temporary" comment about RenderViewHost from render_frame_host.h.
Charlie says this will not go away (this was confusing me and made me thing that
the relationship between some of these things is different than it is) but will
likely just be renamed in the future).

BUG=596283, 482049
==========

to

==========
Updates to DownloadUrlParameters in preparation for OOPIF changes

This should have no behavior change except for theoretical crash fix.

Renames DownloadUrlParameters::FromWebContents to CreateForWebContentsMainFrame
to be more clear what this actually does. Clean up the constructors.
WebContentsImpl::SaveFrameWithHeaders is a bit different. The functionality is
the same but I separated out the frame computation code so that in the future we
can replace the frame with the right one and the rest of the function will be
correct (we won't want to use the main frame in the future).

Add null check for RenderFrameHost in a RenderViewContextMenu that was
previously not checking for null. This is a potential crash if the frame
disappears when the menu is open.

Make the RenderViewContextMenu construct a DownloadUrlParameters object
referencing the frame doing the downloading rather than the main frame of the
page.

Rename some child_id variables to process_id which makes more sense to me.

Add some comments clarifying which RenderViewHost should be used for
DownloadUrlParameters and ResourceRequestInfo.

Remove "temporary" comment about RenderViewHost from render_frame_host.h.
Charlie says this will not go away (this was confusing me and made me thing that
the relationship between some of these things is different than it is) but will
likely just be renamed in the future).

BUG=596283, 482049
==========

[brettw, 2016-05-13 17:07:49]

The CQ bit was checked by brettw@chromium.org to run a CQ dry run

[brettw, 2016-05-13 17:07:51]

fix

[commit-bot: I haz the power, 2016-05-13 17:08:33]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1977623002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1977623002/60001

[commit-bot: I haz the power, 2016-05-13 18:12:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-13 18:12:27]

Dry run: This issue passed the CQ dry run.

[asanka, 2016-05-13 20:53:42]

There were a couple of places where we were using ::FromWebContents() to
initiatlize DownloadUrlParameters. Looks like they won't be quite correct for
OOPIF. If so, would fixing those be in scope for this CL?

https://codereview.chromium.org/1977623002/diff/40001/content/browser/android...
File content/browser/android/download_controller_android_impl.cc (right):

https://codereview.chromium.org/1977623002/diff/40001/content/browser/android...
content/browser/android/download_controller_android_impl.cc:84:
content::DownloadUrlParameters::CreateForWebContentsMainFrame(
I don't think this is correct. :-/

https://codereview.chromium.org/1977623002/diff/40001/content/public/browser/...
File content/public/browser/download_url_parameters.h (right):

https://codereview.chromium.org/1977623002/diff/40001/content/public/browser/...
content/public/browser/download_url_parameters.h:74: // associating it with a
fraem don't receive the same security checks
*frame

https://codereview.chromium.org/1977623002/diff/40001/content/public/browser/...
content/public/browser/download_url_parameters.h:85: // different RenderView
than the WebContents' main RenderView.
Shall we get rid of the RVID in DownloadUrlParameters? If a valid
render_process_id and a render_frame_id is present,
DownloadManagerImpl::DownloadUrl should be able to look up the render_view_id.

https://codereview.chromium.org/1977623002/diff/60001/content/browser/downloa...
File content/browser/download/drag_download_file.cc (right):

https://codereview.chromium.org/1977623002/diff/60001/content/browser/downloa...
content/browser/download/drag_download_file.cc:69:
DownloadUrlParameters::CreateForWebContentsMainFrame(
This one is also fishy.

[brettw, 2016-05-13 21:20:57]

I don't actually understand either of the two cases you called out as fishy very
well, so I would be inclined to file bugs and note those bugs in the code.

https://codereview.chromium.org/1977623002/diff/40001/content/public/browser/...
File content/public/browser/download_url_parameters.h (right):

https://codereview.chromium.org/1977623002/diff/40001/content/public/browser/...
content/public/browser/download_url_parameters.h:85: // different RenderView
than the WebContents' main RenderView.
On 2016/05/13 20:53:41, asanka wrote:
> Shall we get rid of the RVID in DownloadUrlParameters? If a valid
> render_process_id and a render_frame_id is present,
> DownloadManagerImpl::DownloadUrl should be able to look up the render_view_id.

I tried this on the path to generating this patch. This gets passed into the
ResourceRequestInfo which is a much, much bigger change than this.

https://codereview.chromium.org/1977623002/diff/60001/content/browser/downloa...
File content/browser/download/drag_download_file.cc (right):

https://codereview.chromium.org/1977623002/diff/60001/content/browser/downloa...
content/browser/download/drag_download_file.cc:69:
DownloadUrlParameters::CreateForWebContentsMainFrame(
On 2016/05/13 20:53:41, asanka wrote:
> This one is also fishy.

I thought about this one. I was thinking this was dragging a file to the tab so
it didn't really matter, but on further reflection I think this is dragging a
file out of a tab which I agree sounds wrong.

[asanka, 2016-05-18 02:12:51]

Sorry about the delay. I'll get to this first thing in the morning.

[asanka, 2016-05-18 15:26:37]

lgtm.

I still think we should associate created downloads with the correct
RenderFrame. But it can be done separately from this CL. Would you mind filing a
bug for it?

Render frames under the same WebContents may be associated with different
SiteInstances. Storage partitions are currently a function of <BrowserContext,
SiteInstance>. Hence, consumers of the API should assume that a single
WebContents may combine render frames associated with different storage
partitions. Some of the call sites that use CreateForWebContentsMainFrame()
don't respect this split and are only correct if all render frames within a
single WebContents are restricted to a single StoragePartition.

Currently this assumption works out since it lines up with the historical
relationship between WebContents <-> StoragePartition as implemented in
practice. But I don't think it's healthy to leave these assumptions in the code.

https://codereview.chromium.org/1977623002/diff/60001/content/browser/downloa...
File content/browser/download/drag_download_file.cc (right):

https://codereview.chromium.org/1977623002/diff/60001/content/browser/downloa...
content/browser/download/drag_download_file.cc:69:
DownloadUrlParameters::CreateForWebContentsMainFrame(
On 2016/05/13 at 21:20:57, brettw wrote:
> On 2016/05/13 20:53:41, asanka wrote:
> > This one is also fishy.
> 
> I thought about this one. I was thinking this was dragging a file to the tab
so it didn't really matter, but on further reflection I think this is dragging a
file out of a tab which I agree sounds wrong.

Yeah. This is HTML5 drag & drop. See the "dragging files" example at
http://www.html5rocks.com/en/tutorials/dnd/basics/ .

[brettw, 2016-05-23 20:12:38]

On 2016/05/18 15:26:37, asanka wrote:
> lgtm.
> 
> I still think we should associate created downloads with the correct
> RenderFrame. But it can be done separately from this CL. Would you mind filing
a
> bug for it?

I filed https://bugs.chromium.org/p/chromium/issues/detail?id=614134 and I
linked to that in a todo for the drag-and-drop case.

[brettw, 2016-05-23 20:14:21]

The CQ bit was checked by brettw@chromium.org

[brettw, 2016-05-23 20:14:21]

The patchset sent to the CQ was uploaded after l-g-t-m from asanka@chromium.org
Link to the patchset: https://codereview.chromium.org/1977623002/#ps80001
(title: "Review comments")

[commit-bot: I haz the power, 2016-05-23 20:14:36]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1977623002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1977623002/80001

[commit-bot: I haz the power, 2016-05-23 21:19:34]

Description was changed from

==========
Updates to DownloadUrlParameters in preparation for OOPIF changes

This should have no behavior change except for theoretical crash fix.

Renames DownloadUrlParameters::FromWebContents to CreateForWebContentsMainFrame
to be more clear what this actually does. Clean up the constructors.
WebContentsImpl::SaveFrameWithHeaders is a bit different. The functionality is
the same but I separated out the frame computation code so that in the future we
can replace the frame with the right one and the rest of the function will be
correct (we won't want to use the main frame in the future).

Add null check for RenderFrameHost in a RenderViewContextMenu that was
previously not checking for null. This is a potential crash if the frame
disappears when the menu is open.

Make the RenderViewContextMenu construct a DownloadUrlParameters object
referencing the frame doing the downloading rather than the main frame of the
page.

Rename some child_id variables to process_id which makes more sense to me.

Add some comments clarifying which RenderViewHost should be used for
DownloadUrlParameters and ResourceRequestInfo.

Remove "temporary" comment about RenderViewHost from render_frame_host.h.
Charlie says this will not go away (this was confusing me and made me thing that
the relationship between some of these things is different than it is) but will
likely just be renamed in the future).

BUG=596283, 482049
==========

to

==========
Updates to DownloadUrlParameters in preparation for OOPIF changes

This should have no behavior change except for theoretical crash fix.

Renames DownloadUrlParameters::FromWebContents to CreateForWebContentsMainFrame
to be more clear what this actually does. Clean up the constructors.
WebContentsImpl::SaveFrameWithHeaders is a bit different. The functionality is
the same but I separated out the frame computation code so that in the future we
can replace the frame with the right one and the rest of the function will be
correct (we won't want to use the main frame in the future).

Add null check for RenderFrameHost in a RenderViewContextMenu that was
previously not checking for null. This is a potential crash if the frame
disappears when the menu is open.

Make the RenderViewContextMenu construct a DownloadUrlParameters object
referencing the frame doing the downloading rather than the main frame of the
page.

Rename some child_id variables to process_id which makes more sense to me.

Add some comments clarifying which RenderViewHost should be used for
DownloadUrlParameters and ResourceRequestInfo.

Remove "temporary" comment about RenderViewHost from render_frame_host.h.
Charlie says this will not go away (this was confusing me and made me thing that
the relationship between some of these things is different than it is) but will
likely just be renamed in the future).

BUG=596283, 482049
==========

[commit-bot: I haz the power, 2016-05-23 21:19:36]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-05-23 21:21:12]

Description was changed from

==========
Updates to DownloadUrlParameters in preparation for OOPIF changes

This should have no behavior change except for theoretical crash fix.

Renames DownloadUrlParameters::FromWebContents to CreateForWebContentsMainFrame
to be more clear what this actually does. Clean up the constructors.
WebContentsImpl::SaveFrameWithHeaders is a bit different. The functionality is
the same but I separated out the frame computation code so that in the future we
can replace the frame with the right one and the rest of the function will be
correct (we won't want to use the main frame in the future).

Add null check for RenderFrameHost in a RenderViewContextMenu that was
previously not checking for null. This is a potential crash if the frame
disappears when the menu is open.

Make the RenderViewContextMenu construct a DownloadUrlParameters object
referencing the frame doing the downloading rather than the main frame of the
page.

Rename some child_id variables to process_id which makes more sense to me.

Add some comments clarifying which RenderViewHost should be used for
DownloadUrlParameters and ResourceRequestInfo.

Remove "temporary" comment about RenderViewHost from render_frame_host.h.
Charlie says this will not go away (this was confusing me and made me thing that
the relationship between some of these things is different than it is) but will
likely just be renamed in the future).

BUG=596283, 482049
==========

to

==========
Updates to DownloadUrlParameters in preparation for OOPIF changes

This should have no behavior change except for theoretical crash fix.

Renames DownloadUrlParameters::FromWebContents to CreateForWebContentsMainFrame
to be more clear what this actually does. Clean up the constructors.
WebContentsImpl::SaveFrameWithHeaders is a bit different. The functionality is
the same but I separated out the frame computation code so that in the future we
can replace the frame with the right one and the rest of the function will be
correct (we won't want to use the main frame in the future).

Add null check for RenderFrameHost in a RenderViewContextMenu that was
previously not checking for null. This is a potential crash if the frame
disappears when the menu is open.

Make the RenderViewContextMenu construct a DownloadUrlParameters object
referencing the frame doing the downloading rather than the main frame of the
page.

Rename some child_id variables to process_id which makes more sense to me.

Add some comments clarifying which RenderViewHost should be used for
DownloadUrlParameters and ResourceRequestInfo.

Remove "temporary" comment about RenderViewHost from render_frame_host.h.
Charlie says this will not go away (this was confusing me and made me thing that
the relationship between some of these things is different than it is) but will
likely just be renamed in the future).

BUG=596283, 482049

Committed: https://crrev.com/760f7448c5798bfd55734e412f09f562e3a469b2
Cr-Commit-Position: refs/heads/master@{#395412}
==========

[commit-bot: I haz the power, 2016-05-23 21:21:13]

Patchset 5 (id:??) landed as
https://crrev.com/760f7448c5798bfd55734e412f09f562e3a469b2
Cr-Commit-Position: refs/heads/master@{#395412}