Pass correct file system context for uploads with PlzNavigate

Pass correct file system context for uploads with PlzNavigate

The PlzNavigate code path currently passes a nullptr for the file system
context used for creating uploads. This CL passes the correct context
from the RenderFrameHost initiating the navigation.

BUG=709769
Review-Url: https://codereview.chromium.org/2900973002
Cr-Commit-Position: refs/heads/master@{#474337}
Committed: https://chromium.googlesource.com/chromium/src/+/adf0a9975c64c1220fa306cb0c2c9d0f9e5f2b6b

[nasko, 2017-05-24 04:23:14]

Description was changed from

==========
Speculative fix for ChromeOS upload crash.

BUG=709769
==========

to

==========
Pass correct file system context for uploads with PlzNavigate

The PlzNavigate code path currently passes a nullptr for the file system
context used for creating uploads. This CL passes the correct context
from the RenderFrameHost initiating the navigation.

BUG=709769
==========

[nasko, 2017-05-24 04:23:28]

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

[commit-bot: I haz the power, 2017-05-24 04:24:15]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-24 06:01:39]

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

[commit-bot: I haz the power, 2017-05-24 06:01:40]

Dry run: This issue passed the CQ dry run.

[nasko, 2017-05-24 16:00:32]

nasko@chromium.org changed reviewers:
+ jam@chromium.org

[nasko, 2017-05-24 16:00:33]

Hey John,
Can you review this CL for me? It passes the filesystem context to the upload
code, so it can properly upload files from the CrOS files app.
Thanks in advance!
Nasko

https://codereview.chromium.org/2900973002/diff/20001/content/browser/loader/...
File content/browser/loader/navigation_url_loader_impl.cc (right):

https://codereview.chromium.org/2900973002/diff/20001/content/browser/loader/...
content/browser/loader/navigation_url_loader_impl.cc:57:
base::Unretained(storage_partition->GetFileSystemContext()),
This is equivalent to how the ResourceRequesterInfo is passed between threads,
therefore is safe.

[jam, 2017-05-24 16:08:07]

lgtm

https://codereview.chromium.org/2900973002/diff/20001/content/browser/loader/...
File content/browser/loader/navigation_url_loader_impl_core.h (right):

https://codereview.chromium.org/2900973002/diff/20001/content/browser/loader/...
content/browser/loader/navigation_url_loader_impl_core.h:14: #include
"storage/browser/fileapi/file_system_context.h"
nit: you include this in the header and cc file.. just forward declare here?

[mmenke, 2017-05-24 16:10:00]

Driveby comment:  Is there a test for this?

[nasko, 2017-05-24 16:11:35]

On 2017/05/24 16:10:00, mmenke wrote:
> Driveby comment:  Is there a test for this?

I've noted on the bug that there is not and I don't know how to write one. I've
asked the folks working on CrOS to help out write one, but don't want to delay
the fix for that.

[nasko, 2017-05-24 16:19:45]

https://codereview.chromium.org/2900973002/diff/20001/content/browser/loader/...
File content/browser/loader/navigation_url_loader_impl_core.h (right):

https://codereview.chromium.org/2900973002/diff/20001/content/browser/loader/...
content/browser/loader/navigation_url_loader_impl_core.h:14: #include
"storage/browser/fileapi/file_system_context.h"
On 2017/05/24 16:08:07, jam wrote:
> nit: you include this in the header and cc file.. just forward declare here?

Done.

[nasko, 2017-05-24 16:19:54]

The CQ bit was checked by nasko@chromium.org

[nasko, 2017-05-24 16:19:54]

The patchset sent to the CQ was uploaded after l-g-t-m from jam@chromium.org
Link to the patchset: https://codereview.chromium.org/2900973002/#ps40001
(title: "Remove unneeded includes.")

[commit-bot: I haz the power, 2017-05-24 16:20:11]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-24 17:28:51]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1495642794003410,
"parent_rev": "5dc9bafa3dcdc44f93757689ccff9629e8d085cf", "commit_rev":
"adf0a9975c64c1220fa306cb0c2c9d0f9e5f2b6b"}

[commit-bot: I haz the power, 2017-05-24 17:29:06]

Description was changed from

==========
Pass correct file system context for uploads with PlzNavigate

The PlzNavigate code path currently passes a nullptr for the file system
context used for creating uploads. This CL passes the correct context
from the RenderFrameHost initiating the navigation.

BUG=709769
==========

to

==========
Pass correct file system context for uploads with PlzNavigate

The PlzNavigate code path currently passes a nullptr for the file system
context used for creating uploads. This CL passes the correct context
from the RenderFrameHost initiating the navigation.

BUG=709769

Review-Url: https://codereview.chromium.org/2900973002
Cr-Commit-Position: refs/heads/master@{#474337}
Committed:
https://chromium.googlesource.com/chromium/src/+/adf0a9975c64c1220fa306cb0c2c...
==========

[commit-bot: I haz the power, 2017-05-24 17:29:08]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/adf0a9975c64c1220fa306cb0c2c...