Serialize processing of getUserMedia() requests.

Serialize processing of getUserMedia() requests.

This is in anticipation to using the spec-compliant algorithm for
constraints processing.
The standard algorithm requires knowing the capabilities of all devices
before selecting the settings to be used. If two requests are processed
in parallel this can lead to races that would result in unexpected
errors. For example, there may be two requests that require the same
camera but prefer (though not require) different resolutions.
If the requests are handled in parallel and the camera has not been
opened yet, both requests will see that all resolutions are supported
and both will try to open the camera with different resolutions and one
of the requests will fail unxpectedly.
If the requests are handled sequentially, the camera will be opened with
the resolution selected by the first request, and the second request will
see that that resolution is the only available one. If that resolution
satisfies the constraints for the second request, the request will
succeed. Otherwise, it will fail in a spec-compliant way.

This CL also includes:
* A minor refactoring to expose fewer implementation details to the
  unit test.
* Removal of some unnecessary includes and logging statements.
* More consistent naming of variables. request_info for
  UserMediaRequestInfo and request or user_media_request for
  WebUserMediaRequest.

BUG=657733
Review-Url: https://codereview.chromium.org/2764043002
Cr-Commit-Position: refs/heads/master@{#458560}
Committed: https://chromium.googlesource.com/chromium/src/+/d727ed27c3dde3c85195625b97e79472eda73508

[Guido Urdaneta, 2017-03-21 14:41:35]

Patchset #1 (id:1) has been deleted

[Guido Urdaneta, 2017-03-21 14:49:29]

Patchset #1 (id:20001) has been deleted

[Guido Urdaneta, 2017-03-21 14:50:36]

Description was changed from

==========
Serialize processing of getUserMedia() requests.

This is in anticipation to using the spec-compliant algorithm for
constraints processing.
The standard algorithm requires knowing the capabilities of all devices
before selecting the settings to be used. If two requests are processed
in parallel this can lead to races that would result in unexpected
errors. For example, there may be two requests that require the same
camera but prefer (though not require) different resolutions.
If the requests are handled in parallel and the camera has not been
opened yet, both requests will see that all resolutions are supported
and both will try to open the camera with different resolutions and one
of the requests will fail unxpectedly.
If the requests are handled sequentially, the camera will be opened with
the resolution selected by the first request, and the second request will
see that that resolution is the only available one. If that resolution
satisfies the constraints for the second request, the request will
succeed. Otherwise, it will fail in a spec-compliant way.

This CL also includes:
* A minor refactoring to expose fewer implementation details to the
  unit test.
* Removal of some unnecessary includes and logging statements.

BUG=657733
==========

to

==========
Serialize processing of getUserMedia() requests.

This is in anticipation to using the spec-compliant algorithm for
constraints processing.
The standard algorithm requires knowing the capabilities of all devices
before selecting the settings to be used. If two requests are processed
in parallel this can lead to races that would result in unexpected
errors. For example, there may be two requests that require the same
camera but prefer (though not require) different resolutions.
If the requests are handled in parallel and the camera has not been
opened yet, both requests will see that all resolutions are supported
and both will try to open the camera with different resolutions and one
of the requests will fail unxpectedly.
If the requests are handled sequentially, the camera will be opened with
the resolution selected by the first request, and the second request will
see that that resolution is the only available one. If that resolution
satisfies the constraints for the second request, the request will
succeed. Otherwise, it will fail in a spec-compliant way.

This CL also includes:
* A minor refactoring to expose fewer implementation details to the
  unit test.
* Removal of some unnecessary includes and logging statements.
* More consistent naming of variables. request_info for
  UserMediaRequestInfo and request or user_media_request for
  WebUserMediaRequest.

BUG=657733
==========

[Guido Urdaneta, 2017-03-21 14:50:50]

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

[commit-bot: I haz the power, 2017-03-21 14:51:06]

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-03-21 16:13:08]

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

[commit-bot: I haz the power, 2017-03-21 16:13:09]

Dry run: This issue passed the CQ dry run.

[Guido Urdaneta, 2017-03-21 16:13:57]

guidou@chromium.org changed reviewers:
+ tommi@chromium.org

[Guido Urdaneta, 2017-03-21 16:14:10]

Hi, PTAL.

[tommi (sloooow) - chröme, 2017-03-21 16:59:34]

