Bug 147139 - API::Session should clean up its storage in the network process when destroyed.
Summary: API::Session should clean up its storage in the network process when destroyed.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Andreas Kling
URL:
Keywords: InRadar
: 147138 (view as bug list)
Depends on:
Blocks:
 
Reported: 2015-07-20 19:54 PDT by Andreas Kling
Modified: 2015-07-21 12:55 PDT (History)
4 users (show)

See Also:

Attachments
Proposed patch (1.65 KB, patch)
2015-07-20 20:02 PDT, Andreas Kling 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas Kling 2015-07-20 19:54:03 PDT 
<rdar://problem/21838764>
Comment 1 Andreas Kling 2015-07-20 20:02:18 PDT 
Created attachment 257157 [details]
Proposed patch
Comment 2 Chris Dumez 2015-07-20 20:03:54 PDT 
*** Bug 147138 has been marked as a duplicate of this bug. ***
Comment 3 Alexey Proskuryakov 2015-07-20 20:17:12 PDT 
Comment on attachment 257157 [details]
Proposed patch

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

> Source/WebKit2/ChangeLog:3
> +        API::Session should clean up its storage in the network process when destroyed.

Shouldn't it do the same for WebContent processes?
Comment 4 Andreas Kling 2015-07-20 20:27:52 PDT 
Comment on attachment 257157 [details]
Proposed patch

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

>> Source/WebKit2/ChangeLog:3
>> +        API::Session should clean up its storage in the network process when destroyed.
> 
> Shouldn't it do the same for WebContent processes?

Hm, for the WebContent processes I only see calls that send:

    Messages::WebProcess::EnsurePrivateBrowsingSession(SessionID::legacyPrivateSessionID())

So they always use the same SessionID, never anything created by WKSessionCreate()
Comment 5 Alexey Proskuryakov 2015-07-20 20:39:08 PDT 
Comment on attachment 257157 [details]
Proposed patch

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

>>> Source/WebKit2/ChangeLog:3
>>> +        API::Session should clean up its storage in the network process when destroyed.
>> 
>> Shouldn't it do the same for WebContent processes?
> 
> Hm, for the WebContent processes I only see calls that send:
> 
>     Messages::WebProcess::EnsurePrivateBrowsingSession(SessionID::legacyPrivateSessionID())
> 
> So they always use the same SessionID, never anything created by WKSessionCreate()

It looks like WebContent processes get the session ID using WebPage::SetSessionID messages. I don't know why it's done differently for these processes now, would consult with Anders.
Comment 6 Anders Carlsson 2015-07-21 10:36:46 PDT 
(In reply to comment #5)
> Comment on attachment 257157 [details]

> It looks like WebContent processes get the session ID using
> WebPage::SetSessionID messages. I don't know why it's done differently for
> these processes now, would consult with Anders.

I think it's less important for the web processes since they have a much shorter lifespan (usually).

Going forward, WKSessionRef should go away in favor of WebsiteDataStore, and it should be impossible to change sessions after the fact (since we haven't supported that feature since Mountain Lion).
Comment 7 Andreas Kling 2015-07-21 12:01:58 PDT 
Comment on attachment 257157 [details]
Proposed patch

Thanks for the comment, Anders. cq+'ing.
Comment 8 WebKit Commit Bot 2015-07-21 12:55:42 PDT 
Comment on attachment 257157 [details]
Proposed patch

Clearing flags on attachment: 257157

Committed r187115: <http://trac.webkit.org/changeset/187115>
Comment 9 WebKit Commit Bot 2015-07-21 12:55:47 PDT 
All reviewed patches have been landed.  Closing bug.