Dropping BrowsingDataFileSystemHelper browser_test in favor of a unit_test.

Dropping BrowsingDataFileSystemHelper browser_test in favor of a unit_test.

BUG=None
TEST=


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=88531

[Mike West, 2011-06-03 09:54:28]

This looks right, but heapchecker doesn't like it at all.

I need some help tracking down the leak.  I'm sure it's something I'm doing
wrong, but it's not jumping out at me.  TestingProfile's destructor gets called,
but none of the fileapi::* bits seem be destroyed.  I don't see any obvious
points where I'm holding on to references...

If you have a few minutes, I'd appreciate another look.

[Mike West, 2011-06-03 09:58:44]

On 2011/06/03 09:54:28, Mike West wrote:

> If you have a few minutes, I'd appreciate another look.

Heapcheck log is at http://www/~mkwst/heapcheck.log

Pretty pictures at http://www/~mkwst/heapcheck.ps

[Eric U., 2011-06-03 18:08:51]

I'll take a look.

On Fri, Jun 3, 2011 at 2:58 AM,  <mkwst@chromium.org> wrote:
> On 2011/06/03 09:54:28, Mike West wrote:
>
>> If you have a few minutes, I'd appreciate another look.
>
> Heapcheck log is at http://www/~mkwst/heapcheck.log
>
> Pretty pictures at http://www/~mkwst/heapcheck.ps
>
> http://codereview.chromium.org/7065056/
>

[Eric U., 2011-06-03 20:25:15]

I found it.  You've declared TestingProfile profile_ above your
BrowserThreads in BrowsingDataFileSystemHelperTest.  That means that,
at destruction time, the profile gets deleted after the threads.
FileSystemContext is declared as:

class FileSystemContext
    : public base::RefCountedThreadSafe<FileSystemContext,
                                        DefaultContextDeleter> {

...where DefaultContextDeleter checks to see if we're on the IO
thread, and if not, posts a task to the IO thread to do the deletion.
By the time this happens, it's too late to be posting tasks there.  If
you just move the declaration of profile_ down to below the
BrowserThreads, everything works and the needed destructors get
called.  There's no cross-thread postTask, since you've mocked
everything up to always believe that it's on the right thread.

On Fri, Jun 3, 2011 at 11:08 AM, Eric U <ericu@google.com> wrote:
> I'll take a look.
>
> On Fri, Jun 3, 2011 at 2:58 AM,  <mkwst@chromium.org> wrote:
>> On 2011/06/03 09:54:28, Mike West wrote:
>>
>>> If you have a few minutes, I'd appreciate another look.
>>
>> Heapcheck log is at http://www/~mkwst/heapcheck.log
>>
>> Pretty pictures at http://www/~mkwst/heapcheck.ps
>>
>> http://codereview.chromium.org/7065056/
>>
>

[Mike West, 2011-06-06 11:48:03]

1. EricU is awesome.

2. The three tiny leaks that still show up are now two tiny leaks, both
suppressed.  The moral of the story is: if there are leaks in your code, wait 3
days for someone else to fix them and then rebase onto ToT.  :)

So, heapcheck passes, tests pass.  I think we should be good to go here.  :)

[jochen (gone - plz use gerrit), 2011-06-09 14:14:49]

LGTM

[commit-bot: I haz the power, 2011-06-09 15:22:16]

Change committed as 88531