Handle early destruction of CanvasCaptureHandler

Handle early destruction of CanvasCaptureHandler

Fuzz testing showed that CanvasCaptureHandler can be destructed
earlier than CanvasCaptureHandler::VideoCapturerSource. Both instances
are owned by Blink side objects, and destruction sequence might be
different(oilpan).

CanvasCaptureHandler invalidates weakptrs in dtor() on main_render_thread.
We can check if weakptr is valid in StopCapture() that also runs
on main_render_thread.

BUG=597077
TEST=Added unittest "DestructHandler" to reproduce the fuzz case.

Committed: https://crrev.com/7dc969b13dd7747c575ee60d8e2e498da60d3ea8
Cr-Commit-Position: refs/heads/master@{#382966}

[emircan, 2016-03-23 01:51:32]

Description was changed from

==========
track fix

BUG=
==========

to

==========
Handle early destruction of CanvasCaptureHandler

Fuzz testing showed that CanvasCaptureHandler can be destructed 
earlier than CanvasCaptureHandler::VideoCapturerSource. Both instances
are owned by Blink side objects, and destruction sequence might be 
different(oilpan). 

CanvasCaptureHandler invalidates weakptrs in dtor() on main_render_thread.
We can check if weakptr is valid in StopCapture() that also runs
on main_render_thread.

BUG=597077
==========

[emircan, 2016-03-23 01:51:32]

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

[emircan, 2016-03-23 01:52:25]

PTAL.

[mcasas, 2016-03-23 17:54:30]

https://codereview.chromium.org/1829563002/diff/1/content/renderer/media/canv...
File content/renderer/media/canvas_capture_handler.cc (right):

https://codereview.chromium.org/1829563002/diff/1/content/renderer/media/canv...
content/renderer/media/canvas_capture_handler.cc:26: class
CanvasCaptureHandler::VideoCapturerSource
nit: could you add a comment saying that
this class is single threaded? If you take
my suggestion in the .h file, also this class
won't need to be CanvasCaptureHandler::

https://codereview.chromium.org/1829563002/diff/1/content/renderer/media/canv...
content/renderer/media/canvas_capture_handler.cc:66: double frame_rate_;
const? :)

https://codereview.chromium.org/1829563002/diff/1/content/renderer/media/canv...
content/renderer/media/canvas_capture_handler.cc:70: // destructed before
StopCapture() call.
nit: s/destructed/destroyed/ ?

https://codereview.chromium.org/1829563002/diff/1/content/renderer/media/canv...
File content/renderer/media/canvas_capture_handler.h (right):

https://codereview.chromium.org/1829563002/diff/1/content/renderer/media/canv...
content/renderer/media/canvas_capture_handler.h:78: class VideoCapturerSource;
I think this class can be just defined
in the .cc file since is manipulated
as a media::VideoCapturerSource.

[emircan, 2016-03-23 22:12:03]

https://codereview.chromium.org/1829563002/diff/1/content/renderer/media/canv...
File content/renderer/media/canvas_capture_handler.cc (right):

https://codereview.chromium.org/1829563002/diff/1/content/renderer/media/canv...
content/renderer/media/canvas_capture_handler.cc:26: class
CanvasCaptureHandler::VideoCapturerSource
On 2016/03/23 17:54:30, mcasas wrote:
> nit: could you add a comment saying that
> this class is single threaded? If you take
> my suggestion in the .h file, also this class
> won't need to be CanvasCaptureHandler::

Done.

https://codereview.chromium.org/1829563002/diff/1/content/renderer/media/canv...
content/renderer/media/canvas_capture_handler.cc:66: double frame_rate_;
On 2016/03/23 17:54:30, mcasas wrote:
> const? :)

Done :)

https://codereview.chromium.org/1829563002/diff/1/content/renderer/media/canv...
content/renderer/media/canvas_capture_handler.cc:70: // destructed before
StopCapture() call.
On 2016/03/23 17:54:30, mcasas wrote:
> nit: s/destructed/destroyed/ ?

Done.

https://codereview.chromium.org/1829563002/diff/1/content/renderer/media/canv...
File content/renderer/media/canvas_capture_handler.h (right):

https://codereview.chromium.org/1829563002/diff/1/content/renderer/media/canv...
content/renderer/media/canvas_capture_handler.h:78: class VideoCapturerSource;
On 2016/03/23 17:54:30, mcasas wrote:
> I think this class can be just defined
> in the .cc file since is manipulated
> as a media::VideoCapturerSource.

