[webkit-reviews] review denied: [Bug 206099] Deploy Ref and RefPtr in DOMWindow::scroll* functions : [Attachment 387396] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Jan 10 17:41:40 PST 2020


Simon Fraser (smfr) <simon.fraser at apple.com> has denied Jack
<shihchieh_lee at apple.com>'s request for review:
Bug 206099: Deploy Ref and RefPtr in DOMWindow::scroll* functions
https://bugs.webkit.org/show_bug.cgi?id=206099

Attachment 387396: Patch

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




--- Comment #6 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 387396
  --> https://bugs.webkit.org/attachment.cgi?id=387396
Patch

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

> Source/WebCore/page/DOMWindow.cpp:1644
> +    auto frame = makeRefPtr(this->frame());
> +    if (!frame)
>	   return;
>  
> +    auto view = makeRefPtr(frame->view());
> +    if (!view)
> +	   return;

We repeat this pattern in various accessors in this file. It's so common I
think we should share code for it.

I also think we should limit 'view' and 'frame' to a scope that ends before the
updateLayoutIgnorePendingStylesheets call. I see no reason to keep a ref to
these across the updateLayoutIgnorePendingStylesheets call.


More information about the webkit-reviews mailing list