Bug 185262 - Assertion failure in NetworkStorageSession::setCookie: privilege of UI process is not set
Summary: Assertion failure in NetworkStorageSession::setCookie: privilege of UI proces...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit API (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sihui Liu
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-05-03 12:08 PDT by Sihui Liu
Modified: 2018-05-07 17:34 PDT (History)
5 users (show)

See Also:


Attachments
Patch (1.56 KB, patch)
2018-05-03 14:05 PDT, Sihui Liu
no flags Details | Formatted Diff | Diff
Patch (1.64 KB, patch)
2018-05-03 17:11 PDT, Sihui Liu
no flags Details | Formatted Diff | Diff
Patch (1.83 KB, patch)
2018-05-03 20:21 PDT, Sihui Liu
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sihui Liu 2018-05-03 12:08:09 PDT
If setting cookie using defaultDataStore before creating a WebView, the app would run into this assertion failure, because the privilege of UI process to access cookie API is not granted yet.
Comment 1 Sihui Liu 2018-05-03 14:05:04 PDT
Created attachment 339463 [details]
Patch
Comment 2 Chris Dumez 2018-05-03 14:12:01 PDT
Comment on attachment 339463 [details]
Patch

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

> Source/WebKit/ChangeLog:3
> +        Assertion failure in NetworkStorageSession::setCookie: privilege of UI process is not set

What assertion? Could you please provide the crash trace on the bug?

> Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStore.mm:50
> +    WTF::setProcessPrivileges(allPrivileges());

This should probably inside WebKit::WebSiteDataStore constructor so it works for C API clients as well, and other APIs that defaultDataStore.
Comment 3 Sihui Liu 2018-05-03 16:44:51 PDT
(In reply to Chris Dumez from comment #2)
> Comment on attachment 339463 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=339463&action=review
> 
> > Source/WebKit/ChangeLog:3
> > +        Assertion failure in NetworkStorageSession::setCookie: privilege of UI process is not set
> 
> What assertion? Could you please provide the crash trace on the bug?
> 
Crash trace:
default	16:36:00.400052 -0700	WKCookieMonster	ASSERTION FAILED: hasProcessPrivilege(ProcessPrivilege::CanAccessRawCookies)
default	16:36:00.400101 -0700	WKCookieMonster	platform/network/cocoa/NetworkStorageSessionCocoa.mm(40) : void WebCore::NetworkStorageSession::setCookie(const WebCore::Cookie &)
default	16:36:00.406075 -0700	WKCookieMonster	1   0x115ffc259 WTFCrash
default	16:36:00.423040 -0700	WKCookieMonster	2   0x106fcaa9e WebCore::NetworkStorageSession::setCookie(WebCore::Cookie const&)
default	16:36:00.428838 -0700	WKCookieMonster	3   0x100399f55 API::HTTPCookieStore::setCookie(WebCore::Cookie const&, WTF::Function<void ()>&&)
default	16:36:00.430665 -0700	WKCookieMonster	4   0x10135ea23 -[WKHTTPCookieStore setCookie:completionHandler:]
default	16:36:00.430741 -0700	WKCookieMonster	5   0x100008260 _T015WKCookieMonster14ViewControllerC04loadC0yyF
default	16:36:00.430766 -0700	WKCookieMonster	6   0x100009144 _T015WKCookieMonster14ViewControllerC04loadC0yyFTo
default	16:36:00.430979 -0700	WKCookieMonster	7   0x7fff514ecb96 -[NSViewController _loadViewIfRequired]
default	16:36:00.431139 -0700	WKCookieMonster	8   0x7fff514ecb01 -[NSViewController view]
default	16:36:00.431268 -0700	WKCookieMonster	9   0x7fff5166b43d -[NSWindow _contentViewControllerChanged]
default	16:36:00.431346 -0700	WKCookieMonster	10  0x7fff56025e8e -[NSObject(NSKeyValueCoding) setValue:forKey:]
default	16:36:00.431473 -0700	WKCookieMonster	11  0x7fff516ae189 -[NSWindow setValue:forKey:]
default	16:36:00.431601 -0700	WKCookieMonster	12  0x7fff516ae0ed -[NSIBUserDefinedRuntimeAttributesConnector establishConnection]
default	16:36:00.431722 -0700	WKCookieMonster	13  0x7fff5147a7e5 -[NSIBObjectData nibInstantiateWithOwner:options:topLevelObjects:]
default	16:36:00.431846 -0700	WKCookieMonster	14  0x7fff51575e0e -[NSNib _instantiateNibWithExternalNameTable:options:]
default	16:36:00.431970 -0700	WKCookieMonster	15  0x7fff51575a6a -[NSNib _instantiateWithOwner:options:topLevelObjects:]
default	16:36:00.432128 -0700	WKCookieMonster	16  0x7fff51ccc4e8 -[NSStoryboard instantiateControllerWithIdentifier:]
default	16:36:00.432247 -0700	WKCookieMonster	17  0x7fff5146eaf3 NSApplicationMain
default	16:36:00.432288 -0700	WKCookieMonster	18  0x10000e41d main
default	16:36:00.432314 -0700	WKCookieMonster	19  0x7fff7c39f015 start

---------------
In loadView():
 let cookieStore = WKWebsiteDataStore.default().httpCookieStore
 cookieStore.setCookie(cookie) {...}
Comment 4 Sihui Liu 2018-05-03 17:11:23 PDT
Created attachment 339490 [details]
Patch
Comment 5 Chris Dumez 2018-05-03 18:27:24 PDT
Comment on attachment 339490 [details]
Patch

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

> Source/WebKit/UIProcess/API/APIWebsiteDataStore.cpp:70
> +    WTF::setProcessPrivileges(allPrivileges());

I said WebKit::WebsiteDataStore not the API one :) the existing call in in ProcessPool, not its API object.
Comment 6 Sihui Liu 2018-05-03 20:21:07 PDT
Created attachment 339511 [details]
Patch
Comment 7 WebKit Commit Bot 2018-05-04 09:51:21 PDT
Comment on attachment 339511 [details]
Patch

Clearing flags on attachment: 339511

Committed r231356: <https://trac.webkit.org/changeset/231356>
Comment 8 WebKit Commit Bot 2018-05-04 09:51:23 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Radar WebKit Bug Importer 2018-05-04 09:52:18 PDT
<rdar://problem/39979381>
Comment 10 Geoffrey Garen 2018-05-04 14:39:45 PDT
It should be possible to test this with an API test that sets a cookie before creating a WebView. Subtle cases like this are extra valuable to test.
Comment 11 Sihui Liu 2018-05-07 17:34:34 PDT
(In reply to Geoffrey Garen from comment #10)
> It should be possible to test this with an API test that sets a cookie
> before creating a WebView. Subtle cases like this are extra valuable to test.

Yes, a reason we bypassed this case with API test is that process privileges were granted to TestController in initialization. I added an API test for this case: https://bugs.webkit.org/show_bug.cgi?id=185406.