Start of support for multiple WebsiteDataStore/SessionIDs per process

• Component: WebKit2
• Status: RESOLVED FIXED

[Brady Eidson]

Start of support for multiple WebsiteDataStore/SessionIDs per process

[Brady Eidson]

Created attachment 308508 [details]
Preliminary patch

No ChangeLogs because this isn't quite ready to land (testing things locally), but review can start!

[Alex Christensen]

Comment on attachment 308508 [details]
Preliminary patch

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

We should make a test that makes a WKWebView with a persistent cookie storage at a known location, close the WKWebView and destroy the session, then make another WKWebView with a persistent cookie storage at the same known location and make sure that the cookies are there.  I could imagine some problems with closing cookie storages that such a test would help prevent.

> Source/WebKit2/UIProcess/WebsiteData/Cocoa/WebsiteDataStoreCocoa.mm:71
> +#if PLATFORM(IOS)

Why only iOS and not Mac?

> Source/WebKit2/WebProcess/WebCoreSupport/mac/WebFrameNetworkingContext.mm:86
> +#if PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101100

Why only Mac and not iOS?

> Source/WebKit2/WebProcess/WebProcess.cpp:521
> +void WebProcess::destroySessionOrWebsiteDataStore(SessionID sessionID)

I think destroySession would be a better name.

[Build Bot]

Comment on attachment 308508 [details]
Preliminary patch

Attachment 308508 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/3625056

New failing tests:
imported/w3c/web-platform-tests/html/semantics/tabular-data/processing-model-1/span-limits.html

[Build Bot]

Created attachment 308512 [details]
Archive of layout-test-results from ews112 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Brady Eidson]

(In reply to Alex Christensen from comment #2)
> Comment on attachment 308508 [details]
> Preliminary patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=308508&action=review
> 
> We should make a test that makes a WKWebView with a persistent cookie
> storage at a known location, close the WKWebView and destroy the session,
> then make another WKWebView with a persistent cookie storage at the same
> known location and make sure that the cookies are there.  I could imagine
> some problems with closing cookie storages that such a test would help
> prevent.
> 
> > Source/WebKit2/UIProcess/WebsiteData/Cocoa/WebsiteDataStoreCocoa.mm:71
> > +#if PLATFORM(IOS)
> 
> Why only iOS and not Mac?

Per NetworkProcessCreationParameters and WebProcessCreationParameters, we currently only need sandbox handle for the cookie path on iOS.

> > Source/WebKit2/WebProcess/WebCoreSupport/mac/WebFrameNetworkingContext.mm:86
> > +#if PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101100
> 
> Why only Mac and not iOS?

Per NetworkProcessCreationParameters and WebProcessCreationParameters, we currently only need the identifying data on Mac

> > Source/WebKit2/WebProcess/WebProcess.cpp:521
> > +void WebProcess::destroySessionOrWebsiteDataStore(SessionID sessionID)
> 
> I think destroySession would be a better name.

K.

[Brady Eidson]

Created attachment 308538 [details]
Patch

[Build Bot]

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[Build Bot]

Attachment 308538 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 30 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Brady Eidson]

Created attachment 308540 [details]
Patch

[Brady Eidson]

Created attachment 308542 [details]
Patch

[Brady Eidson]

Created attachment 308552 [details]
Patch

[Geoffrey Garen]

Comment on attachment 308552 [details]
Patch

r=me

[Brady Eidson]

Created attachment 308555 [details]
Patch

[Brady Eidson]

Created attachment 308561 [details]
Patch

[Brady Eidson]

Created attachment 308562 [details]
Patch

[Brady Eidson]

Created attachment 308587 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 308587 [details]
Patch

Rejecting attachment 308587 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'validate-changelog', '--check-oops', '--non-interactive', 308587, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

ChangeLog entry in Tools/ChangeLog contains OOPS!.

Full output: http://webkit-queues.webkit.org/results/3628567

[Brady Eidson]

Created attachment 308588 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 308588 [details]
Patch

Clearing flags on attachment: 308588

Committed r215941: <http://trac.webkit.org/changeset/215941>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Alexey Proskuryakov]

I suspect that this broke all perf tests: https://build.webkit.org/builders/Apple%20El%20Capitan%20Release%20WK2%20%28Perf%29/builds/296/steps/perf-test/logs/stdio

[Ryan Haddad]

This also caused an API test failure on ios-simulator: https://bugs.webkit.org/show_bug.cgi?id=171513