<rdar://problem/73901967>
Created attachment 422637 [details] Patch
<rdar://problem/75194080>
Created attachment 422640 [details] Fix non-internal builds
Comment on attachment 422640 [details] Fix non-internal builds View in context: https://bugs.webkit.org/attachment.cgi?id=422640&action=review r=me > Source/WebCore/page/ContextMenuContext.h:47 > + IntRect selectionBounds() const { return m_selectionBounds; } `const IntRect&` > Source/WebCore/page/ContextMenuController.cpp:846 > + auto selectedString = m_context.hitTestResult().selectedText(); NIT: I dunno if it's actually more efficient/performant/etc, but I usually mark stuff like this as `auto&`. > Source/WebCore/page/ContextMenuController.cpp:849 > + if (auto view = makeRefPtr(frame->view()); view && !selectedString.isEmpty()) { Is it worth putting this inside a `if (!selectedString.isEmpty()) { ... }` to avoid churning the ref count if not? > Source/WebKit/Shared/ContextMenuContextData.h:64 > + WebCore::IntRect selectionBounds() const { return m_selectionBounds; } `const WebCore::IntRect&`
Comment on attachment 422640 [details] Fix non-internal builds View in context: https://bugs.webkit.org/attachment.cgi?id=422640&action=review >> Source/WebCore/page/ContextMenuContext.h:47 >> + IntRect selectionBounds() const { return m_selectionBounds; } > > `const IntRect&` 👍🏻 >> Source/WebCore/page/ContextMenuController.cpp:846 >> + auto selectedString = m_context.hitTestResult().selectedText(); > > NIT: I dunno if it's actually more efficient/performant/etc, but I usually mark stuff like this as `auto&`. 👍🏻 >> Source/WebCore/page/ContextMenuController.cpp:849 >> + if (auto view = makeRefPtr(frame->view()); view && !selectedString.isEmpty()) { > > Is it worth putting this inside a `if (!selectedString.isEmpty()) { ... }` to avoid churning the ref count if not? Good call! >> Source/WebKit/Shared/ContextMenuContextData.h:64 >> + WebCore::IntRect selectionBounds() const { return m_selectionBounds; } > > `const WebCore::IntRect&` 👍🏻
> >> Source/WebCore/page/ContextMenuController.cpp:846 > >> + auto selectedString = m_context.hitTestResult().selectedText(); > > > > NIT: I dunno if it's actually more efficient/performant/etc, but I usually mark stuff like this as `auto&`. > > 👍🏻 Actually, never mind — this doesn't work here because selectedText() returns a temporary value.
Created attachment 422651 [details] For EWS
Committed r274148: <https://commits.webkit.org/r274148> All reviewed patches have been landed. Closing bug and clearing flags on attachment 422651 [details].