Issue 3582002: Add deletable file refs to Blobs

Issue 3582002: Add deletable file refs to Blobs (Closed)

Created:
10 years, 2 months ago by michaeln

Modified:
9 years, 7 months ago

Reviewers:
jianli

CC:
chromium-reviews, darin-cc_chromium.org, dpranke+watch_chromium.org, pam+watch_chromium.org

Base URL:
svn://chrome-svn/chrome/trunk/src/

Visibility:
Public.

Description

Add deletable file refs to Blobs BUG=52486 TEST=manual Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=62485

Patch Set 4 : '' #

Created: 10 years, 2 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+84 lines, -51 lines)			Patch
M	webkit/blob/blob_data.h	View	1 2 3	4 chunks	+13 lines, -6 lines	0 comments	Download
M	webkit/blob/blob_storage_controller.h	View		2 chunks	+7 lines, -0 lines	0 comments	Download
M	webkit/blob/blob_storage_controller.cc	View	1 2 3	3 chunks	+56 lines, -40 lines	0 comments	Download
M	webkit/tools/test_shell/test_shell_webblobregistry_impl.cc	View	1 2 3	2 chunks	+8 lines, -5 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

jianli

LGTM. Except several minor problems. http://codereview.chromium.org/3582002/diff/5001/6001 File webkit/blob/blob_data.h (right): http://codereview.chromium.org/3582002/diff/5001/6001#newcode142 webkit/blob/blob_data.h:142: } Please add an ...

10 years, 2 months ago (2010-10-07 22:49:45 UTC) #2

michaeln

10 years, 2 months ago (2010-10-08 19:54:34 UTC) #3

On 2010/10/07 22:49:45, jianli wrote:
> LGTM. Except several minor problems.
> 
> http://codereview.chromium.org/3582002/diff/5001/6001
> File webkit/blob/blob_data.h (right):
> 
> http://codereview.chromium.org/3582002/diff/5001/6001#newcode142
> webkit/blob/blob_data.h:142: }
> Please add an empty line after that.

Done

> 
> http://codereview.chromium.org/3582002/diff/5001/6002
> File webkit/blob/blob_storage_controller.cc (right):
> 
> http://codereview.chromium.org/3582002/diff/5001/6002#newcode52
> webkit/blob/blob_storage_controller.cc:52: // no longer needed. If so, there
> will be a deletable reference for it.
> The comment is a bit complicated. How about something simpler like:
>   // If this is a temporary file that should be deleted when it's no longer
> needed, there will be a deletable reference for it.

Done

> http://codereview.chromium.org/3582002/diff/5001/6002#newcode98
> webkit/blob/blob_storage_controller.cc:98:
> target_blob_data->AttachDeletableFileReference(deletable_file);
> Can we have a helper function for this to avoid the duplicate code?

Done

Expand Messages | Collapse Messages