Modern IDB: In the VersionChangeEvent for delete database calls, newVersion should be null instead of 0

[Brady Eidson]

Modern IDB: In the VersionChangeEvent for delete database calls, oldVersion should be null instead of 0

[Brady Eidson]

This will require a custom binding.

[Brady Eidson]

Created attachment 265973 [details]
Patch v1

[Brady Eidson]

These build failures are linker errors on finding stuff for jsNull().

From the mac build:
Undefined symbols for architecture x86_64:
  "__ZN3JSC7JSValueC1ENS0_9JSNullTagE", referenced from:
      __ZNK7WebCore23JSIDBVersionChangeEvent10newVersionERN3JSC9ExecStateE in JSIDBVersionChangeEventCustom.o
  "__ZN3JSC7JSValueC1Ey", referenced from:
      __ZNK7WebCore23JSIDBVersionChangeEvent10oldVersionERN3JSC9ExecStateE in JSIDBVersionChangeEventCustom.o
      __ZNK7WebCore23JSIDBVersionChangeEvent10newVersionERN3JSC9ExecStateE in JSIDBVersionChangeEventCustom.o
ld: symbol(s) not found for architecture x86_64

From EFL:
WebCore::JSIDBVersionChangeEvent::newVersion(JSC::ExecState&) const: error: undefined reference to 'JSC::JSValue::JSValue(JSC::JSValue::JSNullTag)'

I'm confused by these because:
1 - This patch builds fine for me locally on Mac.
2 - Plenty of other custom bindings use "jsNull()" directly.

[Brady Eidson]

mac-debug built fine.

The others didn't.

How is this a release vs debug issue?  =/

[Brady Eidson]

Created attachment 265978 [details]
Patch v2 - Shot in the dark...

[WebKit Commit Bot]

Attachment 265978 [details] did not pass style-queue:


ERROR: Source/WebCore/bindings/js/JSIDBVersionChangeEventCustom.cpp:30:  You should not add a blank line before implementation file's own header.  [build/include_order] [4]
Total errors found: 1 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Brady Eidson]

This is definitely a debug vs release issue, as I can reproduce the linker error with a local release build.

*WHY*?

No friggin' clue.

[Brady Eidson]

Created attachment 265982 [details]
Patch v3

[Brady Eidson]

Some debug-vs-release build difference in which JSValue functions come inlined or not.  *sigh*

Fixed!

[Sam Weinig]

Comment on attachment 265982 [details]
Patch v3

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

> Source/WebCore/ChangeLog:4
> +        Modern IDB: In the VersionChangeEvent for delete database calls, oldVersion should be null instead of 0.
> +        https://bugs.webkit.org/show_bug.cgi?id=151481

This title does not match what the patch does.  The patch changes the behavior of newVersion, not oldVersion.

> Source/WebCore/Modules/indexeddb/IDBVersionChangeEvent.idl:30
> +    [Custom] readonly attribute unsigned long long oldVersion;

No need for this to be custom.

> Source/WebCore/Modules/indexeddb/IDBVersionChangeEvent.idl:31
> +    [Custom] readonly attribute unsigned long long newVersion;

Can this use the ? syntax instead of using a custom binding?

[Brady Eidson]

Patch was right, title was wrong.

Trying the optional attribute in bindings right now.

[Brady Eidson]

Created attachment 265991 [details]
Patch v4

[Brady Eidson]

Created attachment 265992 [details]
Patch v5

[Brady Eidson]

https://trac.webkit.org/changeset/192698