[webkit-reviews] review granted: [Bug 31913] [Android] Upstream WebCore/history/android: Require some platform specific state attached to HistoryItem. : [Attachment 43925] Patch

[bugzilla-daemon at webkit.org]

Brady Eidson <beidson at apple.com> has granted Ben Murdoch <benm at google.com>'s
request for review:
Bug 31913: [Android] Upstream WebCore/history/android: Require some platform
specific state attached to HistoryItem.
https://bugs.webkit.org/show_bug.cgi?id=31913

Attachment 43925: Patch
https://bugs.webkit.org/attachment.cgi?id=43925&action=review

------- Additional Comments from Brady Eidson <beidson at apple.com>

> Index: WebCore/history/HistoryItem.h
> ===================================================================

> +#if PLATFORM(ANDROID)
> +#include "AndroidWebHistoryBridge.h"
> +#endif
> +
>  namespace WebCore {

Since nothing in the header needs the full class declaration, this should be a
forward declaration instead of an #include

I really hate the name "bridge."  It implies a completely different pattern
than what I think you're going for here.  We used to call the platform/API glue
interface between WebCore and WebKit the "bridge".  Your bridge object seems to
simply be a container for platform specific data.  Can't you call it something
else that isn't as ambiguous?