[webkit-reviews] review granted: [Bug 198805] openDatabase should return an empty object when WebSQL is disabled and quirk is enabled : [Attachment 371987] Patch

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has granted Sihui Liu <sihui_liu at apple.com>'s
request for review:
Bug 198805: openDatabase should return an empty object when WebSQL is disabled
and quirk is enabled
https://bugs.webkit.org/show_bug.cgi?id=198805

Attachment 371987: Patch

https://bugs.webkit.org/attachment.cgi?id=371987&action=review




--- Comment #2 from Geoffrey Garen <ggaren at apple.com> ---
Comment on attachment 371987
  --> https://bugs.webkit.org/attachment.cgi?id=371987
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=371987&action=review

I think we need a regression test for this. So, I think you'll also need to add
something like window.internals.enableWebSQLQuirk() for testing.

> Source/WebCore/bindings/js/JSDOMWindowCustom.cpp:599
> +	   ASSERT(name == "null");
> +	   ASSERT(version == "null");
> +	   ASSERT(displayName == "null");
> +	   ASSERT(!estimatedSize);

I don't think we want to ASSERT here. Instead, we want to check whether all
four arguments are null, and return an empty object if so, and otherwise
continue with normal behavior. That way, "if (openDatabase(null, null, null,
null)" will be true, and websites will stop assuming we're in private browsing
mode.

> Source/WebCore/bindings/js/JSDOMWindowCustom.cpp:601
> +	   return JSValue();

This line returns the C++ null JSValue(), which is only valid if you throw an
exception, and will otherwise cause a crash.

To return an empty object, you can use the constructEmptyObject() helper
function.