Bug 55732 – WebKit2: Refactor classes in GenericCallback.h

• Component: WebKit2
• Status: RESOLVED FIXED

[Jeff Miller]

Refactor classes in GenericCallback.h

[Jeff Miller]

Created attachment 84663 [details]
Patch

[Darin Adler]

Comment on attachment 84663 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=84663&action=review

Change looks good. Perhaps we should put RefCounted in CallbackBase too?

> Source/WebKit2/ChangeLog:17
> +        Remove FIXME comment about making a version of GenericCallback with two arguments, and defining
> +        ComputedPagesCallback.  This isn't really feasible.  Also, the parameters are a const Vector<WebCore::IntRect>&
> +        and a double, and I don't think a callback with a class and POD parameter types is really worth a generic class.

I don’t understand why it isn’t feasible. I think we can make a template the works with both class and POD types.

[Jeff Miller]

(In reply to comment #2)
> (From update of attachment 84663 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=84663&action=review
> 
> Change looks good. Perhaps we should put RefCounted in CallbackBase too?

Good idea, I'll make this change before landing the patch.

> 
> > Source/WebKit2/ChangeLog:17
> > +        Remove FIXME comment about making a version of GenericCallback with two arguments, and defining
> > +        ComputedPagesCallback.  This isn't really feasible.  Also, the parameters are a const Vector<WebCore::IntRect>&
> > +        and a double, and I don't think a callback with a class and POD parameter types is really worth a generic class.
> 
> I don’t understand why it isn’t feasible. I think we can make a template the works with both class and POD types.

We certainly could, but it seems somewhat unlikely that it would be used more than once.  In any case, I'll restore the FIXME comment.

[Jeff Miller]

Committed r80322: <http://trac.webkit.org/changeset/80322>

[WebKit Review Bot]

http://trac.webkit.org/changeset/80322 might have broken Windows 7 Release (Tests)
The following tests are not passing:
http/tests/cookies/double-quoted-value-with-semi-colon.html
http/tests/cookies/simple-cookies-expired.html
http/tests/cookies/simple-cookies-max-age.html
http/tests/incremental/split-hex-entities.pl