JSCustomVoidCallback should also be derived from ActiveDOMCallback

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Yong Li]

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.

[Yong Li]

Change the tile

[Yong Li]

(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]

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]

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 on attachment 118110 [details]
proposed patch

Attachment 118110 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/10744075

[Yong Li]

Created attachment 118115 [details]
fix the style

[WebKit Review Bot]

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]

Created attachment 118118 [details]
should be done in this way

[WebKit Review Bot]

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]

Created attachment 118210 [details]
Ready for review

[Yong Li]

(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.