RESOLVED FIXED167532
IconLoadingClient API doesn't work asynchronously
https://bugs.webkit.org/show_bug.cgi?id=167532
Summary IconLoadingClient API doesn't work asynchronously
Brady Eidson
Reported 2017-01-27 15:55:56 PST
IconLoadingClient API doesn't work asynchronously This is because the WTF::Function is not properly moved into the objective-C block, and in fact it can't be; Blocks must be copyable. Making it a std::function instead works.
Attachments
Patch (5.83 KB, patch)
2017-01-27 16:00 PST, Brady Eidson
achristensen: review+
Brady Eidson
Comment 1 2017-01-27 15:57:48 PST
Brady Eidson
Comment 2 2017-01-27 16:00:04 PST
WebKit Commit Bot
Comment 3 2017-01-27 16:02:56 PST
Attachment 299972 [details] did not pass style-queue: ERROR: Source/WebKit2/UIProcess/Cocoa/IconLoadingDelegate.h:58: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit2/UIProcess/Cocoa/IconLoadingDelegate.mm:74: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit2/UIProcess/WebPageProxy.cpp:6731: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit2/UIProcess/API/mac/WKView.mm:874: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebKit2/UIProcess/API/mac/WKView.mm:874: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit2/UIProcess/API/APIIconLoadingClient.h:42: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/WebKit2/UIProcess/API/APIIconLoadingClient.h:42: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 7 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.
Brady Eidson
Comment 4 2017-01-27 16:13:42 PST
Alex gave in-person r+ with a small change. https://trac.webkit.org/changeset/211310
Sam Weinig
Comment 5 2017-01-27 20:42:59 PST
I don't understand what the issue is here. In my testing, WTF::Function works fine with blocks. Can you give an example where it doesn't work?
Alex Christensen
Comment 6 2017-01-27 22:24:28 PST
Comment on attachment 299972 [details] Patch It doesn't work with blocks that are copied or not called immediately.
Sam Weinig
Comment 7 2017-01-28 08:57:43 PST
(In reply to comment #6) > Comment on attachment 299972 [details] > Patch > > It doesn't work with blocks that are copied or not called immediately. Can you give me an example? I made some unit tests that all seem to work when copying a block, but perhaps I am missing something. e.g. this works: TEST(FunctionBlock, BlockCopyAsync) { WTF::initializeMainThread(); typedef void (^BlockType)(); __block bool value = false; BlockType block = ^{ value = true; }; Function<void()> function((BlockType)[block copy]); callOnMainThread(WTFMove(function)); TestWebKitAPI::Util::run(&value); ASSERT_TRUE(value); }
Note You need to log in before you can comment on or make changes to this bug.