https://codereview.chromium.org/2764043002/diff/40001/content/renderer/media/...
File content/renderer/media/user_media_client_impl.cc (right):

https://codereview.chromium.org/2764043002/diff/40001/content/renderer/media/...
content/renderer/media/user_media_client_impl.cc:246: int request_id;
lots of public member variables - intentional? Maybe this should be a struct? 
If not, then the variables should have a postfix underscore (and ideally be
private).

Maybe some of the implementation that uses these variables should be moved into
the class?

Also, the convention is to have variables below methods.

[Guido Urdaneta, 2017-03-21 18:41:34]

https://codereview.chromium.org/2764043002/diff/40001/content/renderer/media/...
File content/renderer/media/user_media_client_impl.cc (right):

https://codereview.chromium.org/2764043002/diff/40001/content/renderer/media/...
content/renderer/media/user_media_client_impl.cc:246: int request_id;
On 2017/03/21 16:59:34, tommi - chröme wrote:
> lots of public member variables - intentional? Maybe this should be a struct? 
> If not, then the variables should have a postfix underscore (and ideally be
> private).
> 
> Maybe some of the implementation that uses these variables should be moved
into
> the class?
> 
> Also, the convention is to have variables below methods.

Updated to be a style-guide compliant class.
Note that the original intent was to move this class from the .h to the .cc and
keep it as close to the original as possible. The change consisted in adding one
field (stream_controls) and renaming an existing one (generated -> state).

[tommi (sloooow) - chröme, 2017-03-21 19:07:29]

thanks, lgtm

[Guido Urdaneta, 2017-03-21 20:13:46]

The CQ bit was checked by guidou@chromium.org

[commit-bot: I haz the power, 2017-03-21 20:14:41]

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-03-21 21:42:51]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1490127226200220,
"parent_rev": "e345a1b1fa759b19df872d727b30d34727cc199d", "commit_rev":
"d727ed27c3dde3c85195625b97e79472eda73508"}

[commit-bot: I haz the power, 2017-03-21 21:43:40]

Description was changed from

==========
Serialize processing of getUserMedia() requests.

This is in anticipation to using the spec-compliant algorithm for
constraints processing.
The standard algorithm requires knowing the capabilities of all devices
before selecting the settings to be used. If two requests are processed
in parallel this can lead to races that would result in unexpected
errors. For example, there may be two requests that require the same
camera but prefer (though not require) different resolutions.
If the requests are handled in parallel and the camera has not been
opened yet, both requests will see that all resolutions are supported
and both will try to open the camera with different resolutions and one
of the requests will fail unxpectedly.
If the requests are handled sequentially, the camera will be opened with
the resolution selected by the first request, and the second request will
see that that resolution is the only available one. If that resolution
satisfies the constraints for the second request, the request will
succeed. Otherwise, it will fail in a spec-compliant way.

This CL also includes:
* A minor refactoring to expose fewer implementation details to the
  unit test.
* Removal of some unnecessary includes and logging statements.
* More consistent naming of variables. request_info for
  UserMediaRequestInfo and request or user_media_request for
  WebUserMediaRequest.

BUG=657733
==========

to

==========
Serialize processing of getUserMedia() requests.

This is in anticipation to using the spec-compliant algorithm for
constraints processing.
The standard algorithm requires knowing the capabilities of all devices
before selecting the settings to be used. If two requests are processed
in parallel this can lead to races that would result in unexpected
errors. For example, there may be two requests that require the same
camera but prefer (though not require) different resolutions.
If the requests are handled in parallel and the camera has not been
opened yet, both requests will see that all resolutions are supported
and both will try to open the camera with different resolutions and one
of the requests will fail unxpectedly.
If the requests are handled sequentially, the camera will be opened with
the resolution selected by the first request, and the second request will
see that that resolution is the only available one. If that resolution
satisfies the constraints for the second request, the request will
succeed. Otherwise, it will fail in a spec-compliant way.

This CL also includes:
* A minor refactoring to expose fewer implementation details to the
  unit test.
* Removal of some unnecessary includes and logging statements.
* More consistent naming of variables. request_info for
  UserMediaRequestInfo and request or user_media_request for
  WebUserMediaRequest.

BUG=657733

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

[commit-bot: I haz the power, 2017-03-21 21:43:41]

Committed patchset #2 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/d727ed27c3dde3c85195625b97e7...