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

Issue 3951001: Use scoped_refptr for refcounted param in SafeBrowsingService. (Closed)

Created:
10 years, 2 months ago by willchan no longer on Chromium
Modified:
9 years, 7 months ago
Reviewers:
lzheng
CC:
chromium-reviews, ben+cc_chromium.org
Visibility:
Public.

Description

Use scoped_refptr for refcounted param in SafeBrowsingService. BUG=28083 TEST=none Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=63215

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+1 line, -5 lines) Patch
M chrome/browser/safe_browsing/safe_browsing_service.cc View 2 chunks +1 line, -5 lines 2 comments Download

Messages

Total messages: 5 (0 generated)
willchan no longer on Chromium
10 years, 2 months ago (2010-10-20 06:47:47 UTC) #1
willchan no longer on Chromium
Um oops, I committed the wrong cl. Lei, it's a very simple change. Lemme know ...
10 years, 2 months ago (2010-10-20 16:19:24 UTC) #2
lzheng
http://codereview.chromium.org/3951001/diff/1/2 File chrome/browser/safe_browsing/safe_browsing_service.cc (right): http://codereview.chromium.org/3951001/diff/1/2#newcode349 chrome/browser/safe_browsing/safe_browsing_service.cc:349: URLRequestContextGetter* request_context_getter) { Should this become scoped_refptr<URLRequestContextGetter>?
10 years, 2 months ago (2010-10-20 18:03:55 UTC) #3
willchan no longer on Chromium
http://codereview.chromium.org/3951001/diff/1/2 File chrome/browser/safe_browsing/safe_browsing_service.cc (right): http://codereview.chromium.org/3951001/diff/1/2#newcode349 chrome/browser/safe_browsing/safe_browsing_service.cc:349: URLRequestContextGetter* request_context_getter) { On 2010/10/20 18:03:55, lzheng wrote: > ...
10 years, 2 months ago (2010-10-20 18:06:49 UTC) #4
lzheng1
10 years, 2 months ago (2010-10-20 19:00:44 UTC) #5
LGTM

On Wed, Oct 20, 2010 at 11:06 AM, <willchan@chromium.org> wrote:

>
> http://codereview.chromium.org/3951001/diff/1/2
> File chrome/browser/safe_browsing/safe_browsing_service.cc (right):
>
> http://codereview.chromium.org/3951001/diff/1/2#newcode349
> chrome/browser/safe_browsing/safe_browsing_service.cc:349:
> URLRequestContextGetter* request_context_getter) {
> On 2010/10/20 18:03:55, lzheng wrote:
>
>> Should this become scoped_refptr<URLRequestContextGetter>?
>>
>
> Not necessary, and there was a chromium-dev discussion where the
> majority of people prefer that it is passed as a raw pointer.  The
> scoped_refptr is just to make sure that the RunnableMethod task will
> have a Tuple that uses a scoped_refptr, so the parameter will be
> guaranteed to be valid for the lifetime of this function call.
>
> It's sort of complicated.  Read the bug thread for details on the nature
> of the bug.
>
>
> http://codereview.chromium.org/3951001/show
>

Thanks for the pointer. So, the cast to T* happens during callback time
which makes the code safe.

Lei

Powered by Google App Engine
This is Rietveld 408576698