Bug 185722

Summary: Safari optimized flow should be releasing viewer to prevent memory growth with subsequent launches/closes
Product: WebKit Reporter: Dean Jackson <dino>
Component: New BugsAssignee: Dean Jackson <dino>
Status: RESOLVED FIXED    
Severity: Normal CC: ddkilzer, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch ddkilzer: review+

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>