[webkit-reviews] review granted: [Bug 37671] Add WebHistoryClient to WebKit2 : [Attachment 53471] Patch

[bugzilla-daemon at webkit.org]

Anders Carlsson <andersca at apple.com> has granted Sam Weinig <sam at webkit.org>'s
request for review:
Bug 37671: Add WebHistoryClient to WebKit2
https://bugs.webkit.org/show_bug.cgi?id=37671

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

------- Additional Comments from Anders Carlsson <andersca at apple.com>

> +	   * Shared/CoreIPCSupport/WebPageProxyMessageKinds.h:
> +	   (WebPageProxyMessage::):
> +	   * Shared/WebNavigationDataStore.h: Added.
> +	   (WebKit::WebNavigationDataStore::encode):
> +	   (WebKit::WebNavigationDataStore::decode):
> +	   * UIProcess/API/C/WKAPICast.h:
> +	   (toWK):
> +	   (toRef):
> +	   * UIProcess/API/C/WKBase.h:
> +	   * UIProcess/API/C/WKNavigationData.cpp: Added.
> +	   (WKNavigationDataGetTitle):
> +	   (WKNavigationDataGetURL):
> +	   (WKNavigationDataRetain):
> +	   (WKNavigationDataRelease):
> +	   * UIProcess/API/C/WKNavigationData.h: Added.
> +	   * UIProcess/API/C/WKPage.cpp:
> +	   (WKPageSetPageHistoryClient):
> +	   * UIProcess/API/C/WKPage.h:
> +	   * UIProcess/API/C/WebKit2.h:
> +	   * UIProcess/WebHistoryClient.cpp: Copied from
UIProcess/WebUIClient.cpp.
> +	   (WebKit::WebHistoryClient::WebHistoryClient):
> +	   (WebKit::WebHistoryClient::initialize):
> +	   (WebKit::WebHistoryClient::didNavigateWithNavigationData):
> +	   (WebKit::WebHistoryClient::didPerformClientRedirect):
> +	   (WebKit::WebHistoryClient::didPerformServerRedirect):
> +	   (WebKit::WebHistoryClient::didUpdateHistoryTitle):
> +	   * UIProcess/WebHistoryClient.h: Copied from UIProcess/WebUIClient.h.

> +	   * UIProcess/WebNavigationData.cpp: Added.
> +	   (WebKit::WebNavigationData::WebNavigationData):
> +	   (WebKit::WebNavigationData::~WebNavigationData):
> +	   * UIProcess/WebNavigationData.h: Added.
> +	   (WebKit::WebNavigationData::create):
> +	   (WebKit::WebNavigationData::title):
> +	   (WebKit::WebNavigationData::url):
> +	   * UIProcess/WebPageProxy.cpp:
> +	   (WebKit::WebPageProxy::initializeHistoryClient):
> +	   (WebKit::WebPageProxy::didReceiveMessage):
> +	   (WebKit::WebPageProxy::didNavigateWithNavigationData):
> +	   (WebKit::WebPageProxy::didPerformClientRedirect):
> +	   (WebKit::WebPageProxy::didPerformServerRedirect):
> +	   (WebKit::WebPageProxy::didUpdateHistoryTitle):
> +	   * UIProcess/WebPageProxy.h:
> +	   * WebKit2.xcodeproj/project.pbxproj:
> +	   * WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
> +	   (WebKit::WebFrameLoaderClient::updateGlobalHistory):
> +	   (WebKit::WebFrameLoaderClient::updateGlobalHistoryRedirectLinks):
> +	   (WebKit::WebFrameLoaderClient::setTitle):
> +	   * win/WebKit2.vcproj:
> +

You can remove all the functions here - they don't really add anything.

> +
> +    static bool decode(CoreIPC::ArgumentDecoder& decoder,
WebNavigationDataStore& s)
> +    {

Please use a longer name here than just 's'.

> +	   if (!decoder.decode(s.url))
> +	       return false;
> +	   if (!decoder.decode(s.title))
> +	       return false;
> +	   return true;
> +    }
> +
> +    // FIXME: Add the remaining items we want to track for history.
> +    WebCore::String url;
> +    WebCore::String title;
> +};
> +
> +} // namespace WebKit
> +
> +#endif // WebNavigationDataStore_h
> Index: WebKit2/Shared/CoreIPCSupport/WebPageProxyMessageKinds.h
> ===================================================================
> --- WebKit2/Shared/CoreIPCSupport/WebPageProxyMessageKinds.h	(revision
57651)
> +++ WebKit2/Shared/CoreIPCSupport/WebPageProxyMessageKinds.h	(working copy)
> @@ -53,6 +53,9 @@ enum Kind {
>      DidFinishProgress,
>      DidFirstLayoutForFrame,
>      DidFirstVisuallyNonEmptyLayoutForFrame,
> +    DidNavigateWithNavigationData,

Could we merge DidNavigateWithNavigationData with DidCommitLoadForFrame
somehow?

r=me