Cast: make global_instance_ more strict

make global_instance_ more strict
Fix a bug where global_instance_ was not set to zero.
Fix MockRenderThread to call OnChannelClosing properly.

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

[hubbe, 2014-02-18 23:58:25]

As requested, change global_instance_ = NULL to DCHECK(!global_instance_)

[acolwell GONE FROM CHROMIUM, 2014-02-19 00:11:24]

https://codereview.chromium.org/169063003/diff/1/chrome/renderer/media/cast_i...
File chrome/renderer/media/cast_ipc_dispatcher.cc (right):

https://codereview.chromium.org/169063003/diff/1/chrome/renderer/media/cast_i...
chrome/renderer/media/cast_ipc_dispatcher.cc:76: global_instance_ = NULL;
Doesn't the DCHECK in OnFilterRemoved() fire if you add this since the
ChannelProxy::Context::OnChannelClosed() calls that method immediately after
this method.
(https://code.google.com/p/chromium/codesearch#chromium/src/ipc/ipc_channel_pr...)

https://codereview.chromium.org/169063003/diff/1/content/public/test/mock_ren...
File content/public/test/mock_render_thread.cc (right):

https://codereview.chromium.org/169063003/diff/1/content/public/test/mock_ren...
content/public/test/mock_render_thread.cc:32: filter->OnChannelClosing();
I think this should also include filter->OnFilterRemoved(). I think we should be
emulating the ChannelProxy::Context::OnChannelClosed() code here
https://code.google.com/p/chromium/codesearch#chromium/src/ipc/ipc_channel_pr...

I'm on the fence about the OnChannelClosing() call you've added here though
because this class does not appear to be emulating the OnChannelConnected() call
so I'm not sure of the OnChannelClosing() call makes sense here.

[hubbe, 2014-02-19 00:29:51]

PTAL

https://codereview.chromium.org/169063003/diff/1/chrome/renderer/media/cast_i...
File chrome/renderer/media/cast_ipc_dispatcher.cc (right):

https://codereview.chromium.org/169063003/diff/1/chrome/renderer/media/cast_i...
chrome/renderer/media/cast_ipc_dispatcher.cc:76: global_instance_ = NULL;
On 2014/02/19 00:11:24, acolwell wrote:
> Doesn't the DCHECK in OnFilterRemoved() fire if you add this since the
> ChannelProxy::Context::OnChannelClosed() calls that method immediately after
> this method.
>
(https://code.google.com/p/chromium/codesearch#chromium/src/ipc/ipc_channel_pr...)

You're right. Fixed.

https://codereview.chromium.org/169063003/diff/1/content/public/test/mock_ren...
File content/public/test/mock_render_thread.cc (right):

https://codereview.chromium.org/169063003/diff/1/content/public/test/mock_ren...
content/public/test/mock_render_thread.cc:32: filter->OnChannelClosing();
On 2014/02/19 00:11:24, acolwell wrote:
> I think this should also include filter->OnFilterRemoved(). I think we should
be
> emulating the ChannelProxy::Context::OnChannelClosed() code here
>
https://code.google.com/p/chromium/codesearch#chromium/src/ipc/ipc_channel_pr...
> 
> I'm on the fence about the OnChannelClosing() call you've added here though
> because this class does not appear to be emulating the OnChannelConnected()
call
> so I'm not sure of the OnChannelClosing() call makes sense here.

Better?

[acolwell GONE FROM CHROMIUM, 2014-02-19 00:49:36]

lgtm

[hubbe, 2014-02-19 00:56:02]

Adding sky for MockRenderThread ownership.

[sky, 2014-02-19 01:02:51]

LGTM

[hubbe, 2014-02-19 02:44:31]

The CQ bit was checked by hubbe@chromium.org

[commit-bot: I haz the power, 2014-02-19 02:45:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hubbe@chromium.org/169063003/70001

[commit-bot: I haz the power, 2014-02-19 04:08:18]

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

[commit-bot: I haz the power, 2014-02-19 04:08:19]

Retried try job too often on win_rel for step(s) remoting_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[hubbe, 2014-02-19 19:39:19]

The CQ bit was checked by hubbe@chromium.org

[commit-bot: I haz the power, 2014-02-19 19:41:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hubbe@chromium.org/169063003/70001

[commit-bot: I haz the power, 2014-02-19 20:58:33]

Change committed as 252078