Chromium Code Reviews
Help | Chromium Project | Sign in
(907)

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 years, 5 months ago by willchan
Modified:
2 years, 11 months ago
Reviewers:
lzheng
CC:
chromium-reviews_chromium.org, 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) Lint Patch
M chrome/browser/safe_browsing/safe_browsing_service.cc View 2 chunks +1 line, -5 lines 2 comments ? errors Download
Commit:

Messages

Total messages: 5
willchan
3 years, 5 months ago #1
willchan
Um oops, I committed the wrong cl. Lei, it's a very simple change. Lemme know ...
3 years, 5 months ago #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>?
3 years, 5 months ago #3
willchan
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: > ...
3 years, 5 months ago #4
lzheng1
3 years, 5 months ago #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
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1275:d14800f88434