[webkit-reviews] review granted: [Bug 207683] Replace the usages of (IPC::Attachment fencePort) with IPC::MachPort : [Attachment 409310] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Sep 21 13:44:24 PDT 2020


Daniel Bates <dbates at webkit.org> has granted Peng Liu <peng.liu6 at apple.com>'s
request for review:
Bug 207683: Replace the usages of (IPC::Attachment fencePort) with
IPC::MachPort
https://bugs.webkit.org/show_bug.cgi?id=207683

Attachment 409310: Patch

https://bugs.webkit.org/attachment.cgi?id=409310&action=review




--- Comment #2 from Daniel Bates <dbates at webkit.org> ---
Comment on attachment 409310
  --> https://bugs.webkit.org/attachment.cgi?id=409310
Patch

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

This patch looks good.

> Source/WebKit/UIProcess/Cocoa/VideoFullscreenManagerProxy.mm:795
> +    if (DrawingAreaProxy* drawingArea = m_page->drawingArea())

OK as-is. auto* could be used here too.

> Source/WebKit/UIProcess/Cocoa/VideoFullscreenManagerProxy.mm:799
> +   
m_page->send(Messages::VideoFullscreenManager::SetVideoLayerFrameFenced(context
Id, frame, fenceSendRight));

OK as-is.

> Source/WebKit/WebProcess/WebPage/WebPage.cpp:3176
> +void WebPage::setTopContentInsetFenced(float contentInset, const
WTF::MachSendRight& machSendRight)

OK as-is. Could be improved a tiny bit by taking by rvalue reference then need
to patch up other places in this patch though. The param name sounds a bit
ambiguous too and same comment for same param in WebProcess code.

> Source/WebKit/WebProcess/WebPage/WebPage.cpp:3178
> +    m_drawingArea->addFence(machSendRight);

OK as-is. If ^^^ taken then could patch up addFence to take by rvalue reference
and move into.


More information about the webkit-reviews mailing list