Oops, calling {Producer,Consumer}CloseImplNoLock() isn't the same as calling {Producer,Consumer}Clo…

Oops, calling {Producer,Consumer}CloseImplNoLock() isn't the same as calling {Producer,Consumer}Close().

In particular, we need to wake up any waiters when we go away. (We'll
need this when serializing for real also.)

R=yzshen@chromium.org

Committed: https://chromium.googlesource.com/external/mojo/+/0e2a3b700db35890978b6e0c3b30acab6f76e76a

[viettrungluu, 2015-02-18 23:33:47]

https://codereview.chromium.org/936173002/ now depends on this.

[yzshen1, 2015-02-19 18:07:50]

https://codereview.chromium.org/936283002/diff/1/mojo/edk/system/data_pipe.cc
File mojo/edk/system/data_pipe.cc (right):

https://codereview.chromium.org/936283002/diff/1/mojo/edk/system/data_pipe.cc...
mojo/edk/system/data_pipe.cc:494: ProducerCloseImplNoLock();
I feel that usually XXXImpl should do all (or most) of XXX's work. This doesn't
seem the case. Maybe we should change it to use a more specific name? It helps
to avoid such confusion in the future.

https://codereview.chromium.org/936283002/diff/1/mojo/edk/system/data_pipe.cc...
mojo/edk/system/data_pipe.cc:509: ConsumerCloseImplNoLock();
ditto

[viettrungluu, 2015-02-19 18:23:51]

On 2015/02/19 18:07:50, yzshen1 wrote:
> https://codereview.chromium.org/936283002/diff/1/mojo/edk/system/data_pipe.cc
> File mojo/edk/system/data_pipe.cc (right):
> 
>
https://codereview.chromium.org/936283002/diff/1/mojo/edk/system/data_pipe.cc...
> mojo/edk/system/data_pipe.cc:494: ProducerCloseImplNoLock();
> I feel that usually XXXImpl should do all (or most) of XXX's work. This
doesn't
> seem the case. Maybe we should change it to use a more specific name? It helps
> to avoid such confusion in the future.

I can't think of a reasonable name that clarifies more than it confuses.

> 
>
https://codereview.chromium.org/936283002/diff/1/mojo/edk/system/data_pipe.cc...
> mojo/edk/system/data_pipe.cc:509: ConsumerCloseImplNoLock();
> ditto

[yzshen1, 2015-02-19 18:25:02]

lgtm

[viettrungluu, 2015-02-19 18:25:27]

On 2015/02/19 18:23:51, viettrungluu wrote:
> On 2015/02/19 18:07:50, yzshen1 wrote:
> >
https://codereview.chromium.org/936283002/diff/1/mojo/edk/system/data_pipe.cc
> > File mojo/edk/system/data_pipe.cc (right):
> > 
> >
>
https://codereview.chromium.org/936283002/diff/1/mojo/edk/system/data_pipe.cc...
> > mojo/edk/system/data_pipe.cc:494: ProducerCloseImplNoLock();
> > I feel that usually XXXImpl should do all (or most) of XXX's work. This
> doesn't
> > seem the case. Maybe we should change it to use a more specific name? It
helps
> > to avoid such confusion in the future.
> 
> I can't think of a reasonable name that clarifies more than it confuses.

(Also, it does do most of XXX's work in this case.)

> 
> > 
> >
>
https://codereview.chromium.org/936283002/diff/1/mojo/edk/system/data_pipe.cc...
> > mojo/edk/system/data_pipe.cc:509: ConsumerCloseImplNoLock();
> > ditto

[viettrungluu, 2015-02-19 18:27:12]

Committed patchset #1 (id:1) manually as
0e2a3b700db35890978b6e0c3b30acab6f76e76a (presubmit successful).