[webkit-reviews] review requested: [Bug 17210] Eliminate platform-specific #ifdefs in ResourceError.h and AuthenticationChallenge.h : [Attachment 18986] patch v1

[bugzilla-daemon at webkit.org]

MorganL <morganl.webkit at yahoo.com> has asked  for review:
Bug 17210: Eliminate platform-specific #ifdefs in ResourceError.h and
AuthenticationChallenge.h
http://bugs.webkit.org/show_bug.cgi?id=17210

Attachment 18986: patch v1
http://bugs.webkit.org/attachment.cgi?id=18986&action=edit

------- Additional Comments from MorganL <morganl.webkit at yahoo.com>
This patch introduces ResourceErrorBase and AuthenticationChallengeBase,
similar to what was done for ResourceResponse and ResourceRequest.

Some things to note about this patch:

1- Generic implementations of AuthenticationChallenge, ResourceError,
ResourceRequest, and ResourceResponse are now in a newly created
network/generic directory.  Ports that do not need to customize these classes
can just include the generic ones.  There does not seem to be any convention
for this in WebKit, so I'm not sure how this will be received.	Please advise
if you prefer another way.

2- The global operator== defined for many of these types is now implemented by
calling a compare function defined on the *Base class.	That compare function
then calls a platformCompare function to allow the platform to implement the
equality test for platform-specific fields.  I chose the name "compare" after
seeing it used in Vector.h.  I applied this technique to the operator==
implemented for ResourceResponse, which frees ResourceResponseBase.cpp of a
platform-specific #ifdef.

3- Many of these classes require lazy initialization on Mac and CF ports.  I
came up with a fairly uniform way of dealing with this.  Each method that may
need to perform lazy initialization, now calls a lazyInit function defined on
the *Base class.  That function simply downcasts to the non-Base class and
calls a platformLazyInit function.  The *Base class also defines
platformLazyInit to do nothing.  That way, if a port does not define
platformLazyInit, then all of the lazyInit calls should be simply eliminated by
the compiler.  This takes advantage of how method names are shadowed by
subclasses in C++.

I've tried to be fairly methodical with this patch.  Hopefully once you review
one of the classes, the changes to the second will be obvious and also
straightforward to review.