Bug 128410 - Add API for accessing session ephemerality from a page bundle
Summary: Add API for accessing session ephemerality from a page bundle
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Martin Hock
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-02-07 16:11 PST by Martin Hock
Modified: 2014-02-10 10:54 PST (History)
2 users (show)

See Also:


Attachments
patch (4.94 KB, patch)
2014-02-07 16:17 PST, Martin Hock
ap: review-
Details | Formatted Diff | Diff
expose just the bool from bundle (3.07 KB, patch)
2014-02-07 17:23 PST, Martin Hock
ap: review+
ap: commit-queue-
Details | Formatted Diff | Diff
rename method (3.14 KB, patch)
2014-02-10 10:10 PST, Martin Hock
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Hock 2014-02-07 16:11:23 PST
Add API for accessing session from a page or bundle.
Comment 1 Martin Hock 2014-02-07 16:17:07 PST
Created attachment 223510 [details]
patch
Comment 2 Alexey Proskuryakov 2014-02-07 16:57:42 PST
Comment on attachment 223510 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=223510&action=review

I think that it is tricky to expose a session API when we don't really track sessions internally yet.

I would suggest not doing anything in UIProcess API, and exposing WKBundlePageIsUsingEphemeralSession in bundle API.

> Source/WebKit2/UIProcess/API/C/WKPage.cpp:1496
> +    return toAPI(session.release().leakRef());

An API function that returns something that should be released should have "create" or "copy" in the name, not "get".

This one would be "WKPageCopySession" (but note the above, let's not add it yet).

> Source/WebKit2/UIProcess/APISession.h:39
> +    static PassRefPtr<Session> fromID(uint64_t sessionID);

This looks like something SessionTracker would be responsible for, not Session itself.

> Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundlePage.cpp:548
> +    RefPtr<API::Session> session = API::Session::fromID(toImpl(pageRef)->sessionID());

UI process API and bundle API are different. APISession.h file is in UIProcess directory, so it cannot be used here.

You have this header indirectly included via WKSharedAPICast.h. But it shouldn't be there, as WKSession.h itself is in UIProcess. The include should be in WKAPICast.h instead.
Comment 3 Martin Hock 2014-02-07 17:23:22 PST
Created attachment 223523 [details]
expose just the bool from bundle
Comment 4 Alexey Proskuryakov 2014-02-08 02:24:02 PST
Comment on attachment 223523 [details]
expose just the bool from bundle

View in context: https://bugs.webkit.org/attachment.cgi?id=223523&action=review

> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:2110
> +bool WebPage::isEphemeral() const

Why not use the same name, isUsingEphemeralSession()? "Page is ephemeral" is not a concept we have.
Comment 5 Martin Hock 2014-02-10 10:10:22 PST
Created attachment 223722 [details]
rename method
Comment 6 WebKit Commit Bot 2014-02-10 10:54:43 PST
Comment on attachment 223722 [details]
rename method

Clearing flags on attachment: 223722

Committed r163800: <http://trac.webkit.org/changeset/163800>
Comment 7 WebKit Commit Bot 2014-02-10 10:54:45 PST
All reviewed patches have been landed.  Closing bug.