[Webkit-unassigned] [Bug 46405] Add idl and mock classes for FileSystemSync for FileSystem API

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=46405


David Levin <levin at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #69771|review?                     |review+, commit-queue-
               Flag|                            |




--- Comment #16 from David Levin <levin at chromium.org>  2010-10-05 13:04:33 PST ---
(From update of attachment 69771)
View in context: https://bugs.webkit.org/attachment.cgi?id=69771&action=review

Just a few things to address.

> WebCore/bindings/js/JSDirectoryEntrySyncCustom.cpp:72
> +    RefPtr<Flags> flags = getFlags(exec, exec->argument(1));

Test for flags being 0?

Due to 
if (argument.isNull() || argument.isUndefined() || !argument.isObject())
        return 0;

> WebCore/bindings/js/JSDirectoryEntrySyncCustom.cpp:89
> +    RefPtr<Flags> flags = getFlags(exec, exec->argument(1));

Test for flags being 0?

> WebCore/bindings/v8/custom/V8DirectoryEntrySyncCustom.cpp:48
> +#define GET_FLAGS_EXCEPTION_BLOCK(type, var, value) \

Macros are bad in so many ways. Please get rid of the macro and just write out the code.

I guess this is already done for strings, but I'd rather we no do it again here.

> WebCore/bindings/v8/custom/V8DirectoryEntrySyncCustom.cpp:53
> +        if (block.HasCaught()) {          \

This shouldn't have {} (single line statement).

> WebCore/bindings/v8/custom/V8DirectoryEntrySyncCustom.cpp:88
> +    RefPtr<Flags> flags = getFlags(args[1]);

Shouldn't this check for flags being 0? 

Is there a layout test for this error case?
If not, please add one.

> WebCore/bindings/v8/custom/V8DirectoryEntrySyncCustom.cpp:103
> +    RefPtr<Flags> flags = getFlags(args[1]);

Test for flags being 0.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.