Support cancellation of request for opening device.

Support cancellation of request for opening device.

Since Pepper starts to use media stream UI to get device access permission, it needs a way to cancel OpenDevice request, e.g., when page is reloaded.

This patch also fixes a long standing bug in CancelGenerateStream which mixed up request_id with ipc_request_id.

BUG=252904
R=tommi@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=210043

[wjia(left Chromium), 2013-07-03 00:24:15]

We might combine CancelGenerateStream and CancelOpenDevice into one function
CancelRequest. But since it's likely this patch will be merged into earlier
milestone, I'd keep change as small as possible. After the merge, we can combine
the two on ToT.

[tommi (sloooow) - chröme, 2013-07-03 08:05:12]

I've just got one question below.  If the answer to it is that what uniquely
identifies a request is not just the ID but also the event handler, then I think
it would be good to add an IsRequest(id, event_handler) method to the request
class (or in the media stream class) to make that clear and to make sure that
all ID checks also check the event handler.

https://codereview.chromium.org/18420011/diff/1/content/renderer/media/media_...
File content/renderer/media/media_stream_dispatcher.cc (right):

https://codereview.chromium.org/18420011/diff/1/content/renderer/media/media_...
content/renderer/media/media_stream_dispatcher.cc:98: it->handler.get() ==
event_handler.get()) {
is this the right thing to do?  it seems to me that canceling a request with a
specific ID should always cancel that request.
Is it possible perhaps that there be more than one request with the same ID? 
(if so, this needs to be documented)

[wjia(left Chromium), 2013-07-03 17:41:35]

PTAL.

https://codereview.chromium.org/18420011/diff/1/content/renderer/media/media_...
File content/renderer/media/media_stream_dispatcher.cc (right):

https://codereview.chromium.org/18420011/diff/1/content/renderer/media/media_...
content/renderer/media/media_stream_dispatcher.cc:98: it->handler.get() ==
event_handler.get()) {
On 2013/07/03 08:05:12, tommi wrote:
> is this the right thing to do?  it seems to me that canceling a request with a
> specific ID should always cancel that request.
> Is it possible perhaps that there be more than one request with the same ID? 
> (if so, this needs to be documented)

Yes, it's the right thing to do. The request_id is created by each client and
there could be multiple clients, e.g., MediaStreamImpl and
PepperPluginDelegateImpl. Each client should guarantee its request_id is unique
within that handler. 

When MediaStreamDispatcher receives a request, it will map pair (request_id,
handler) to a unique ipc_request.

[tommi (sloooow) - chröme, 2013-07-03 17:51:20]

Thanks for the info. What do you say about adding a method to do the match
(see previous comment) instead of relying on all places to check both the
id and event handler?  If you do that then you have my lgtm :)


On Wed, Jul 3, 2013 at 7:41 PM, <wjia@chromium.org> wrote:

> PTAL.
>
>
>
> https://codereview.chromium.**org/18420011/diff/1/content/**
>
renderer/media/media_stream_**dispatcher.cc<https://codereview.chromium.org/18420011/diff/1/content/renderer/media/media_stream_dispatcher.cc>
> File content/renderer/media/media_**stream_dispatcher.cc (right):
>
> https://codereview.chromium.**org/18420011/diff/1/content/**
>
renderer/media/media_stream_**dispatcher.cc#newcode98<https://codereview.chromium.org/18420011/diff/1/content/renderer/media/media_stream_dispatcher.cc#newcode98>
> content/renderer/media/media_**stream_dispatcher.cc:98: it->handler.get()
> == event_handler.get()) {
> On 2013/07/03 08:05:12, tommi wrote:
>
>> is this the right thing to do?  it seems to me that canceling a
>>
> request with a
>
>> specific ID should always cancel that request.
>> Is it possible perhaps that there be more than one request with the
>>
> same ID?
>
>> (if so, this needs to be documented)
>>
>
> Yes, it's the right thing to do. The request_id is created by each
> client and there could be multiple clients, e.g., MediaStreamImpl and
> PepperPluginDelegateImpl. Each client should guarantee its request_id is
> unique within that handler.
>
> When MediaStreamDispatcher receives a request, it will map pair
> (request_id, handler) to a unique ipc_request.
>
>
https://codereview.chromium.**org/18420011/<https://codereview.chromium.org/1...
>

[tommi (sloooow) - chröme, 2013-07-03 18:56:27]

I somehow missed that you already did this in your latest patch set, so lgtm!

[wjia(left Chromium), 2013-07-03 21:33:05]

Committed patchset #2 manually as r210043 (presubmit successful).