Add PepperViewProxy to protect PepperView and ChromotingInstance on shutdown

Add PepperViewProxy to protect PepperView and ChromotingInstance on shutdown

Adding a refcounted PepperViewProxy so that we can detach PepperView when
ChromotingInstance is destroyed. PepperViewProxy also performs the task of
thread delegation for PepperView so we can assume PepperView is used only on
pepper thread.

BUG=65696
TEST=None

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=72568

[Alpha Left Google, 2011-01-25 18:40:12]

+sergeyu

[Sergey Ulanov, 2011-01-25 20:38:44]

Why can't we just make PepperView ref-counted?

[Sergey Ulanov, 2011-01-25 21:29:36]

http://codereview.chromium.org/6359010/diff/2001/remoting/client/plugin/chrom...
File remoting/client/plugin/chromoting_instance.cc (right):

http://codereview.chromium.org/6359010/diff/2001/remoting/client/plugin/chrom...
remoting/client/plugin/chromoting_instance.cc:125:
view_->SetViewport(position.x(), position.y(),
view_proxy_?

http://codereview.chromium.org/6359010/diff/2001/remoting/client/plugin/peppe...
File remoting/client/plugin/pepper_view.cc (right):

http://codereview.chromium.org/6359010/diff/2001/remoting/client/plugin/peppe...
remoting/client/plugin/pepper_view.cc:68: NewTracedMethod(this,
&PepperView::OnPaintDone)));
Do we have any guarantees that this task is not called after this object is
destroyed?
Use ScopedRunnableMethodFactory to create tasks here.

http://codereview.chromium.org/6359010/diff/2001/remoting/client/plugin/peppe...
remoting/client/plugin/pepper_view.cc:110: NewTracedMethod(this,
&PepperView::OnPaintDone)));
same here

http://codereview.chromium.org/6359010/diff/2001/remoting/client/plugin/peppe...
File remoting/client/plugin/pepper_view.h (right):

http://codereview.chromium.org/6359010/diff/2001/remoting/client/plugin/peppe...
remoting/client/plugin/pepper_view.h:83:
DISABLE_RUNNABLE_METHOD_REFCOUNT(remoting::PepperView);
Remove this.

http://codereview.chromium.org/6359010/diff/2001/remoting/client/plugin/peppe...
File remoting/client/plugin/pepper_view_proxy.cc (right):

http://codereview.chromium.org/6359010/diff/2001/remoting/client/plugin/peppe...
remoting/client/plugin/pepper_view_proxy.cc:1: // Copyright (c) 2010 The
Chromium Authors. All rights reserved.
2011

http://codereview.chromium.org/6359010/diff/2001/remoting/client/plugin/peppe...
File remoting/client/plugin/pepper_view_proxy.h (right):

http://codereview.chromium.org/6359010/diff/2001/remoting/client/plugin/peppe...
remoting/client/plugin/pepper_view_proxy.h:1: // Copyright (c) 2010 The Chromium
Authors. All rights reserved.
2011

[Alpha Left Google, 2011-01-25 22:20:46]

http://codereview.chromium.org/6359010/diff/2001/remoting/client/plugin/chrom...
File remoting/client/plugin/chromoting_instance.cc (right):

http://codereview.chromium.org/6359010/diff/2001/remoting/client/plugin/chrom...
remoting/client/plugin/chromoting_instance.cc:125:
view_->SetViewport(position.x(), position.y(),
On 2011/01/25 21:29:37, sergeyu wrote:
> view_proxy_?

This is called on pepper thread so this is safe.

http://codereview.chromium.org/6359010/diff/2001/remoting/client/plugin/peppe...
File remoting/client/plugin/pepper_view.cc (right):

http://codereview.chromium.org/6359010/diff/2001/remoting/client/plugin/peppe...
remoting/client/plugin/pepper_view.cc:68: NewTracedMethod(this,
&PepperView::OnPaintDone)));
On 2011/01/25 21:29:37, sergeyu wrote:
> Do we have any guarantees that this task is not called after this object is
> destroyed?
> Use ScopedRunnableMethodFactory to create tasks here.

Done.

http://codereview.chromium.org/6359010/diff/2001/remoting/client/plugin/peppe...
remoting/client/plugin/pepper_view.cc:110: NewTracedMethod(this,
&PepperView::OnPaintDone)));
On 2011/01/25 21:29:37, sergeyu wrote:
> same here

Done.

http://codereview.chromium.org/6359010/diff/2001/remoting/client/plugin/peppe...
File remoting/client/plugin/pepper_view.h (right):

http://codereview.chromium.org/6359010/diff/2001/remoting/client/plugin/peppe...
remoting/client/plugin/pepper_view.h:83:
DISABLE_RUNNABLE_METHOD_REFCOUNT(remoting::PepperView);
On 2011/01/25 21:29:37, sergeyu wrote:
> Remove this.

Done.

http://codereview.chromium.org/6359010/diff/2001/remoting/client/plugin/peppe...
File remoting/client/plugin/pepper_view_proxy.cc (right):

http://codereview.chromium.org/6359010/diff/2001/remoting/client/plugin/peppe...
remoting/client/plugin/pepper_view_proxy.cc:1: // Copyright (c) 2010 The
Chromium Authors. All rights reserved.
On 2011/01/25 21:29:37, sergeyu wrote:
> 2011

Done.

http://codereview.chromium.org/6359010/diff/2001/remoting/client/plugin/peppe...
File remoting/client/plugin/pepper_view_proxy.h (right):

http://codereview.chromium.org/6359010/diff/2001/remoting/client/plugin/peppe...
remoting/client/plugin/pepper_view_proxy.h:1: // Copyright (c) 2010 The Chromium
Authors. All rights reserved.
On 2011/01/25 21:29:37, sergeyu wrote:
> 2011

Done.

[Sergey Ulanov, 2011-01-25 22:26:53]

LGTM, after |control_reader_| is removed from JingleConnectionToHost.

http://codereview.chromium.org/6359010/diff/13001/remoting/protocol/jingle_co...
File remoting/protocol/jingle_connection_to_host.cc (right):

http://codereview.chromium.org/6359010/diff/13001/remoting/protocol/jingle_co...
remoting/protocol/jingle_connection_to_host.cc:25: control_reader_(new
MessageReader()),
Just remove control_reader_

http://codereview.chromium.org/6359010/diff/13001/remoting/protocol/jingle_co...
File remoting/protocol/jingle_connection_to_host.h (right):

http://codereview.chromium.org/6359010/diff/13001/remoting/protocol/jingle_co...
remoting/protocol/jingle_connection_to_host.h:94: scoped_refptr<MessageReader>
control_reader_;
This was replaced with dispatcher_, so we don't need control_reader_ anymore, it
can be removed. Thanks for fixing it.