[Webkit-unassigned] [Bug 127255] Session API

[bugzilla-daemon at webkit.org]

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





--- Comment #3 from Alexey Proskuryakov <ap at webkit.org>  2014-01-19 12:28:50 PST ---
(From update of attachment 221593)
View in context: https://bugs.webkit.org/attachment.cgi?id=221593&action=review

> Source/WebCore/page/Page.h:421
> +    uint64_t getSessionID() { return m_sessionID; }

WebKit style is to not have a "get" prefix on getters. We use this prefix for functions that return result via an argument, e.g. 

bool getFoobar(Foobar& result);

> Source/WebKit2/UIProcess/API/C/WKSessionRef.h:39
>  WK_EXPORT bool WKSessionGetEphemeral(WKSessionRef session);

As previously commented, this should be WKSessionIsEphemeral, not WKSessionGetEphemeral. The name WKSessionGetEphemeral means that we are getting and returning an ephemeral session.

> Source/WebKit2/WebProcess/WebCoreSupport/mac/WebFrameNetworkingContext.mm:107
> +        if (frame()->page()->getSessionID())
> +            return *SessionTracker::session(frame()->page()->getSessionID());
>  
> +        if (frame()->settings().privateBrowsingEnabled())
> +            return *SessionTracker::session(SessionTracker::legacyPrivateSessionID);

This is not the right approach. The knowledge about legacy session IDs should be isolated as close to API layer as possible, most of the code should simply use sessions.

Having separate code paths for sessions and for private browsing everywhere we used to check for privateBrowsingEnabled() is very fragile and confusing.

> Source/WebKit2/WebProcess/WebPage/WebPage.h:878
> +    uint64_t m_sessionID;

I wouldn't duplicate the knowledge in this class, WebCore already knows what session it is.

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