Rename MediaStreamVideoSource:GetCurrentFormatImpl() to GetCurrentFormat()

Rename MediaStreamVideoSource:GetCurrentFormatImpl() to GetCurrentFormat()

BUG=706408
Review-Url: https://codereview.chromium.org/2970823003
Cr-Commit-Position: refs/heads/master@{#484336}
Committed: https://chromium.googlesource.com/chromium/src/+/1bfef9a8c89ec3356cfc564d68f6ceffef08901b

[Guido Urdaneta, 2017-07-04 14:24:27]

Patchset #1 (id:1) has been deleted

[Guido Urdaneta, 2017-07-04 14:24:55]

Description was changed from

==========
Rename MediaStreamVideoSource:GetCurrentFormatImpl() to GetCurrentFormat()

BUG=706408
==========

to

==========
Rename MediaStreamVideoSource:GetCurrentFormatImpl() to GetCurrentFormat()

BUG=706408
==========

[Guido Urdaneta, 2017-07-04 14:24:55]

guidou@chromium.org changed reviewers:
+ bbudge@chromium.org, hbos@chromium.org

[Guido Urdaneta, 2017-07-04 14:25:26]

hbos@: please review content/renderer/media

bbudge@: please review content/renderer/pepper

[hbos_chromium, 2017-07-04 14:34:30]

lgtm % comments

https://codereview.chromium.org/2970823003/diff/20001/content/renderer/media/...
File content/renderer/media/media_stream_video_capturer_source.h (right):

https://codereview.chromium.org/2970823003/diff/20001/content/renderer/media/...
content/renderer/media/media_stream_video_capturer_source.h:54:
base::Optional<media::VideoCaptureFormat> GetCurrentFormat() const override;
nit: Shouldn't this be moved to the public section now?

https://codereview.chromium.org/2970823003/diff/20001/content/renderer/media/...
File content/renderer/media/mock_media_stream_video_source.cc (right):

https://codereview.chromium.org/2970823003/diff/20001/content/renderer/media/...
content/renderer/media/mock_media_stream_video_source.cc:71: return
base::Optional<media::VideoCaptureFormat>(format_);
We DCHECK with the sequence_checker_, it's protected.

https://codereview.chromium.org/2970823003/diff/20001/content/renderer/media/...
File content/renderer/media/mock_media_stream_video_source.h (right):

https://codereview.chromium.org/2970823003/diff/20001/content/renderer/media/...
content/renderer/media/mock_media_stream_video_source.h:60:
base::Optional<media::VideoCaptureFormat> GetCurrentFormat() const override;
public?

https://codereview.chromium.org/2970823003/diff/20001/content/renderer/pepper...
File content/renderer/pepper/pepper_media_stream_video_track_host.cc (right):

https://codereview.chromium.org/2970823003/diff/20001/content/renderer/pepper...
content/renderer/pepper/pepper_media_stream_video_track_host.cc:431:
base::Optional<media::VideoCaptureFormat> GetCurrentFormat() const override {
public?

[Guido Urdaneta, 2017-07-05 15:28:25]

https://codereview.chromium.org/2970823003/diff/20001/content/renderer/media/...
File content/renderer/media/media_stream_video_capturer_source.h (right):

https://codereview.chromium.org/2970823003/diff/20001/content/renderer/media/...
content/renderer/media/media_stream_video_capturer_source.h:54:
base::Optional<media::VideoCaptureFormat> GetCurrentFormat() const override;
On 2017/07/04 14:34:29, hbos_chromium wrote:
> nit: Shouldn't this be moved to the public section now?

no need.

https://codereview.chromium.org/2970823003/diff/20001/content/renderer/media/...
File content/renderer/media/mock_media_stream_video_source.cc (right):

https://codereview.chromium.org/2970823003/diff/20001/content/renderer/media/...
content/renderer/media/mock_media_stream_video_source.cc:71: return
base::Optional<media::VideoCaptureFormat>(format_);
On 2017/07/04 14:34:29, hbos_chromium wrote:
> We DCHECK with the sequence_checker_, it's protected.

There is no sequence_checker_ in this class. Did you mean something else?

https://codereview.chromium.org/2970823003/diff/20001/content/renderer/media/...
File content/renderer/media/mock_media_stream_video_source.h (right):

https://codereview.chromium.org/2970823003/diff/20001/content/renderer/media/...
content/renderer/media/mock_media_stream_video_source.h:60:
base::Optional<media::VideoCaptureFormat> GetCurrentFormat() const override;
On 2017/07/04 14:34:30, hbos_chromium wrote:
> public?

No need.

https://codereview.chromium.org/2970823003/diff/20001/content/renderer/pepper...
File content/renderer/pepper/pepper_media_stream_video_track_host.cc (right):

https://codereview.chromium.org/2970823003/diff/20001/content/renderer/pepper...
content/renderer/pepper/pepper_media_stream_video_track_host.cc:431:
base::Optional<media::VideoCaptureFormat> GetCurrentFormat() const override {
On 2017/07/04 14:34:30, hbos_chromium wrote:
> public?

no need.

[hbos_chromium, 2017-07-05 15:33:30]

https://codereview.chromium.org/2970823003/diff/20001/content/renderer/media/...
File content/renderer/media/mock_media_stream_video_source.cc (right):

https://codereview.chromium.org/2970823003/diff/20001/content/renderer/media/...
content/renderer/media/mock_media_stream_video_source.cc:71: return
base::Optional<media::VideoCaptureFormat>(format_);
On 2017/07/05 15:28:25, Guido Urdaneta wrote:
> On 2017/07/04 14:34:29, hbos_chromium wrote:
> > We DCHECK with the sequence_checker_, it's protected.
> 
> There is no sequence_checker_ in this class. Did you mean something else?

There's SEQUENCE_CHECKER(sequence_checker_) in MediaStreamVideoSource's
protected section.
https://cs.chromium.org/chromium/src/content/renderer/media/media_stream_vide...

[Guido Urdaneta, 2017-07-05 16:03:46]

https://codereview.chromium.org/2970823003/diff/20001/content/renderer/media/...
File content/renderer/media/mock_media_stream_video_source.cc (right):

https://codereview.chromium.org/2970823003/diff/20001/content/renderer/media/...
content/renderer/media/mock_media_stream_video_source.cc:71: return
base::Optional<media::VideoCaptureFormat>(format_);
On 2017/07/05 15:33:30, hbos_chromium wrote:
> On 2017/07/05 15:28:25, Guido Urdaneta wrote:
> > On 2017/07/04 14:34:29, hbos_chromium wrote:
> > > We DCHECK with the sequence_checker_, it's protected.
> > 
> > There is no sequence_checker_ in this class. Did you mean something else?
> 
> There's SEQUENCE_CHECKER(sequence_checker_) in MediaStreamVideoSource's
> protected section.
>
https://cs.chromium.org/chromium/src/content/renderer/media/media_stream_vide...

Done. Also filed crbug.com/739393 to add sequence checks to MSVCS and its
internal classes.

[bbudge, 2017-07-05 16:56:43]

lgtm

[Guido Urdaneta, 2017-07-05 17:34:15]

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

[commit-bot: I haz the power, 2017-07-05 17:34:39]

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-07-05 19:14:43]

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

[commit-bot: I haz the power, 2017-07-05 19:14:44]

Dry run: This issue passed the CQ dry run.

[Guido Urdaneta, 2017-07-05 19:58:04]

The CQ bit was checked by guidou@chromium.org

[Guido Urdaneta, 2017-07-05 19:58:04]

The patchset sent to the CQ was uploaded after l-g-t-m from hbos@chromium.org
Link to the patchset: https://codereview.chromium.org/2970823003/#ps60001
(title: "add DCHECK")

[commit-bot: I haz the power, 2017-07-05 19:58:14]

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-07-05 20:01:50]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1499284684080480,
"parent_rev": "c6719b3aa128592368ee53536855c22d1bbbc116", "commit_rev":
"1bfef9a8c89ec3356cfc564d68f6ceffef08901b"}

[commit-bot: I haz the power, 2017-07-05 20:02:05]

Description was changed from

==========
Rename MediaStreamVideoSource:GetCurrentFormatImpl() to GetCurrentFormat()

BUG=706408
==========

to

==========
Rename MediaStreamVideoSource:GetCurrentFormatImpl() to GetCurrentFormat()

BUG=706408

Review-Url: https://codereview.chromium.org/2970823003
Cr-Commit-Position: refs/heads/master@{#484336}
Committed:
https://chromium.googlesource.com/chromium/src/+/1bfef9a8c89ec3356cfc564d68f6...
==========

[commit-bot: I haz the power, 2017-07-05 20:02:07]

Committed patchset #3 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/1bfef9a8c89ec3356cfc564d68f6...