Summary: | Stop using testRunner.setPrivateBrowsingEnabled_DEPRECATED() in http/tests/adClickAttribution/conversion-disabled-in-ephemeral-session.html | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||||
Component: | Tools / Tests | Assignee: | Chris Dumez <cdumez> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | achristensen, beidson, commit-queue, ggaren, webkit-bot-watchers-bugzilla, webkit-bug-importer, wilander, youennf | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 201546 | ||||||||
Attachments: |
|
Description
Chris Dumez
2019-09-08 17:37:19 PDT
Created attachment 378343 [details]
Patch
Comment on attachment 378343 [details]
Patch
Changes to the test case look good. Do we have a suitable reviewer for the TestRunner changes?
(In reply to John Wilander from comment #2) > Comment on attachment 378343 [details] > Patch > > Changes to the test case look good. Do we have a suitable reviewer for the > TestRunner changes? I asked Alex to take a look. Thanks for double checking the test. Comment on attachment 378343 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=378343&action=review > Source/WebKit/UIProcess/API/C/WKFramePolicyListener.cpp:-58 > - RELEASE_ASSERT_WITH_MESSAGE(sessionID.isEphemeral() || sessionID == PAL::SessionID::defaultSessionID(), "If WebsitePolicies specifies a WebsiteDataStore, the data store's session must be default or non-persistent."); Please remove the corresponding check in the ObjC API. It's also no longer needed. > Tools/DumpRenderTree/TestRunner.cpp:2567 > + ASSERT(!(m_shouldSwapToEphemeralSessionOnNextNavigation && m_shouldSwapToDefaultSessionOnNextNavigation)); ditto with comment below. > Tools/WebKitTestRunner/TestController.cpp:2822 > + ASSERT(!(shouldSwapToEphemeralSessionOnNextNavigation && shouldSwapToDefaultSessionOnNextNavigation)); You could just assert that they are unequal. > Tools/WebKitTestRunner/TestController.cpp:2824 > + WKRetainPtr<WKWebsiteDataStoreRef> newSession = TestController::websiteDataStore(); There's no need to retain this. Just use a plain old WKWebsiteDataStoreRef Comment on attachment 378343 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=378343&action=review >> Tools/WebKitTestRunner/TestController.cpp:2824 >> + WKRetainPtr<WKWebsiteDataStoreRef> newSession = TestController::websiteDataStore(); > > There's no need to retain this. Just use a plain old WKWebsiteDataStoreRef This is needed for the assignment of WKWebsiteDataStoreCreateNonPersistentDataStore() 2 lines below. Created attachment 378413 [details]
Patch
Comment on attachment 378413 [details] Patch Clearing flags on attachment: 378413 Committed r249675: <https://trac.webkit.org/changeset/249675> All reviewed patches have been landed. Closing bug. It appears that this change introduced an API test failure: TestWebKitAPI.WebKit.UpdateWebsitePoliciesInvalid _RegisterApplication(), FAILED TO establish the default connection to the WindowServer, _CGSDefaultConnection() is NULL. _RegisterApplication(), FAILED TO establish the default connection to the WindowServer, _CGSDefaultConnection() is NULL. /Volumes/Data/slave/mojave-release/build/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsitePolicies.mm:1546 Value of: sawException Actual: false Expected: true https://build.webkit.org/builders/Apple%20Mojave%20Release%20WK2%20(Tests)/builds/6506 (In reply to Alex Christensen from comment #11) > http://trac.webkit.org/r249705 Thanks. |