Bug 43382 – Auto-generate {DOMWindow,WorkerContext}::openDatabase{Sync}

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Dumitru Daniliuc]

Kinuko recently added support for callback arguments to code generator, which means that DOMWindow::openDatabase, and WorkerContext::openDatabase{Sync} can be auto-generated. We should do that.

[Dumitru Daniliuc]

Created attachment 63438 [details]
patch

Even though the patch is huge, most of it is mechanical changes. The only interesting changes are in CodeGenerator{JS|V8}.pm.

[WebKit Review Bot]

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

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebCore/bindings/scripts/test/JS/JSTestCallback.cpp:47:  Use 0 instead of NULL.  [readability/null] [4]
WebCore/ChangeLog:86:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 2 in 20 files


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

[Adam Barth]

Comment on attachment 63438 [details]
patch

WebCore/ChangeLog:26
 +  
extra blank line.

Wow, this patch is awesome.  Thanks!

[Dumitru Daniliuc]

(In reply to comment #3)
> (From update of attachment 63438 [details])
> WebCore/ChangeLog:26
>  +  
> extra blank line.

Following Darin Adler's style of ChangeLog entries, where all changed files are split into groups, according to the changes made to them. :)

Also, I've addressed the issues reported by the style bot.

[Adam Barth]

Ah, ok.

[Dumitru Daniliuc]

landed: r64835.

[WebKit Review Bot]

http://trac.webkit.org/changeset/64835 might have broken Qt Linux Release minimal

[Dumitru Daniliuc]

The patch broke the "Qt Linux minimal" and "Gtk Linux 64-bit Debug" builds. Both builds are fixed now.

[Pavel Feldman]

Rolling it out for breaking 800 chromium tests. Will roll in in case it was not the real cause.

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	LayoutTests/ChangeLog
	M	LayoutTests/fast/workers/storage/open-database-inputs-sync-expected.txt
	M	LayoutTests/fast/workers/storage/resources/open-database-inputs-sync.js
	M	LayoutTests/storage/null-callbacks.html
	M	WebCore/Android.jscbindings.mk
	M	WebCore/Android.v8bindings.mk
	M	WebCore/CMakeLists.txt
	M	WebCore/ChangeLog
	M	WebCore/GNUmakefile.am
	M	WebCore/WebCore.gypi
	M	WebCore/WebCore.pro
	M	WebCore/WebCore.vcproj/WebCore.vcproj
	M	WebCore/WebCore.xcodeproj/project.pbxproj
	M	WebCore/bindings/js/JSBindingsAllInOne.cpp
	M	WebCore/bindings/js/JSDOMWindowCustom.cpp
	A	WebCore/bindings/js/JSDatabaseCustom.cpp
	A	WebCore/bindings/js/JSDatabaseSyncCustom.cpp
	M	WebCore/bindings/js/JSWorkerContextCustom.cpp
	M	WebCore/bindings/scripts/CodeGeneratorGObject.pm
	M	WebCore/bindings/scripts/CodeGeneratorJS.pm
	M	WebCore/bindings/scripts/CodeGeneratorV8.pm
	M	WebCore/bindings/scripts/test/JS/JSTestCallback.cpp
	M	WebCore/bindings/scripts/test/JS/JSTestObj.cpp
	M	WebCore/bindings/scripts/test/TestObj.idl
	M	WebCore/bindings/scripts/test/V8/V8TestObj.cpp
	M	WebCore/bindings/v8/V8Binding.h
	M	WebCore/bindings/v8/custom/V8DOMWindowCustom.cpp
	A	WebCore/bindings/v8/custom/V8DatabaseCustom.cpp
	A	WebCore/bindings/v8/custom/V8DatabaseSyncCustom.cpp
	M	WebCore/bindings/v8/custom/V8WorkerContextCustom.cpp
	M	WebCore/page/DOMWindow.idl
	M	WebCore/storage/Database.cpp
	M	WebCore/storage/Database.h
	M	WebCore/storage/Database.idl
	M	WebCore/storage/DatabaseSync.cpp
	M	WebCore/storage/DatabaseSync.h
	M	WebCore/storage/DatabaseSync.idl
	M	WebCore/workers/WorkerContext.idl
Committed r64840

[Dumitru Daniliuc]

Reverting the patch fixed the Chromium problem. I'll patch it in my Chromium client and fix the problem before I resubmit it.

[Dumitru Daniliuc]

it looks like the checks i wanted to add to the code generators are too strict and would probably break a lot of (badly coded) websites. so i'll work on a patch that only affects the DB bindings.

[Dumitru Daniliuc]

Created attachment 63888 [details]
patch

[WebKit Review Bot]

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

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebCore/bindings/scripts/test/JS/JSTestCallback.cpp:47:  Use 0 instead of NULL.  [readability/null] [4]
Total errors found: 1 in 21 files


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

[Dumitru Daniliuc]

> WebCore/bindings/scripts/test/JS/JSTestCallback.cpp:47:  Use 0 instead of NULL.  [readability/null] [4]

It's a comment.

[David Levin]

(In reply to comment #14)
> > WebCore/bindings/scripts/test/JS/JSTestCallback.cpp:47:  Use 0 instead of NULL.  [readability/null] [4]
> 
> It's a comment.

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

[Adam Barth]

Comment on attachment 63888 [details]
patch

Nice solution.  Thanks.

[Dumitru Daniliuc]

(In reply to comment #15)
> (In reply to comment #14)
> > > WebCore/bindings/scripts/test/JS/JSTestCallback.cpp:47:  Use 0 instead of NULL.  [readability/null] [4]
> > 
> > It's a comment.
> 
> See https://bugs.webkit.org/show_bug.cgi?id=34605

I'll change the comment from NULL to 0, but I haven't heard anybody saying "if X is zero, then ..." when talking about pointers; it's always "if X is null, then...". Therefore, imho, it should be OK to use NULL instead of 0 in comments.

[David Levin]

(In reply to comment #17)
> I'll change the comment from NULL to 0, but I haven't heard anybody saying "if X is zero, then ..." when talking about pointers; it's always "if X is null, then...". Therefore, imho, it should be OK to use NULL instead of 0 in comments.

No a big deal.

fwiw, null doesn't get flagged (only NULL).

[Dumitru Daniliuc]

re-landed: r65005.