Bug 37671

Summary: Add WebHistoryClient to WebKit2
Product: WebKit Reporter: Sam Weinig <sam>
Component: New BugsAssignee: Sam Weinig <sam>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, eric, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch andersca: review+

Description Sam Weinig 2010-04-15 13:45:13 PDT
Add WebHistoryClient to WebKit2.
Comment 1 Sam Weinig 2010-04-15 13:52:00 PDT
Created attachment 53471 [details]
Patch
Comment 2 WebKit Review Bot 2010-04-15 13:55:47 PDT
Attachment 53471 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebKit2/UIProcess/API/C/WKNavigationData.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebKit2/UIProcess/WebHistoryClient.h:32:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
WebKit2/UIProcess/API/C/WKPage.cpp:127:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebKit2/UIProcess/WebHistoryClient.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:49:  Alphabetical sorting problem.  [build/include_order] [4]
WebKit2/UIProcess/WebNavigationData.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Total errors found: 6 in 21 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Anders Carlsson 2010-04-15 14:08:31 PDT
Comment on attachment 53471 [details]
Patch


> +        * 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
Comment 4 Sam Weinig 2010-04-15 14:16:15 PDT
> > --- 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?

I don't think so. Maybe Brady would know.
Comment 5 WebKit Review Bot 2010-04-15 15:16:07 PDT
http://trac.webkit.org/changeset/57676 might have broken Tiger Intel Release
The following changes are on the blame list:
http://trac.webkit.org/changeset/57674
http://trac.webkit.org/changeset/57675
http://trac.webkit.org/changeset/57676
Comment 6 Eric Seidel (no email) 2010-04-21 18:15:28 PDT
Attachment 53471 [details] was posted by a committer and has review+, assigning to Sam Weinig for commit.