Introduce SharedMemoryDataConsumerHandle.

Introduce SharedMemoryDataConsumerHandle.

Introduce SharedMemoryDataConsumerHandle which is a WebDataConsumerHandle
implementation for Chrome IPC Shared Buffer. Currently the handler can only be
used on the thread on which it is created, but it should be fixed in the future.

BUG=480746
Committed: https://crrev.com/2cb393011228d099d9e42611dd397767ec12c2f7
Cr-Commit-Position: refs/heads/master@{#332788}

[yhirano, 2015-05-08 06:17:53]

Patchset #2 (id:20001) has been deleted

[yhirano, 2015-05-08 07:41:03]

Patchset #2 (id:40001) has been deleted

[yhirano, 2015-05-08 08:00:46]

Patchset #2 (id:60001) has been deleted

[yhirano, 2015-05-08 08:01:47]

yhirano@chromium.org changed reviewers:
+ tyoshino@chromium.org

[yhirano, 2015-05-08 08:01:48]

This CL depends on https://codereview.chromium.org/1103813002/.

[yhirano, 2015-05-20 06:57:52]

ping

[tyoshino (SeeGerritForStatus), 2015-05-26 08:05:14]

https://codereview.chromium.org/1118233002/diff/80001/content/child/shared_me...
File content/child/shared_memory_data_consumer_handle.cc (right):

https://codereview.chromium.org/1118233002/diff/80001/content/child/shared_me...
content/child/shared_memory_data_consumer_handle.cc:167:
context_->AdvanceFirstOffset(read_size);
how about exposing only a single method named Consume that does
AdvanceFirstOffset and then PopReadData unless you have use cases in your mind
in which only one of them is used?

Between AdvanceFirstOffset and PopReadData, the context holds a fully consumed
data with which e.g. current IsEmpty() doesn't work correctly.

https://codereview.chromium.org/1118233002/diff/80001/content/child/shared_me...
File content/child/shared_memory_data_consumer_handle.h (right):

https://codereview.chromium.org/1118233002/diff/80001/content/child/shared_me...
content/child/shared_memory_data_consumer_handle.h:22: class Context;
any reason to place this not together with the other private members?

[yhirano, 2015-05-26 08:19:43]

https://codereview.chromium.org/1118233002/diff/80001/content/child/shared_me...
File content/child/shared_memory_data_consumer_handle.cc (right):

https://codereview.chromium.org/1118233002/diff/80001/content/child/shared_me...
content/child/shared_memory_data_consumer_handle.cc:167:
context_->AdvanceFirstOffset(read_size);
On 2015/05/26 08:05:13, tyoshino wrote:
> how about exposing only a single method named Consume that does
> AdvanceFirstOffset and then PopReadData unless you have use cases in your mind
> in which only one of them is used?
> 
> Between AdvanceFirstOffset and PopReadData, the context holds a fully consumed
> data with which e.g. current IsEmpty() doesn't work correctly.

Done.

https://codereview.chromium.org/1118233002/diff/80001/content/child/shared_me...
File content/child/shared_memory_data_consumer_handle.h (right):

https://codereview.chromium.org/1118233002/diff/80001/content/child/shared_me...
content/child/shared_memory_data_consumer_handle.h:22: class Context;
On 2015/05/26 08:05:13, tyoshino wrote:
> any reason to place this not together with the other private members?

It needs to be declared before Writer.

[tyoshino (SeeGerritForStatus), 2015-05-28 07:59:53]

lgtm

https://codereview.chromium.org/1118233002/diff/100001/content/child/shared_m...
File content/child/shared_memory_data_consumer_handle.cc (right):

https://codereview.chromium.org/1118233002/diff/100001/content/child/shared_m...
content/child/shared_memory_data_consumer_handle.cc:76: 
one blank line

[yhirano, 2015-05-28 08:10:59]

https://codereview.chromium.org/1118233002/diff/100001/content/child/shared_m...
File content/child/shared_memory_data_consumer_handle.cc (right):

https://codereview.chromium.org/1118233002/diff/100001/content/child/shared_m...
content/child/shared_memory_data_consumer_handle.cc:76: 
On 2015/05/28 07:59:52, tyoshino wrote:
> one blank line

Done.

[yhirano, 2015-06-03 07:32:25]

Patchset #6 (id:160001) has been deleted

[yhirano, 2015-06-03 08:35:19]

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

[yhirano, 2015-06-03 08:35:20]

+nasko for OWNER review.

[nasko, 2015-06-03 14:18:34]

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

[nasko, 2015-06-03 14:18:35]

Adding avi@ to take a look at this one for OWNERS review.

[Avi (use Gerrit), 2015-06-03 15:53:32]

Really only nits; lgtm.

https://codereview.chromium.org/1118233002/diff/180001/content/child/shared_m...
File content/child/shared_memory_data_consumer_handle_unittest.cc (right):

https://codereview.chromium.org/1118233002/diff/180001/content/child/shared_m...
content/child/shared_memory_data_consumer_handle_unittest.cc:109:
EXPECT_EQ(result, kShouldWait);
nit: expected as the first parameter, actual as the second (errors print better
that way)

https://code.google.com/p/chromium/codesearch#chromium/src/testing/gtest/incl...

here and all cases below

[yhirano, 2015-06-04 05:05:24]

Thanks!

https://codereview.chromium.org/1118233002/diff/180001/content/child/shared_m...
File content/child/shared_memory_data_consumer_handle_unittest.cc (right):

https://codereview.chromium.org/1118233002/diff/180001/content/child/shared_m...
content/child/shared_memory_data_consumer_handle_unittest.cc:109:
EXPECT_EQ(result, kShouldWait);
On 2015/06/03 15:53:31, Avi wrote:
> nit: expected as the first parameter, actual as the second (errors print
better
> that way)
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/testing/gtest/incl...
> 
> here and all cases below

Done.

[yhirano, 2015-06-04 05:05:28]

The CQ bit was checked by yhirano@chromium.org

[yhirano, 2015-06-04 05:05:28]

The patchset sent to the CQ was uploaded after l-g-t-m from
tyoshino@chromium.org, avi@chromium.org
Link to the patchset: https://codereview.chromium.org/1118233002/#ps220001
(title: "")

[commit-bot: I haz the power, 2015-06-04 05:05:48]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1118233002/220001

[commit-bot: I haz the power, 2015-06-04 06:08:13]

Committed patchset #8 (id:220001)

[commit-bot: I haz the power, 2015-06-04 06:09:15]

Patchset 8 (id:??) landed as
https://crrev.com/2cb393011228d099d9e42611dd397767ec12c2f7
Cr-Commit-Position: refs/heads/master@{#332788}