Initialize cc::VideoFrameProviderClientImpl::active_video_layer_ to NULL.

Initialize cc::VideoFrameProviderClientImpl::active_video_layer_.

As of r259048, WebMediaPlayerImpl now notifies the compositor of new
frames directly on the compositing thread. If
VideoFrameProviderClientImpl::DidRecieveFrame() is called before
VideoLayerImpl::DidBecomeActive() is called,
VideoFrameProviderClientImpl may attempt to call
VideoLayerImpl::SetNeedsRedraw() using an uninitialized pointer.

BUG=357145
R=enne@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=260360

[scherkus (not reviewing), 2014-03-27 20:30:30]

few things:
  1) not sure how useful the test is ... it's a bit contrived
  2) not 100% sure if the ordering of events (DidReceiveFrame() being called
before DidBecomeActive()) is possible in production as I haven't been able to
reproduce the crash locally
  3) however this *would* explain the strange crash dumps I've investigated
where active_video_layer_ appears to be garbage

[scherkus (not reviewing), 2014-03-28 20:39:57]

danakj: ping!

[enne (OOO), 2014-03-28 21:01:34]

lgtm

I don't like "this shouldn't crash" style tests.  What about having the test
verify that the active video layer is NULL beforehand and not NULL after?

[enne (OOO), 2014-03-28 21:01:56]

Also, please format your change description to 72 columns max.  :)

[scherkus (not reviewing), 2014-03-28 21:36:41]

agree w.r.t. crash tests

PTAL at the new test

[scherkus (not reviewing), 2014-03-28 21:37:29]

https://codereview.chromium.org/214543011/diff/40001/cc/layers/video_layer_im...
File cc/layers/video_layer_impl_unittest.cc (right):

https://codereview.chromium.org/214543011/diff/40001/cc/layers/video_layer_im...
cc/layers/video_layer_impl_unittest.cc:94:
EXPECT_TRUE(client->active_video_layer());
this should probably be an EXPECT_EQ()

[enne (OOO), 2014-03-28 21:39:35]

Test changes lgtm, thanks!

(Also, sorry to be pedantic about change descriptions, but the first line of the
description gets used in git tools like shortlog or other things.  So, maybe
don't wrap that even if long? Or ideally make it shorter?  I backseat edited it
for you, but just saying.)

https://codereview.chromium.org/214543011/diff/40001/cc/layers/video_layer_im...
File cc/layers/video_layer_impl_unittest.cc (right):

https://codereview.chromium.org/214543011/diff/40001/cc/layers/video_layer_im...
cc/layers/video_layer_impl_unittest.cc:94:
EXPECT_TRUE(client->active_video_layer());
On 2014/03/28 21:37:29, scherkus wrote:
> this should probably be an EXPECT_EQ()

If it fails, then you know it's NULL.  So, maybe not all that useful.

[scherkus (not reviewing), 2014-03-28 21:50:28]

On 2014/03/28 21:39:35, enne wrote:
> Test changes lgtm, thanks!
> 
> (Also, sorry to be pedantic about change descriptions, but the first line of
the
> description gets used in git tools like shortlog or other things.  So, maybe
> don't wrap that even if long? Or ideally make it shorter?  I backseat edited
it
> for you, but just saying.)

hear you loud and clear and thanks for the edit!

(I too like to have the summary fit on a single line but didn't notice that vim
wrapped the ending of it to a new line)
 
>
https://codereview.chromium.org/214543011/diff/40001/cc/layers/video_layer_im...
> File cc/layers/video_layer_impl_unittest.cc (right):
> 
>
https://codereview.chromium.org/214543011/diff/40001/cc/layers/video_layer_im...
> cc/layers/video_layer_impl_unittest.cc:94:
> EXPECT_TRUE(client->active_video_layer());
> On 2014/03/28 21:37:29, scherkus wrote:
> > this should probably be an EXPECT_EQ()
> 
> If it fails, then you know it's NULL.  So, maybe not all that useful.

[scherkus (not reviewing), 2014-03-28 21:50:38]

The CQ bit was checked by scherkus@chromium.org

[commit-bot: I haz the power, 2014-03-28 21:53:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/scherkus@chromium.org/214543011/60001

[commit-bot: I haz the power, 2014-03-28 22:23:46]

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

[commit-bot: I haz the power, 2014-03-28 22:23:47]

Try jobs failed on following builders:
  tryserver.chromium on win_chromium_compile_dbg

[scherkus (not reviewing), 2014-03-28 23:13:38]

The CQ bit was checked by scherkus@chromium.org

[commit-bot: I haz the power, 2014-03-28 23:14:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/scherkus@chromium.org/214543011/60001

[commit-bot: I haz the power, 2014-03-29 00:11:23]

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

[commit-bot: I haz the power, 2014-03-29 00:11:24]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_chromeos_rel

[enne (OOO), 2014-03-29 00:12:17]

The CQ bit was checked by enne@chromium.org

[commit-bot: I haz the power, 2014-03-29 00:16:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/scherkus@chromium.org/214543011/60001

[commit-bot: I haz the power, 2014-03-29 01:51:00]

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

[commit-bot: I haz the power, 2014-03-29 01:51:00]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_chromeos_rel

[scherkus (not reviewing), 2014-03-29 02:28:40]

The CQ bit was checked by scherkus@chromium.org

[commit-bot: I haz the power, 2014-03-29 02:28:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/scherkus@chromium.org/214543011/60001

[commit-bot: I haz the power, 2014-03-29 04:08:47]

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

[commit-bot: I haz the power, 2014-03-29 04:08:47]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_chromeos_rel

[scherkus (not reviewing), 2014-03-29 04:27:41]

Committed patchset #4 manually as r260360 (presubmit successful).