Use CoreAnimation fences instead of synchronous IPC to synchronize resize
Created attachment 255521 [details] Patch
Comment on attachment 255521 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=255521&action=review Looks OK. Some problems that need to be fixed before landing. > Source/WebKit2/UIProcess/API/mac/WKView.mm:4438 > + // This SPI is only used on 10.9 and below, and is incompatible with the fence-based drawing area size synchronization in 10.10+. Maybe ASSERT_NOT_REACHED? > Source/WebKit2/UIProcess/API/mac/WKView.mm:4439 > +#if __MAC_OS_X_VERSION_MIN_REQUIRED <= 1090 Given the comment above, maybe #if !HAVE(COREANIMATION_FENCES) is a better thing to write here? > Source/WebKit2/UIProcess/API/mac/WKView.mm:4454 > + // This SPI is only used on 10.9 and below, and is incompatible with the fence-based drawing area size synchronization in 10.10+. Maybe ASSERT_NOT_REACHED? > Source/WebKit2/UIProcess/API/mac/WKView.mm:4455 > +#if __MAC_OS_X_VERSION_MIN_REQUIRED <= 1090 Given the comment above, maybe #if !HAVE(COREANIMATION_FENCES) is a better thing to write here? > Source/WebKit2/UIProcess/mac/TiledCoreAnimationDrawingAreaProxy.mm:168 > + m_webPageProxy.process().send(Messages::DrawingArea::UpdateGeometry(m_size, m_layerPosition, true, fencePort), m_webPageProxy.pageID()); > + > } Stray blank line here. I suggest omitting it. > Source/WebKit2/WebProcess/WebPage/DrawingArea.h:35 > +#include <WebCore/MachSendRight.h> This needs to be inside PLATFORM(COCOA) to avoid breaking GTK and EFL builds. I think it would be better just to forward declare as "class MachSendRight", since we only need to compile "const WebCore::MachSendRight&", and a forward declaration is sufficient for that. And those don’t necessarily need to be guarded with PLATFORM(COCOA). I don’t mind leaving the #if out, although some prefer that because they think it’s worth the extra complexity and ugliness to guarantee a compile time error if we try to use the type name at all.
Created attachment 256467 [details] Patch
> > Source/WebKit2/UIProcess/API/mac/WKView.mm:4439 > > +#if __MAC_OS_X_VERSION_MIN_REQUIRED <= 1090 > > Given the comment above, maybe #if !HAVE(COREANIMATION_FENCES) is a better > thing to write here? I didn't do this because the code is really not used on 10.10, and leaving it this way means we'll get to delete it sooner :) I made all your other changes, and added a lot more code re: the invalidation of the fences if a sync message arrives. I think this needs an andersca review because of the IPC changes.
Attachment 256467 [details] did not pass style-queue: ERROR: Source/WebKit2/Platform/IPC/Connection.h:203: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit2/Platform/IPC/Connection.h:310: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit2/Platform/IPC/Connection.cpp:708: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 3 in 20 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 256468 [details] fix the build
Attachment 256468 [details] did not pass style-queue: ERROR: Source/WebKit2/Platform/IPC/Connection.h:203: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit2/Platform/IPC/Connection.h:310: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit2/Platform/IPC/Connection.cpp:708: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 3 in 20 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 256471 [details] Patch
Attachment 256471 [details] did not pass style-queue: ERROR: Source/WebKit2/Platform/IPC/Connection.h:203: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit2/Platform/IPC/Connection.h:310: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit2/Platform/IPC/Connection.cpp:708: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 3 in 20 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 256471 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=256471&action=review > Source/WebCore/platform/cocoa/MachSendRight.h:38 > + WEBCORE_EXPORT MachSendRight() = default; This doesn't need WEBCORE_EXPORT. > Source/WebKit2/Platform/IPC/Connection.cpp:221 > + , m_incomingSyncMessageCallbackQueue(WorkQueue::create("com.apple.WebKit.IPC.IncomingSyncMessageCallbackQueue")) This should be made on demand, there's no point in creating it for all the various connections that don't use this feature. > Source/WebKit2/Platform/IPC/Connection.cpp:713 > + std::lock_guard<std::mutex> lock(m_incomingSyncMessageCallbackMutex); > + m_incomingSyncMessageCallbacks.add(m_nextIncomingSyncMessageCallbackID, callback); > + > + return m_nextIncomingSyncMessageCallbackID++; Please change this to get the callback ID before incrementing it so you can make it start at 0. > Source/WebKit2/Platform/IPC/Connection.cpp:732 > + for (auto it = m_incomingMessages.begin(), end = m_incomingMessages.end(); it != end; ++it) { > + std::unique_ptr<MessageDecoder>& message = *it; > + if (message->isSyncMessage()) > + hasIncomingSyncMessage = true; > + } Modern for-loop please! > Source/WebKit2/Platform/IPC/Connection.h:312 > + uint64_t m_nextIncomingSyncMessageCallbackID { 1 }; Should start at zero here.
http://trac.webkit.org/changeset/186662
Reopening to attach new patch.
Created attachment 256617 [details] tiny followup
And http://trac.webkit.org/changeset/186693