Bug 55732 - WebKit2: Refactor classes in GenericCallback.h
Summary: WebKit2: Refactor classes in GenericCallback.h
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Jeff Miller
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-03-03 17:20 PST by Jeff Miller
Modified: 2011-03-03 23:07 PST (History)
3 users (show)

See Also:


Attachments
Patch (8.89 KB, patch)
2011-03-03 17:30 PST, Jeff Miller
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jeff Miller 2011-03-03 17:20:21 PST
Refactor classes in GenericCallback.h
Comment 1 Jeff Miller 2011-03-03 17:30:50 PST
Created attachment 84663 [details]
Patch
Comment 2 Darin Adler 2011-03-03 18:13:46 PST
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.
Comment 3 Jeff Miller 2011-03-03 21:32:17 PST
(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.
Comment 4 Jeff Miller 2011-03-03 21:36:49 PST
Committed r80322: <http://trac.webkit.org/changeset/80322>
Comment 5 WebKit Review Bot 2011-03-03 23:07:20 PST
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