Bug 43382

Summary: Auto-generate {DOMWindow,WorkerContext}::openDatabase{Sync}
Product: WebKit Reporter: Dumitru Daniliuc <dumi>
Component: New BugsAssignee: Dumitru Daniliuc <dumi>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, eric, levin, pfeldman, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
patch
abarth: review+, dumi: commit-queue-
patch abarth: review+, dumi: commit-queue-

Description Dumitru Daniliuc 2010-08-02 14:43:14 PDT
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.
Comment 1 Dumitru Daniliuc 2010-08-04 03:44:58 PDT
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.
Comment 2 WebKit Review Bot 2010-08-04 03:47:28 PDT
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.
Comment 3 Adam Barth 2010-08-05 16:59:47 PDT
Comment on attachment 63438 [details]
patch

WebCore/ChangeLog:26
 +  
extra blank line.

Wow, this patch is awesome.  Thanks!
Comment 4 Dumitru Daniliuc 2010-08-05 17:07:14 PDT
(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.
Comment 5 Adam Barth 2010-08-05 17:19:42 PDT
Ah, ok.
Comment 6 Dumitru Daniliuc 2010-08-06 04:47:53 PDT
landed: r64835.
Comment 7 WebKit Review Bot 2010-08-06 04:52:42 PDT
http://trac.webkit.org/changeset/64835 might have broken Qt Linux Release minimal
Comment 8 Dumitru Daniliuc 2010-08-06 05:41:08 PDT
The patch broke the "Qt Linux minimal" and "Gtk Linux 64-bit Debug" builds. Both builds are fixed now.
Comment 9 Pavel Feldman 2010-08-06 06:01:10 PDT
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
Comment 10 Dumitru Daniliuc 2010-08-06 06:26:35 PDT
Reverting the patch fixed the Chromium problem. I'll patch it in my Chromium client and fix the problem before I resubmit it.
Comment 11 Dumitru Daniliuc 2010-08-06 16:18:21 PDT
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.
Comment 12 Dumitru Daniliuc 2010-08-09 05:22:22 PDT
Created attachment 63888 [details]
patch
Comment 13 WebKit Review Bot 2010-08-09 05:24:38 PDT
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.
Comment 14 Dumitru Daniliuc 2010-08-09 05:25:21 PDT
> WebCore/bindings/scripts/test/JS/JSTestCallback.cpp:47:  Use 0 instead of NULL.  [readability/null] [4]

It's a comment.
Comment 15 David Levin 2010-08-09 07:23:53 PDT
(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
Comment 16 Adam Barth 2010-08-09 10:51:03 PDT
Comment on attachment 63888 [details]
patch

Nice solution.  Thanks.
Comment 17 Dumitru Daniliuc 2010-08-09 13:41:07 PDT
(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.
Comment 18 David Levin 2010-08-09 13:48:50 PDT
(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).
Comment 19 Dumitru Daniliuc 2010-08-09 14:38:05 PDT
re-landed: r65005.