Bug 73945 - JSCustomVoidCallback should also be derived from ActiveDOMCallback
Summary: JSCustomVoidCallback should also be derived from ActiveDOMCallback
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 49401
Blocks:
  Show dependency treegraph
 
Reported: 2011-12-06 13:17 PST by Yong Li
Modified: 2012-01-05 11:08 PST (History)
2 users (show)

See Also:


Attachments
proposed patch (3.30 KB, patch)
2011-12-06 14:20 PST, Yong Li
webkit-ews: commit-queue-
Details | Formatted Diff | Diff
fix the style (3.33 KB, patch)
2011-12-06 14:38 PST, Yong Li
no flags Details | Formatted Diff | Diff
should be done in this way (3.32 KB, patch)
2011-12-06 15:01 PST, Yong Li
no flags Details | Formatted Diff | Diff
Ready for review (3.37 KB, patch)
2011-12-07 07:45 PST, Yong Li
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yong Li 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.
Comment 1 Yong Li 2011-12-06 13:37:11 PST
Change the tile
Comment 2 Yong Li 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
Comment 3 Yong Li 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.
Comment 4 WebKit Review Bot 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.
Comment 5 Early Warning System Bot 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
Comment 6 Yong Li 2011-12-06 14:38:47 PST
Created attachment 118115 [details]
fix the style
Comment 7 WebKit Review Bot 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.
Comment 8 Yong Li 2011-12-06 15:01:48 PST
Created attachment 118118 [details]
should be done in this way
Comment 9 WebKit Review Bot 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.
Comment 10 Yong Li 2011-12-07 07:45:03 PST
Created attachment 118210 [details]
Ready for review
Comment 11 Yong Li 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.