Fill size and modified time entry on FileSystem API directory view

Fill size and modified time entry on FileSystem API directory view

BUG=89134
TEST=None


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

[tzik, 2011-07-13 05:34:44]

Hi.
As the subject, I'm trying to fill size and modified time on directory view of
filesystem: scheme.
Could you review this patch?

This patch depends on http://codereview.chromium.org/7355013/.

[adamk, 2011-07-13 23:15:32]

Would you mind adding some basic test to FileSystemDirURLRequestJobTest now that
we've solidified somewhat on what the output looks like?  I left it out
originally because it was something of a placeholder, but it's been a few months
without changes (beyond this one).

[tzik, 2011-07-14 10:39:45]

I modified a test tighter.

Should we make it more, like 
DCHECK_EQ(delegate_->response_data_, ***) ?

[adamk, 2011-07-14 22:04:38]

On 2011/07/14 10:39:45, tzik wrote:
> I modified a test tighter.
> 
> Should we make it more, like 
> DCHECK_EQ(delegate_->response_data_, ***) ?

I guess I was thinking of something more like this: I'm generally not a fan of
having to override parts of the code under test in order to enable testing.  In
particular, it looks like the only thing requiring the test harness to capture
the entry in AppendEntry() is the last_modified_time; what if you instead just
matched the text output against a regexp?  The rest of the data (name, size,
is_directory) is known by the caller of VerifyEntry().

[tzik, 2011-07-19 08:40:40]

On 2011/07/14 22:04:38, adamk wrote:
> On 2011/07/14 10:39:45, tzik wrote:
> > I modified a test tighter.
> > 
> > Should we make it more, like 
> > DCHECK_EQ(delegate_->response_data_, ***) ?
> 
> I guess I was thinking of something more like this: I'm generally not a fan of
> having to override parts of the code under test in order to enable testing. 
In
> particular, it looks like the only thing requiring the test harness to capture
> the entry in AppendEntry() is the last_modified_time; what if you instead just
> matched the text output against a regexp?  The rest of the data (name, size,
> is_directory) is known by the caller of VerifyEntry().

Thanks for reviewing.
Then, how about this version of patchset?

[adamk, 2011-07-19 15:41:11]

lgtm

[commit-bot: I haz the power, 2011-07-21 04:44:19]

Change committed as 93336