RESOLVED FIXED 185722
Safari optimized flow should be releasing viewer to prevent memory growth with subsequent launches/closes
https://bugs.webkit.org/show_bug.cgi?id=185722
Summary Safari optimized flow should be releasing viewer to prevent memory growth wit...
Dean Jackson
Reported 2018-05-17 04:01:31 PDT
Safari optimized flow should be releasing viewer to prevent memory growth with subsequent launches/closes
Attachments
Patch (2.56 KB, patch)
2018-05-17 04:04 PDT, Dean Jackson
ddkilzer: review+
Dean Jackson
Comment 1 2018-05-17 04:01:52 PDT
Dean Jackson
Comment 2 2018-05-17 04:04:13 PDT
David Kilzer (:ddkilzer)
Comment 3 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()))
Dean Jackson
Comment 4 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!
Dean Jackson
Comment 5 2018-05-17 04:30:30 PDT
Note You need to log in before you can comment on or make changes to this bug.