Done.

[mcasas, 2016-03-23 22:28:20]

LGTM - 

let's monitor the cluster fuzz before marking the bug as fixed.

[emircan, 2016-03-23 22:29:49]

Description was changed from

==========
Handle early destruction of CanvasCaptureHandler

Fuzz testing showed that CanvasCaptureHandler can be destructed 
earlier than CanvasCaptureHandler::VideoCapturerSource. Both instances
are owned by Blink side objects, and destruction sequence might be 
different(oilpan). 

CanvasCaptureHandler invalidates weakptrs in dtor() on main_render_thread.
We can check if weakptr is valid in StopCapture() that also runs
on main_render_thread.

BUG=597077
==========

to

==========
Handle early destruction of CanvasCaptureHandler

Fuzz testing showed that CanvasCaptureHandler can be destructed 
earlier than CanvasCaptureHandler::VideoCapturerSource. Both instances
are owned by Blink side objects, and destruction sequence might be 
different(oilpan). 

CanvasCaptureHandler invalidates weakptrs in dtor() on main_render_thread.
We can check if weakptr is valid in StopCapture() that also runs
on main_render_thread.

BUG=597077
TEST=Added unittest "DestructHandler" to reproduce the fuzz case.
==========

[emircan, 2016-03-23 23:06:19]

The CQ bit was checked by emircan@chromium.org

[commit-bot: I haz the power, 2016-03-23 23:06:51]

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

[commit-bot: I haz the power, 2016-03-23 23:12:55]

Description was changed from

==========
Handle early destruction of CanvasCaptureHandler

Fuzz testing showed that CanvasCaptureHandler can be destructed 
earlier than CanvasCaptureHandler::VideoCapturerSource. Both instances
are owned by Blink side objects, and destruction sequence might be 
different(oilpan). 

CanvasCaptureHandler invalidates weakptrs in dtor() on main_render_thread.
We can check if weakptr is valid in StopCapture() that also runs
on main_render_thread.

BUG=597077
TEST=Added unittest "DestructHandler" to reproduce the fuzz case.
==========

to

==========
Handle early destruction of CanvasCaptureHandler

Fuzz testing showed that CanvasCaptureHandler can be destructed 
earlier than CanvasCaptureHandler::VideoCapturerSource. Both instances
are owned by Blink side objects, and destruction sequence might be 
different(oilpan). 

CanvasCaptureHandler invalidates weakptrs in dtor() on main_render_thread.
We can check if weakptr is valid in StopCapture() that also runs
on main_render_thread.

BUG=597077
TEST=Added unittest "DestructHandler" to reproduce the fuzz case.
==========

[commit-bot: I haz the power, 2016-03-23 23:12:56]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-03-23 23:14:06]

Description was changed from

==========
Handle early destruction of CanvasCaptureHandler

Fuzz testing showed that CanvasCaptureHandler can be destructed 
earlier than CanvasCaptureHandler::VideoCapturerSource. Both instances
are owned by Blink side objects, and destruction sequence might be 
different(oilpan). 

CanvasCaptureHandler invalidates weakptrs in dtor() on main_render_thread.
We can check if weakptr is valid in StopCapture() that also runs
on main_render_thread.

BUG=597077
TEST=Added unittest "DestructHandler" to reproduce the fuzz case.
==========

to

==========
Handle early destruction of CanvasCaptureHandler

Fuzz testing showed that CanvasCaptureHandler can be destructed
earlier than CanvasCaptureHandler::VideoCapturerSource. Both instances
are owned by Blink side objects, and destruction sequence might be
different(oilpan).

CanvasCaptureHandler invalidates weakptrs in dtor() on main_render_thread.
We can check if weakptr is valid in StopCapture() that also runs
on main_render_thread.

BUG=597077
TEST=Added unittest "DestructHandler" to reproduce the fuzz case.

Committed: https://crrev.com/7dc969b13dd7747c575ee60d8e2e498da60d3ea8
Cr-Commit-Position: refs/heads/master@{#382966}
==========

[commit-bot: I haz the power, 2016-03-23 23:14:07]

Patchset 2 (id:??) landed as
https://crrev.com/7dc969b13dd7747c575ee60d8e2e498da60d3ea8
Cr-Commit-Position: refs/heads/master@{#382966}