Bug 185722 - Safari optimized flow should be releasing viewer to prevent memory growth with subsequent launches/closes
Summary: Safari optimized flow should be releasing viewer to prevent memory growth wit...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dean Jackson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-05-17 04:01 PDT by Dean Jackson
Modified: 2018-05-17 04:30 PDT (History)
2 users (show)

See Also:


Attachments
Patch (2.56 KB, patch)
2018-05-17 04:04 PDT, Dean Jackson
ddkilzer: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dean Jackson 2018-05-17 04:01:31 PDT
Safari optimized flow should be releasing viewer to prevent memory growth with subsequent launches/closes
Comment 1 Dean Jackson 2018-05-17 04:01:52 PDT
<rdar://problem/40247351>
Comment 2 Dean Jackson 2018-05-17 04:04:13 PDT
Created attachment 340570 [details]
Patch
Comment 3 David Kilzer (:ddkilzer) 2018-05-17 04:21:13 PDT
Comment on attachment 340570 [details]
Patch

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

r=me

> Source/WebKit/UIProcess/Cocoa/SystemPreviewControllerCocoa.mm:90
> +    WebKit::WeakObjCPtr<_WKPreviewControllerDataSource> weakSelf { self };

FWIW, as of Bug 184789 / r230824, you can start using __weak directly.  Not sure how well that plays with C++ lambdas, though.

> Source/WebKit/UIProcess/Cocoa/SystemPreviewControllerCocoa.mm:93
> +        auto strongSelf = weakSelf.get();
> +        if (strongSelf)

Nit: Could use:

        if ((auto strongSelf = weakSelf.get()))
Comment 4 Dean Jackson 2018-05-17 04:26:11 PDT
(In reply to David Kilzer (:ddkilzer) from comment #3)
> Comment on attachment 340570 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=340570&action=review
> 
> r=me
> 
> > Source/WebKit/UIProcess/Cocoa/SystemPreviewControllerCocoa.mm:90
> > +    WebKit::WeakObjCPtr<_WKPreviewControllerDataSource> weakSelf { self };
> 
> FWIW, as of Bug 184789 / r230824, you can start using __weak directly.  Not
> sure how well that plays with C++ lambdas, though.

This can just be a regular block, so __weak would be fine. However, I didn't see any other uses of __weak, so I just copied what is already used.

> 
> > Source/WebKit/UIProcess/Cocoa/SystemPreviewControllerCocoa.mm:93
> > +        auto strongSelf = weakSelf.get();
> > +        if (strongSelf)
> 
> Nit: Could use:
> 
>         if ((auto strongSelf = weakSelf.get()))

Thanks!
Comment 5 Dean Jackson 2018-05-17 04:30:30 PDT
Committed r231893: <https://trac.webkit.org/changeset/231893>