Rename VideoCaptureImplManager to VideoCaptureService

Rename VideoCaptureImplManager to VideoCaptureService

Fix TODO(hclam):This class should be renamed to videoCaptureService in file
video_caputre_impl_manager.h.
Because of a refactoring, The interface of VideoCapture is removed and
VideoCaptureImpl is not the implementation of VideoCapture and
VideoCaptureImplManager also became an inaccurate and bulky name.

VideoCaptureImplManager provides a service to access video capture devices in
the renderer process. VideoCaptureImpl should be hidden away from clients of
the APIs. So VideoCaptureService is a better name. 

BUG=None

[qi1988.yang, 2015-02-26 07:24:33]

qi1988.yang@samsung.com changed reviewers:
+ sandersd@chromium.org

[qi1988.yang, 2015-02-26 07:24:33]

PTAL, thanks!

[Pawel Osciak, 2015-02-26 07:33:15]

posciak@chromium.org changed reviewers:
+ hclam@chromium.org

[sandersd (OOO until July 31), 2015-02-28 01:26:44]

I was assuming that I was intended to leave most of this review to Alpha, but as
it's been quiet so far I'll start if I don't hear something soon.

As a start, can you add to the CL description a motivation for the change, that
is, why VideoCaptureService is a better name than VideoCaptureImplManager?

[Alpha Left Google, 2015-02-28 01:38:42]

Please submit a try job to see if this builds.

Otherwise like sandersd@ said please update the CL description to say why
VideoCaptureService is better.

The reason I put the TODO is because of a refactoring I did to remove the
VideoCapture interface. Hence VideoCaptureImpl is not an implementation of any
interface, and VideoCaptureImplManager also became an inaccurate and bulky name.

What I see is that VideoCaptureImplManager provides a service to access a video
capture device in the renderer. And that the implementation, i.e.
VideoCaptureImpl should be hidden away from clients of the APIs. I concluded
that VideoCaptureService should be a better name.

Since you're doing this change it will be great if you can add your own
reasoning why VideoCaptureService is a better name. If you agree with my
reasoning you can also include some of what I mentioned above.

[qi1988.yang, 2015-02-28 02:43:09]

On 2015/02/28 01:38:42, Alpha wrote:
> Please submit a try job to see if this builds.
> 
> Otherwise like sandersd@ said please update the CL description to say why
> VideoCaptureService is better.
> 
> The reason I put the TODO is because of a refactoring I did to remove the
> VideoCapture interface. Hence VideoCaptureImpl is not an implementation of any
> interface, and VideoCaptureImplManager also became an inaccurate and bulky
name.
> 
> What I see is that VideoCaptureImplManager provides a service to access a
video
> capture device in the renderer. And that the implementation, i.e.
> VideoCaptureImpl should be hidden away from clients of the APIs. I concluded
> that VideoCaptureService should be a better name.
> 
> Since you're doing this change it will be great if you can add your own
> reasoning why VideoCaptureService is a better name. If you agree with my
> reasoning you can also include some of what I mentioned above.

I don't have the permission to do tryjobs. Could you help to do a try?

[qi1988.yang, 2015-03-03 01:55:03]

On 2015/02/28 02:43:09, qi1988.yang wrote:
> On 2015/02/28 01:38:42, Alpha wrote:
> > Please submit a try job to see if this builds.
> > 
> > Otherwise like sandersd@ said please update the CL description to say why
> > VideoCaptureService is better.
> > 
> > The reason I put the TODO is because of a refactoring I did to remove the
> > VideoCapture interface. Hence VideoCaptureImpl is not an implementation of
any
> > interface, and VideoCaptureImplManager also became an inaccurate and bulky
> name.
> > 
> > What I see is that VideoCaptureImplManager provides a service to access a
> video
> > capture device in the renderer. And that the implementation, i.e.
> > VideoCaptureImpl should be hidden away from clients of the APIs. I concluded
> > that VideoCaptureService should be a better name.
> > 
> > Since you're doing this change it will be great if you can add your own
> > reasoning why VideoCaptureService is a better name. If you agree with my
> > reasoning you can also include some of what I mentioned above.
> 
> I don't have the permission to do tryjobs. Could you help to do a try?

The trybot failed is caused by compiler proxy not working? It doesn't matter
with my patch. Please check, Thanks!

[Alpha Left Google, 2015-03-03 21:07:52]

LGTM. Still needs owner approval though.

[sandersd (OOO until July 31), 2015-03-03 21:11:03]

lgtm for content/renderer/media.

[qi1988.yang, 2015-03-04 01:48:37]

The CQ bit was checked by qi1988.yang@samsung.com

[commit-bot: I haz the power, 2015-03-04 01:49:56]

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

[commit-bot: I haz the power, 2015-03-04 01:55:57]

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

[commit-bot: I haz the power, 2015-03-04 01:55:59]

Try jobs failed on following builders:
  android_aosp on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_aosp/bu...)
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)

[qi1988.yang, 2015-03-04 02:00:45]

davidben@ please help to review renderer/ except of media, thanks!

[qi1988.yang, 2015-03-04 02:01:09]

qi1988.yang@samsung.com changed reviewers:
+ davidben@chromium.org

[mcasas, 2015-03-05 00:19:27]

On 2015/03/04 02:01:10, qi1988.yang wrote:

Drive-by. I might off page here... but when having the same 
"thing" on both sides of an IPC message pipe, it's customary
for one to be the "Host" and the other to be the "Impl". So
Impl doesn't mean "implements an interface", although it's
quite confusing.

I would say:VideoCaptureImpl -> VideoCaptureHost
But that already exists! :(

What about VideoCaptureImpl -> VideoCaptureService _and_
VideoCaptureImplManager -> VideoCaptureServiceManager ?

[qi1988.yang, 2015-03-06 02:38:54]

qi1988.yang@samsung.com changed reviewers:
+ sievers@chromium.org
- davidben@chromium.org

[no sievers, 2015-03-06 19:36:09]

https://codereview.chromium.org/960103002/diff/20001/content/renderer/media/v...
File content/renderer/media/video_capture_service.h (right):

https://codereview.chromium.org/960103002/diff/20001/content/renderer/media/v...
content/renderer/media/video_capture_service.h:18: #define
CONTENT_RENDERER_MEDIA_VIDEO_CAPTURE_IMPL_MANAGER_H_
change this

https://codereview.chromium.org/960103002/diff/20001/content/renderer/media/v...
content/renderer/media/video_capture_service.h:37: class VideoCaptureImpl;
drop this?

https://codereview.chromium.org/960103002/diff/20001/content/renderer/media/v...
content/renderer/media/video_capture_service.h:87: // Make all existing
VideoCaptureImpl instances stop/resume delivering
still a lot of references to *Impl everywhere here?

https://codereview.chromium.org/960103002/diff/20001/content/renderer/media/v...
content/renderer/media/video_capture_service.h:96: virtual VideoCaptureImpl*
CreateVideoCaptureImplForTesting(
what about the return types?

https://codereview.chromium.org/960103002/diff/20001/content/renderer/media/v...
content/renderer/media/video_capture_service.h:131: #endif  //
CONTENT_RENDERER_MEDIA_VIDEO_CAPTURE_IMPL_MANAGER_H_
change this