Bug 171809 - Reduce PassRefPtr use
Summary: Reduce PassRefPtr use
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: Alex Christensen
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-05-08 09:54 PDT by Alex Christensen
Modified: 2017-05-08 13:12 PDT (History)
3 users (show)

See Also:


Attachments
Patch (42.78 KB, patch)
2017-05-08 09:55 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (47.76 KB, patch)
2017-05-08 10:07 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (48.46 KB, patch)
2017-05-08 10:42 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (48.38 KB, patch)
2017-05-08 11:30 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (48.20 KB, patch)
2017-05-08 11:31 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (47.94 KB, patch)
2017-05-08 12:11 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Christensen 2017-05-08 09:54:41 PDT
Reduce PassRefPtr use
Comment 1 Alex Christensen 2017-05-08 09:55:16 PDT
Created attachment 309368 [details]
Patch
Comment 2 Build Bot 2017-05-08 09:57:00 PDT
Attachment 309368 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 29 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Chris Dumez 2017-05-08 10:06:29 PDT
Comment on attachment 309368 [details]
Patch

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

r=me with comments, please make sure all bots are happy.

> Source/WebCore/platform/graphics/ca/cocoa/PlatformCALayerCocoa.mm:355
> +        AVPlayerLayer *destinationPlayerLayer = static_cast<PlatformCALayerCocoa&>(newLayer.get()).avPlayerLayer();

Should be a downcast.

> Source/WebKit2/WebProcess/WebPage/VisitedLinkTableController.cpp:54
>      visitedLinkTableControllerPtr = visitedLinkTableController.ptr();

This line seems useless?

> Source/WebKit2/WebProcess/WebPage/WebOpenPanelResultListener.h:57
>      RefPtr<WebCore::FileChooser> m_fileChooser;

Should be a Ref<>.
Comment 4 Alex Christensen 2017-05-08 10:07:54 PDT
Created attachment 309371 [details]
Patch
Comment 5 Chris Dumez 2017-05-08 10:09:07 PDT
Comment on attachment 309371 [details]
Patch

Same comments as before.
Comment 6 Alex Christensen 2017-05-08 10:41:55 PDT
(In reply to Chris Dumez from comment #3)
> > Source/WebCore/platform/graphics/ca/cocoa/PlatformCALayerCocoa.mm:355
> > +        AVPlayerLayer *destinationPlayerLayer = static_cast<PlatformCALayerCocoa&>(newLayer.get()).avPlayerLayer();
> 
> Should be a downcast.
That doesn't compile.
> 
> > Source/WebKit2/WebProcess/WebPage/VisitedLinkTableController.cpp:54
> >      visitedLinkTableControllerPtr = visitedLinkTableController.ptr();
> 
> This line seems useless?
It's needed in this case to set the pointer in the HashMap.  Weird.  We can't use ensure here, though, because the HashMap values are raw pointers, and we need to return a Ref here.
Comment 7 Alex Christensen 2017-05-08 10:42:24 PDT
Created attachment 309378 [details]
Patch
Comment 8 Alex Christensen 2017-05-08 11:30:19 PDT
Created attachment 309384 [details]
Patch
Comment 9 Alex Christensen 2017-05-08 11:31:38 PDT
Created attachment 309386 [details]
Patch
Comment 10 Alex Christensen 2017-05-08 12:11:58 PDT
Created attachment 309392 [details]
Patch
Comment 11 Alex Christensen 2017-05-08 13:12:08 PDT
http://trac.webkit.org/r216448