Changed thread_checker on VideoCaptureImpl and VideoCaptureImplManager.

Changed thread_checker usage on VideoCaptureImpl and VideoCaptureImplManager to make it more explicit that the calls are run on the IO thread.

BUG=463009
TEST=Succesfully ran VideoCaptureImplTest.* and VideoCaptureImplManagerTest.* in content_unittests.

Committed: https://crrev.com/76a4b2c98512e8feed2b2d2ba40421496765d6bd
Cr-Commit-Position: refs/heads/master@{#322051}

[emircan, 2015-03-05 02:44:26]

emircan@chromium.org changed reviewers:
+ mcasas@chromium.org

[emircan, 2015-03-05 02:44:40]

PTAL.

[mcasas, 2015-03-06 19:18:29]

https://codereview.chromium.org/978993002/diff/1/content/renderer/media/video...
File content/renderer/media/video_capture_impl.h (right):

https://codereview.chromium.org/978993002/diff/1/content/renderer/media/video...
content/renderer/media/video_capture_impl.h:51: // All methods must be called on
the IO thread.
Comments for the class are at the file level. Owners might
say otherwise but I'd put those next to the class;

https://codereview.chromium.org/978993002/diff/1/content/renderer/media/video...
content/renderer/media/video_capture_impl.h:182:
scoped_refptr<base::MessageLoopProxy> io_message_loop_;
Make const.

https://codereview.chromium.org/978993002/diff/1/content/renderer/media/video...
File content/renderer/media/video_capture_impl_manager.h (right):

https://codereview.chromium.org/978993002/diff/1/content/renderer/media/video...
content/renderer/media/video_capture_impl_manager.h:44: // All methods must be
called on the IO thread.
The docs for the class are, oddly enough, on the beginning of
the file and state, correctly, that this class is created and
operates on Render Main thread (not Render IO thread). 

Please correct in consequence the naming of the member var
below, i.e.

s/io_message_loop_/main_render_message_loop_/

https://codereview.chromium.org/978993002/diff/1/content/renderer/media/video...
content/renderer/media/video_capture_impl_manager.h:126:
scoped_refptr<base::MessageLoopProxy> io_message_loop_;
Make const. That way, by reading the class declaration
I can quickly infer that is initialised in the ctor and
not touched anymore.

[emircan, 2015-03-10 23:38:36]

https://codereview.chromium.org/978993002/diff/1/content/renderer/media/video...
File content/renderer/media/video_capture_impl.h (right):

https://codereview.chromium.org/978993002/diff/1/content/renderer/media/video...
content/renderer/media/video_capture_impl.h:51: // All methods must be called on
the IO thread.
On 2015/03/06 19:18:29, mcasas wrote:
> Comments for the class are at the file level. Owners might
> say otherwise but I'd put those next to the class; 

I moved the existing comments to here instead.

https://codereview.chromium.org/978993002/diff/1/content/renderer/media/video...
content/renderer/media/video_capture_impl.h:182:
scoped_refptr<base::MessageLoopProxy> io_message_loop_;
On 2015/03/06 19:18:29, mcasas wrote:
> Make const.

This may be overwritten inside Init() method, see there for more info.

https://codereview.chromium.org/978993002/diff/1/content/renderer/media/video...
File content/renderer/media/video_capture_impl_manager.h (right):

https://codereview.chromium.org/978993002/diff/1/content/renderer/media/video...
content/renderer/media/video_capture_impl_manager.h:44: // All methods must be
called on the IO thread.
On 2015/03/06 19:18:29, mcasas wrote:
> The docs for the class are, oddly enough, on the beginning of
> the file and state, correctly, that this class is created and
> operates on Render Main thread (not Render IO thread). 
> 
> Please correct in consequence the naming of the member var
> below, i.e.
> 
> s/io_message_loop_/main_render_message_loop_/

Done.

https://codereview.chromium.org/978993002/diff/1/content/renderer/media/video...
content/renderer/media/video_capture_impl_manager.h:126:
scoped_refptr<base::MessageLoopProxy> io_message_loop_;
On 2015/03/06 19:18:29, mcasas wrote:
> Make const. That way, by reading the class declaration
> I can quickly infer that is initialised in the ctor and
> not touched anymore.

Done.

[mcasas, 2015-03-16 20:34:06]

LGTM modulo my comments.

(You still need an OWNER approval).

https://codereview.chromium.org/978993002/diff/40001/content/renderer/media/v...
File content/renderer/media/video_capture_impl.cc (right):

https://codereview.chromium.org/978993002/diff/40001/content/renderer/media/v...
content/renderer/media/video_capture_impl.cc:65: io_message_loop_ =
base::MessageLoopProxy::current();
My optimisation-seeker-me chills when seeing a member
variable declared and initialised yet potentially not
used (non-symbolic release builds).

Consider using #if DCHECK_IS_ON() or similar
to sandwich declaration and/or assignment.

https://codereview.chromium.org/978993002/diff/40001/content/renderer/media/v...
File content/renderer/media/video_capture_impl.h (right):

https://codereview.chromium.org/978993002/diff/40001/content/renderer/media/v...
content/renderer/media/video_capture_impl.h:173: // thread.
This reads strange. Suggestion:

"IO message loop reference for checking correct class operation."

https://codereview.chromium.org/978993002/diff/40001/content/renderer/media/v...
File content/renderer/media/video_capture_impl_manager.cc (right):

https://codereview.chromium.org/978993002/diff/40001/content/renderer/media/v...
content/renderer/media/video_capture_impl_manager.cc:155:
DCHECK(render_main_message_loop_->BelongsToCurrentThread());
Correct indent.

[emircan, 2015-03-19 00:40:44]

https://codereview.chromium.org/978993002/diff/40001/content/renderer/media/v...
File content/renderer/media/video_capture_impl.cc (right):

https://codereview.chromium.org/978993002/diff/40001/content/renderer/media/v...
content/renderer/media/video_capture_impl.cc:65: io_message_loop_ =
base::MessageLoopProxy::current();
On 2015/03/16 20:34:06, mcasas wrote:
> My optimisation-seeker-me chills when seeing a member
> variable declared and initialised yet potentially not
> used (non-symbolic release builds).
> 
> Consider using #if DCHECK_IS_ON() or similar
> to sandwich declaration and/or assignment.

I see there is no use for it in release builds. Thanks for pointing out.

Adding check on both declaration and assignment.

https://codereview.chromium.org/978993002/diff/40001/content/renderer/media/v...
File content/renderer/media/video_capture_impl.h (right):

https://codereview.chromium.org/978993002/diff/40001/content/renderer/media/v...
content/renderer/media/video_capture_impl.h:173: // thread.
On 2015/03/16 20:34:06, mcasas wrote:
> This reads strange. Suggestion:
> 
> "IO message loop reference for checking correct class operation."

Done.

https://codereview.chromium.org/978993002/diff/40001/content/renderer/media/v...
File content/renderer/media/video_capture_impl_manager.cc (right):

https://codereview.chromium.org/978993002/diff/40001/content/renderer/media/v...
content/renderer/media/video_capture_impl_manager.cc:155:
DCHECK(render_main_message_loop_->BelongsToCurrentThread());
On 2015/03/16 20:34:06, mcasas wrote:
> Correct indent.

Done.

[emircan, 2015-03-19 00:45:51]

emircan@chromium.org changed reviewers:
+ scherkus@chromium.org

[emircan, 2015-03-19 00:45:52]

scherkus@ please take a look. I need an owner review.

[scherkus (not reviewing), 2015-03-23 17:02:16]

lgtm

[mcasas, 2015-03-23 17:07:00]

The CQ bit was checked by mcasas@chromium.org

[mcasas, 2015-03-23 17:07:01]

The patchset sent to the CQ was uploaded after l-g-t-m from mcasas@chromium.org
Link to the patchset: https://codereview.chromium.org/978993002/#ps60001 (title:
"mcasas@ comments and DCHECK_IS_ON() checks added.")

[commit-bot: I haz the power, 2015-03-23 17:07:12]

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

[commit-bot: I haz the power, 2015-03-23 17:18:29]

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

[commit-bot: I haz the power, 2015-03-23 17:18:30]

Try jobs failed on following builders:
  android_chromium_gn_compile_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)

[emircan, 2015-03-23 20:43:00]

Patchset #5 (id:80001) has been deleted

[emircan, 2015-03-24 18:00:10]

The CQ bit was checked by emircan@chromium.org

[emircan, 2015-03-24 18:00:11]

The patchset sent to the CQ was uploaded after l-g-t-m from mcasas@chromium.org,
scherkus@chromium.org
Link to the patchset: https://codereview.chromium.org/978993002/#ps100001
(title: "DCHECK_IS_ON() fix.")

[commit-bot: I haz the power, 2015-03-24 18:00:37]

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

[commit-bot: I haz the power, 2015-03-24 19:50:01]

Committed patchset #5 (id:100001)

[commit-bot: I haz the power, 2015-03-24 19:50:38]

Patchset 5 (id:??) landed as
https://crrev.com/76a4b2c98512e8feed2b2d2ba40421496765d6bd
Cr-Commit-Position: refs/heads/master@{#322051}