WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
201596
Stop using testRunner.setPrivateBrowsingEnabled_DEPRECATED() in http/tests/adClickAttribution/conversion-disabled-in-ephemeral-session.html
https://bugs.webkit.org/show_bug.cgi?id=201596
Summary
Stop using testRunner.setPrivateBrowsingEnabled_DEPRECATED() in http/tests/ad...
Chris Dumez
Reported
2019-09-08 17:37:19 PDT
Stop using testRunner.setPrivateBrowsingEnabled_DEPRECATED() in http/tests/adClickAttribution/conversion-disabled-in-ephemeral-session.html as it is a hack which does not work truly using a private session in WebKit2.
Attachments
Patch
(29.42 KB, patch)
2019-09-08 18:49 PDT
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Patch
(30.49 KB, patch)
2019-09-09 15:51 PDT
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Chris Dumez
Comment 1
2019-09-08 18:49:12 PDT
Created
attachment 378343
[details]
Patch
John Wilander
Comment 2
2019-09-09 15:00:43 PDT
Comment on
attachment 378343
[details]
Patch Changes to the test case look good. Do we have a suitable reviewer for the TestRunner changes?
Chris Dumez
Comment 3
2019-09-09 15:01:21 PDT
(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.
Alex Christensen
Comment 4
2019-09-09 15:05:25 PDT
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
Chris Dumez
Comment 5
2019-09-09 15:36:14 PDT
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.
Chris Dumez
Comment 6
2019-09-09 15:51:48 PDT
Created
attachment 378413
[details]
Patch
WebKit Commit Bot
Comment 7
2019-09-09 16:35:18 PDT
Comment on
attachment 378413
[details]
Patch Clearing flags on attachment: 378413 Committed
r249675
: <
https://trac.webkit.org/changeset/249675
>
WebKit Commit Bot
Comment 8
2019-09-09 16:35:20 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 9
2019-09-09 16:36:21 PDT
<
rdar://problem/55203697
>
Ryan Haddad
Comment 10
2019-09-09 17:32:30 PDT
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
Alex Christensen
Comment 11
2019-09-09 21:51:15 PDT
http://trac.webkit.org/r249705
Chris Dumez
Comment 12
2019-09-09 21:56:06 PDT
(In reply to Alex Christensen from
comment #11
)
>
http://trac.webkit.org/r249705
Thanks.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug