Bug 73945

Summary: JSCustomVoidCallback should also be derived from ActiveDOMCallback
Product: WebKit Reporter: Yong Li <yong.li.webkit>
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: staikos, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 49401    
Bug Blocks:    
Attachments:
Description Flags
proposed patch
webkit-ews: commit-queue-
fix the style
none
should be done in this way
none
Ready for review none

Yong Li
Reported 2011-12-06 13:17:27 PST
Bug 40112 and r64537 introduced ActiveDOMCallback which can be suspended & resumed when needed. However it doesn't cover JSCustomVoidCallback which is used by database transaction as success callback. This results in: 1) the callback can be executed when page loading is deferred (for example, when a JS dialog shows up). I've verified this with Google Chrome. 2) the original issue of Bug 40112 could still exist for success callbacks with real(non-null) transactions. I haven't verified that though. My patch for https://bugs.webkit.org/show_bug.cgi?id=49401 was rolled out otherwise 1) wouldn't be an issue.
Attachments
proposed patch (3.30 KB, patch)
2011-12-06 14:20 PST, Yong Li 		webkit-ews: commit-queue-
DetailsFormatted DiffDiff
fix the style (3.33 KB, patch)
2011-12-06 14:38 PST, Yong Li 		no flags
DetailsFormatted DiffDiff
should be done in this way (3.32 KB, patch)
2011-12-06 15:01 PST, Yong Li 		no flags
DetailsFormatted DiffDiff
Ready for review (3.37 KB, patch)
2011-12-07 07:45 PST, Yong Li 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Yong Li
Comment 1 2011-12-06 13:37:11 PST
Change the tile
Yong Li
Comment 2 2011-12-06 13:46:18 PST
(In reply to comment #0) > Bug 40112 and r64537 introduced ActiveDOMCallback which can be suspended & resumed when needed. > > However it doesn't cover JSCustomVoidCallback which is used by database transaction as success callback. > > This results in: > > 1) the callback can be executed when page loading is deferred (for example, when a JS dialog shows up). I've verified this with Google Chrome. > 2) the original issue of Bug 40112 could still exist for success callbacks with real(non-null) transactions. I haven't verified that though. > > My patch for https://bugs.webkit.org/show_bug.cgi?id=49401 was rolled out otherwise 1) wouldn't be an issue. Ignore 2). seems it shouldn't be a problem
Yong Li
Comment 3 2011-12-06 14:20:29 PST
Created attachment 118110 [details] proposed patch With the patch, the callback event will be discarded when a JS dialog is running. That issue is covered by Bug 49401.
WebKit Review Bot
Comment 4 2011-12-06 14:24:20 PST
Attachment 118110 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ManualTests/database-success-callback.html..." exit_code: 1 Source/WebCore/ChangeLog:1: ChangeLog entry has no bug number [changelog/bugnumber] [5] Source/WebCore/bindings/js/JSCustomVoidCallback.h:33: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 2 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Early Warning System Bot
Comment 5 2011-12-06 14:31:18 PST
Comment on attachment 118110 [details] proposed patch Attachment 118110 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/10744075
Yong Li
Comment 6 2011-12-06 14:38:47 PST
Created attachment 118115 [details] fix the style
WebKit Review Bot
Comment 7 2011-12-06 14:41:14 PST
Attachment 118115 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ManualTests/database-success-callback.html..." exit_code: 1 Source/WebCore/ChangeLog:1: ChangeLog entry has no bug number [changelog/bugnumber] [5] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Yong Li
Comment 8 2011-12-06 15:01:48 PST
Created attachment 118118 [details] should be done in this way
WebKit Review Bot
Comment 9 2011-12-06 15:04:16 PST
Attachment 118118 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ManualTests/database-success-callback.html..." exit_code: 1 Source/WebCore/ChangeLog:1: ChangeLog entry has no bug number [changelog/bugnumber] [5] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Yong Li
Comment 10 2011-12-07 07:45:03 PST
Created attachment 118210 [details] Ready for review
Yong Li
Comment 11 2012-01-05 11:08:27 PST
(In reply to comment #0) > My patch for https://bugs.webkit.org/show_bug.cgi?id=49401 was rolled out otherwise 1) wouldn't be an issue. The patch for https://bugs.webkit.org/show_bug.cgi?id=49401 is landed again, so we no longer need to make JSCustomVoidCallback a ActiveDOMCallback.
Note You need to log in before you can comment on or make changes to this bug.