Bug 172558 – REGRESSION (r216977): 6 leaks introduced in new WebKit2_WKHTTPCookieStoreWithoutProcessPool_Test

• Status: REOPENED
• Component: WebKit2

[David Kilzer (:ddkilzer)]

Need the bug URL (OOPS!).

Reviewed by NOBODY (OOPS!).

* TestWebKitAPI/Tests/WebKit2Cocoa/WKHTTPCookieStore.mm:
(WebKit2_WKHTTPCookieStoreWithoutProcessPool_Test): Fix the leaks.
---
 2 files changed, 18 insertions(+), 6 deletions(-)

[David Kilzer (:ddkilzer)]

Created attachment 311164 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 311164 [details]
Patch

Rejecting attachment 311164 [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-02', 'build', '--no-clean', '--no-update', '--build-style=release', '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
ed 'websiteDataStore' in 'WTF::RetainPtr<WKWebViewConfiguration>'
            configuration.websiteDataStore = defaultStore;
            ~~~~~~~~~~~~~ ^
2 errors generated.

** BUILD FAILED **


The following build commands failed:
	CompileC /Volumes/Data/EWS/WebKit/WebKitBuild/TestWebKitAPI.build/Release/TestWebKitAPILibrary.build/Objects-normal/x86_64/WKHTTPCookieStore.o Tests/WebKit2Cocoa/WKHTTPCookieStore.mm normal x86_64 objective-c++ com.apple.compilers.llvm.clang.1_0.compiler
(1 failure)

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

[David Kilzer (:ddkilzer)]

Created attachment 311185 [details]
Patch v2

[Sam Weinig]

Comment on attachment 311185 [details]
Patch v2

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

> Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKHTTPCookieStore.mm:196
> +            RetainPtr<WKWebViewConfiguration> configuration = adoptNS([[WKWebViewConfiguration alloc] init]);

auto?

> Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKHTTPCookieStore.mm:199
> +            RetainPtr<CookieUIDelegate> delegate = adoptNS([[CookieUIDelegate alloc] init]);

auto?

> Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKHTTPCookieStore.mm:215
> +            RetainPtr<WKWebViewConfiguration> configuration = adoptNS([[WKWebViewConfiguration alloc] init]);

auto?

> Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKHTTPCookieStore.mm:218
> +            RetainPtr<CookieUIDelegate> delegate = adoptNS([[CookieUIDelegate alloc] init]);

auto?

[David Kilzer (:ddkilzer)]

Committed r217423: <http://trac.webkit.org/changeset/217423>

[David Kilzer (:ddkilzer)]

Fix typo.

Committed r217424: <http://trac.webkit.org/changeset/217424>

[WebKit Commit Bot]

Re-opened since this is blocked by bug 172607

[Matt Lewis]

These two revisions:
https://trac.webkit.org/changeset/217424/webkit
https://trac.webkit.org/changeset/217424/webkit

Caused an api failure for: WKHTTPCookieStoreWithoutProcessPool
This was occurring on all platforms.

https://build.webkit.org/builders/Apple%20Sierra%20Release%20WK2%20(Tests)/builds/1765/steps/run-api-tests/logs/stdio
https://build.webkit.org/builders/Apple%20Sierra%20Release%20WK2%20(Tests)/builds/1765

[David Kilzer (:ddkilzer)]

Alex, do you know why this would have caused a timeout?

[David Kilzer (:ddkilzer)]

(In reply to David Kilzer (:ddkilzer) from comment #9)
> Alex, do you know why this would have caused a timeout?

Ha!  I think the problem may be that WKWebView.UIDelegate is a weak pointer and that CookieUIDelegate no longer leaks (and is deallocated at the end of its block), such that the UIDelegate callback never happens and the test times out.

We probably just need a place to assign the CookieUIDelegate so that it's not marked as a leak and not released before the end of the test.

[David Kilzer (:ddkilzer)]

I didn't notice this before, but WebKit2.WKHTTPCookieStoreWithoutProcessPool also leaks two WKWebView objects.

[David Kilzer (:ddkilzer)]

Created attachment 311418 [details]
Patch v2

[David Kilzer (:ddkilzer)]

Comment on attachment 311418 [details]
Patch v2

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

> Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKHTTPCookieStore.mm:209
> +    delegate = nil;
> +    // FIXME: Setting webView = nil here causes persistent test to fail on macOS.

Note the FIXME here.  I don't understand why setting webView to nil here causes the next test to fail.  Seems like a real bug.

[Build Bot]

Comment on attachment 311418 [details]
Patch v2

Attachment 311418 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3828019

New failing tests:
http/tests/cache/cancel-during-revalidation-succeeded.html

[Build Bot]

Created attachment 311420 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.5