[Mojo Video Capture] Do not instantiate in-process VideoCaptureSystem when using service

content/browser/renderer_host/media/in_process_video_capture_device_launcher.h

Index: content/browser/renderer_host/media/in_process_video_capture_device_launcher.h
diff --git a/content/browser/renderer_host/media/in_process_video_capture_device_launcher.h b/content/browser/renderer_host/media/in_process_video_capture_device_launcher.h
index 0a58ebab1dad4280a584b22fbb13eb6d13d0fa80..92ce7eb7c62ada48c217019cd7367a44c67ec185 100644
--- a/content/browser/renderer_host/media/in_process_video_capture_device_launcher.h
+++ b/content/browser/renderer_host/media/in_process_video_capture_device_launcher.h
@@ -23,7 +23,7 @@ class InProcessVideoCaptureDeviceLauncher : public VideoCaptureDeviceLauncher {
  public:
   InProcessVideoCaptureDeviceLauncher(
       scoped_refptr<base::SingleThreadTaskRunner> device_task_runner,
-      media::VideoCaptureSystem* video_capture_system);
+      media::VideoCaptureSystem* optional_video_capture_system);
   ~InProcessVideoCaptureDeviceLauncher() override;
 
   void LaunchDeviceAsync(const std::string& device_id,
@@ -72,7 +72,7 @@ class InProcessVideoCaptureDeviceLauncher : public VideoCaptureDeviceLauncher {
       ReceiveDeviceCallback result_callback);
 
   const scoped_refptr<base::SingleThreadTaskRunner> device_task_runner_;
-  media::VideoCaptureSystem* const video_capture_system_;
+  media::VideoCaptureSystem* const optional_video_capture_system_;

[mcasas, 2017/05/16 22:22:22]

I think having a naked pointer here is enough
to imply that it can be nullptr, so the prefix 
optional_ is unnecessary.  Also, if you really want
to convey the optionality, consider base::Optional
which in its uses [1] states:
[1]
https://chromium.googlesource.com/chromium/src/+/master/docs/optional.md#When...

[chfremer, 2017/05/16 22:45:02]

I disagree with the idea that the raw pointer would be enough to suggest it is
optional, since there are many cases where a raw pointer is used for
non-optional non-owned references.

I feel that the use of base::Optional<> is a bit heavyweight for raw pointer
types (since they already allow nullptr as a value). And I feel that using the
variable name conveys the optionality much more strongly than the type, because
the variable name appears explicitly at every usage site. After this
consideration, I still feel the proposed state is the best option. If you still
prefer one of the other two options, let me know and I'll make the change.

[mcasas, 2017/05/17 23:19:03]

A quick search [1] yields no |optional_| members in content/,
so I'd say, non-owned raw pointers should be checked for
non-nullness.  miu@ WDYT?


[1] grep -rn "optional_" content/ | grep "\.h"

[chfremer, 2017/05/18 17:06:16]

It appears that my reasoning for why I feel that adding the "optional_" prefix
is a good idea did not convince you, so I am removing the prefix.

The fact that there are no |optional_*| members in content/ does not change my
opinion that adding it here would have been a (minor) improvement, though.

   State state_;
 };