[Webkit-unassigned] [Bug 47354] Need WKPage API for serializing and restoring a page's state

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=47354





--- Comment #18 from Darin Adler <darin at apple.com>  2010-10-11 17:00:25 PST ---
(From update of attachment 70478)
View in context: https://bugs.webkit.org/attachment.cgi?id=70478&action=review

> WebKit2/UIProcess/WebPageProxy.h:168
> +    typedef bool (*WebPageProxySessionStateFilterCallback)(WKPageRef, WKStringRef type, WKTypeRef object, void*);

Probably should name the context argument here.

> WebKit2/UIProcess/API/C/WKPage.cpp:164
> +    RefPtr<WebData> state = toImpl(pageRef)->sessionStateData(filter, context);
>      return toAPI(state.release().releaseRef());

No obvious need for a local variable here. Might read better as a single long line.

> WebKit2/UIProcess/API/C/WKPage.h:211
> +typedef bool (*WKPageSessionStateFilterCallback)(WKPageRef page, WKStringRef valueType, WKTypeRef value, void* context);
> +WK_EXPORT WKDataRef WKPageCopySessionState(WKPageRef page, void *context, WKPageSessionStateFilterCallback urlAllowedCallback);

Inconsistent void* vs. void * on these two lines.

> WebKit2/UIProcess/cf/WebBackForwardListCF.cpp:46
> +    const void* keys[2] = { SessionHistoryCurrentIndexKey(), SessionHistoryEntriesKey() };
> +    const void* values[2] = { currentIndex.get(), entries.get() };
> +
> +    RetainPtr<CFDictionaryRef> dictionary(AdoptCF, CFDictionaryCreate(0, keys, values, 2, &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks));

That constant 2 is repeated three times. Would be bad if these didn’t agree. No concrete idea for making it better.

> WebKit2/UIProcess/cf/WebBackForwardListCF.cpp:59
> +        const void* keys[2] = { SessionHistoryEntryURLKey(), SessionHistoryEntryTitleKey() };
> +        const void* values[2] = { url.get(), title.get() };
> +
> +        RetainPtr<CFDictionaryRef> entryDictionary(AdoptCF, CFDictionaryCreate(0, keys, values, 2, &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks));

Ditto.

> WebKit2/UIProcess/cf/WebPageProxyCF.cpp:51
> +    const void* keys[1] = { SessionHistoryKey() };
> +    const void* values[1] = { sessionHistoryDictionary.get() };
> +    
> +    RetainPtr<CFDictionaryRef> stateDictionary(AdoptCF, CFDictionaryCreate(0, keys, values, 1, &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks));

For a single entry you don’t really need to use an array.

> WebKit2/UIProcess/cf/WebPageProxyCF.cpp:56
> +    Vector<unsigned char> stateVector(length + 4);

All these explicit "4" constants are slightly annoying. Maybe sizeof(UInt32)?

> WebKit2/UIProcess/cf/WebPageProxyCF.cpp:72
> +    if (!webData || webData->size() < 4)

Here’s another 4.

> WebKit2/UIProcess/cf/WebPageProxyCF.cpp:83
> +    RetainPtr<CFDataRef> data(AdoptCF, CFDataCreate(0, webData->bytes() + 4, webData->size() - 4));

And two more 4’s.

> WebKit2/UIProcess/cf/WebPageProxyCF.cpp:87
> +    if (propertyListError) {

Don’t you have to CFRelease this?

> WebKit2/UIProcess/cf/WebPageProxyCF.cpp:98
> +    if (CFGetTypeID(propertyList.get()) != CFDictionaryGetTypeID()) {
> +        LOG(SessionState, "SessionState property list is not a CFDictionaryRef (%i) - its CFTypeID is %i", (int)CFDictionaryGetTypeID(), (int)CFGetTypeID(propertyList.get()));
> +        return;
> +    }

I don’t think CFPropertyListCreateWithData can return a non-dictionary.

> WebKit2/WebKit2.xcodeproj/project.pbxproj:653
> -		BC111B0B112F5E4F00337BAB /* WebPageProxy.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = WebPageProxy.cpp; sourceTree = "<group>"; };
> +		BC111B0B112F5E4F00337BAB /* WebPageProxy.cpp */ = {isa = PBXFileReference; explicitFileType = sourcecode.cpp.cpp; fileEncoding = 4; path = WebPageProxy.cpp; sourceTree = "<group>"; };

Why this change?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.