Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(13)

Issue 4053006: Fix remoting to use scoped_refptr. (Closed)

Created:
10 years, 2 months ago by willchan no longer on Chromium
Modified:
9 years, 7 months ago
Reviewers:
Nico, awong
CC:
chromium-reviews, Sergey Ulanov, dmac, awong, garykac, Alpha Left Google
Visibility:
Public.

Description

Fix remoting to use scoped_refptr. BUG=28083 TEST=existing Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=63603

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+3 lines, -3 lines) Patch
M remoting/client/plugin/pepper_view.cc View 1 chunk +3 lines, -3 lines 1 comment Download

Messages

Total messages: 4 (0 generated)
willchan no longer on Chromium
10 years, 2 months ago (2010-10-22 23:46:16 UTC) #1
awong
LGTM On Fri, Oct 22, 2010 at 4:46 PM, <willchan@chromium.org> wrote: > Reviewers: awong, > ...
10 years, 2 months ago (2010-10-22 23:54:15 UTC) #2
Nico
drive-by question http://codereview.chromium.org/4053006/diff/1/2 File remoting/client/plugin/pepper_view.cc (right): http://codereview.chromium.org/4053006/diff/1/2#newcode224 remoting/client/plugin/pepper_view.cc:224: NewTracedMethod(this, &PepperView::OnPartialFrameOutput, Hi Will, OnPartialFrameOutput takes a ...
10 years, 2 months ago (2010-10-24 19:51:38 UTC) #3
willchan no longer on Chromium
10 years, 2 months ago (2010-10-24 20:02:37 UTC) #4
On Sun, Oct 24, 2010 at 12:51 PM, <thakis@chromium.org> wrote:

> drive-by question
>
>
> http://codereview.chromium.org/4053006/diff/1/2
> File remoting/client/plugin/pepper_view.cc (right):
>
> http://codereview.chromium.org/4053006/diff/1/2#newcode224
> remoting/client/plugin/pepper_view.cc:224: NewTracedMethod(this,
> &PepperView::OnPartialFrameOutput,
> Hi Will,
>
> OnPartialFrameOutput takes a raw |frame| pointer, not a scoped_refptr.
> The bug is only about adding make_scoped_refptr if the function does
> take a scoped_refptr. Should OnPartialFrameOutput's parameter be made a
> scoped_refptr?
>
> Nico


Without going into too much detail (read the bug thread for more), this is
not important.  What's important is what is passed into NewRunnableMethod()
and friends, since that will decide the Tuple type created for the Params.
 And we need the Tuple to have a scoped_refptr instead of a raw pointer.
 The function prototype doesn't matter.  The Tuple will only be deleted
after Task::Run() is invoked, so the lifetime is guaranteed during the
method invocation.


>
>
> http://codereview.chromium.org/4053006/show
>

Powered by Google App Engine
This is Rietveld 408576698