[BlobReader] Correct IsInMemory = false when async constructing

[BlobReader] Correct IsInMemory = false when async constructing

BUG=611317
Committed: https://crrev.com/1f4afcf1730afbeaf79d6c115d8e92d7708da931
Cr-Commit-Position: refs/heads/master@{#395142}

[dmurph, 2016-05-20 17:48:21]

dmurph@chromium.org changed reviewers:
+ mmenke@chromium.org

[dmurph, 2016-05-20 17:48:21]

Martin,

Can you PTAL? This is to fix the attached bug.

Daniel

[mmenke, 2016-05-20 17:52:30]

Seems like we have some sort of integration test / layout test for this case. 
Otherwise, LGTM.

And it's Matt, not Martin.

[dmurph, 2016-05-20 17:53:46]

Yeah, adding tests now. And whoops! Sorry, mixing all of my people up :0

On Fri, May 20, 2016 at 10:52 AM <mmenke@chromium.org> wrote:

> Seems like we have some sort of integration test / layout test for this
> case.
> Otherwise, LGTM.
>
> And it's Matt, not Martin.
>
> https://codereview.chromium.org/2000483004/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[dmurph, 2016-05-20 18:26:52]

Done, added tests and comment.

[mmenke, 2016-05-20 18:35:54]

On 2016/05/20 18:26:52, dmurph wrote:
> Done, added tests and comment.

I'd suggest a more integrationy-test - like uploading it to the embedded test
server, requesting the "/echo" URL, and seeing you get the result back, to make
sure net's use continues to be compatible.  Up to you, though.

Test would basically be like:

// If your test fixture doesn't already have one.
MessageLoopForIO message_loop;

EmbeddedTestServer test_server;
ASSERT_TRUE(test_server.Start());

TestURLRequestContext context;
net::TestDelegate delegate;
scoped_ptr<URLRequest> request =
context->CreateRequest(test_server->GetURL(/echo), delegate, ...);
request->SetUploadData(std::move(upload_data));

request->Start();
base::RunLoop().Run();

EXPECT_TRUE(request->GetStatus().success());
EXPECT_EQ(<blob_contents>, delegate.response());


Anyhow, like I said, completely up to you, mostly serves to make sure both net
and blob are agreeing on how the UploadDataStream API is.

[dmurph, 2016-05-20 19:09:21]

On 2016/05/20 at 18:35:54, mmenke wrote:
> On 2016/05/20 18:26:52, dmurph wrote:
> > Done, added tests and comment.
> 
> I'd suggest a more integrationy-test - like uploading it to the embedded test
server, requesting the "/echo" URL, and seeing you get the result back, to make
sure net's use continues to be compatible.  Up to you, though.
> 
> Test would basically be like:
> 
> // If your test fixture doesn't already have one.
> MessageLoopForIO message_loop;
> 
> EmbeddedTestServer test_server;
> ASSERT_TRUE(test_server.Start());
> 
> TestURLRequestContext context;
> net::TestDelegate delegate;
> scoped_ptr<URLRequest> request =
context->CreateRequest(test_server->GetURL(/echo), delegate, ...);
> request->SetUploadData(std::move(upload_data));
> 
> request->Start();
> base::RunLoop().Run();
> 
> EXPECT_TRUE(request->GetStatus().success());
> EXPECT_EQ(<blob_contents>, delegate.response());
> 
> 
> Anyhow, like I said, completely up to you, mostly serves to make sure both net
and blob are agreeing on how the UploadDataStream API is.

I would like to do this, but I'm rather swamped right now. I can make a bug to
finish this up when I have more cycles.

[dmurph, 2016-05-20 19:09:32]

The CQ bit was checked by dmurph@chromium.org

[dmurph, 2016-05-20 19:09:33]

The patchset sent to the CQ was uploaded after l-g-t-m from mmenke@chromium.org
Link to the patchset: https://codereview.chromium.org/2000483004/#ps40001
(title: "tests, and comment")

[commit-bot: I haz the power, 2016-05-20 19:09:50]

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

[commit-bot: I haz the power, 2016-05-20 19:52:06]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-05-20 19:54:45]

Description was changed from

==========
[BlobReader] Correct IsInMemory = false when async constructing

BUG=611317
==========

to

==========
[BlobReader] Correct IsInMemory = false when async constructing

BUG=611317

Committed: https://crrev.com/1f4afcf1730afbeaf79d6c115d8e92d7708da931
Cr-Commit-Position: refs/heads/master@{#395142}
==========

[commit-bot: I haz the power, 2016-05-20 19:54:46]

Patchset 3 (id:??) landed as
https://crrev.com/1f4afcf1730afbeaf79d6c115d8e92d7708da931
Cr-Commit-Position: refs/heads/master@{#395142}