Issue 7054012: Notify read access to the QuotaManager in FileSystemOperation

Issue 7054012: Notify read access to the QuotaManager in FileSystemOperation (Closed)

Created:
9 years, 7 months ago by kinuko

Modified:
9 years, 7 months ago

Reviewers:
ericu, oshima, Dai Mikurube (NOT FULLTIME)

CC:
chromium-reviews, kinuko+watch, darin-cc_chromium.org, Paweł Hajdan Jr.

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

Description

Notify read access to the QuotaManager in FileSystemOperation BUG=none TEST=FileSystemOperationTest.* Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=86400

Patch Set 2 : '' #

Total comments: 5

Created: 9 years, 7 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+153 lines, -49 lines)			Patch
M	webkit/fileapi/file_system_operation.cc	View	1	4 chunks	+13 lines, -4 lines	2 comments	Download
M	webkit/fileapi/file_system_operation_unittest.cc	View	1	23 chunks	+135 lines, -45 lines	3 comments	Download
M	webkit/fileapi/file_system_test_helper.h	View		2 chunks	+5 lines, -0 lines	0 comments	Download

Messages

Total messages: 7 (0 generated)

Expand Messages | Collapse Messages

Dai Mikurube (NOT FULLTIME)

LGTM if the Linux try succeeds. (It looks like not related to this patch.)

9 years, 7 months ago (2011-05-23 11:28:27 UTC) #2

ericu

LGTM http://codereview.chromium.org/7054012/diff/5001/webkit/fileapi/file_system_operation.cc File webkit/fileapi/file_system_operation.cc (left): http://codereview.chromium.org/7054012/diff/5001/webkit/fileapi/file_system_operation.cc#oldcode39 webkit/fileapi/file_system_operation.cc:39: // TODO(dmikurube): Read and set available bytes from ...

9 years, 7 months ago (2011-05-24 03:03:56 UTC) #3

kinuko

http://codereview.chromium.org/7054012/diff/5001/webkit/fileapi/file_system_operation.cc File webkit/fileapi/file_system_operation.cc (left): http://codereview.chromium.org/7054012/diff/5001/webkit/fileapi/file_system_operation.cc#oldcode39 webkit/fileapi/file_system_operation.cc:39: // TODO(dmikurube): Read and set available bytes from the ...

9 years, 7 months ago (2011-05-24 03:09:32 UTC) #4

oshima

http://codereview.chromium.org/7054012/diff/5001/webkit/fileapi/file_system_operation_unittest.cc File webkit/fileapi/file_system_operation_unittest.cc (right): http://codereview.chromium.org/7054012/diff/5001/webkit/fileapi/file_system_operation_unittest.cc#newcode94 webkit/fileapi/file_system_operation_unittest.cc:94: virtual void RegisterClient(QuotaClient* client) OVERRIDE {} Looks like this ...

9 years, 7 months ago (2011-05-25 20:43:38 UTC) #5

kinuko

On 2011/05/25 20:43:38, oshima wrote: > http://codereview.chromium.org/7054012/diff/5001/webkit/fileapi/file_system_operation_unittest.cc > File webkit/fileapi/file_system_operation_unittest.cc (right): > > http://codereview.chromium.org/7054012/diff/5001/webkit/fileapi/file_system_operation_unittest.cc#newcode94 > ...

9 years, 7 months ago (2011-05-25 23:05:38 UTC) #6

oshima

9 years, 7 months ago (2011-05-25 23:13:27 UTC) #7

On Wed, May 25, 2011 at 4:05 PM, <kinuko@chromium.org> wrote:

> On 2011/05/25 20:43:38, oshima wrote:
>
>
>
http://codereview.chromium.org/7054012/diff/5001/webkit/fileapi/file_system_o...
>
>> File webkit/fileapi/file_system_operation_unittest.cc (right):
>>
>
>
>
>
http://codereview.chromium.org/7054012/diff/5001/webkit/fileapi/file_system_o...
>
>> webkit/fileapi/file_system_operation_unittest.cc:94: virtual void
>> RegisterClient(QuotaClient* client) OVERRIDE {}
>> Looks like this has to call OnQuotaManagerDestoryed on the client when
>> this
>> object is deleted?
>>
>
> Hmm, it looks like.
> I wasn't able to find the leak by local valgrind (?) and didn't notice
> it...
> thanks for finding!
>

Note that this is heapchecker bot but not valgrind and they sometimes
disagree and/or report differently.

- oshima

>
> http://codereview.chromium.org/7054012/
>

Expand Messages | Collapse Messages