Stop using PassRefPtr in platform/graphics.
Created attachment 309753 [details] WIP Patch
Created attachment 309755 [details] WIP Patch
Created attachment 309757 [details] WIP Patch
Created attachment 309760 [details] WIP Patch
Created attachment 309762 [details] Patch
Created attachment 309765 [details] Patch
Created attachment 309771 [details] Patch
Created attachment 309777 [details] Patch
Created attachment 309791 [details] Patch
Comment on attachment 309777 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=309777&action=review > Source/WebCore/platform/graphics/filters/FilterOperation.h:182 > ASSERT_NOT_REACHED(); > - return nullptr; > + return *static_cast<FilterOperation*>(nullptr); This looks gross. It should return a RefPtr or RELEASE_ASSERT_NOT_REACHED(). We shouldn't have to check a valid Ref for a nullptr. > Source/WebCore/rendering/RenderLayer.cpp:3717 > if (renderer().document().deviceScaleFactor() >= 2) { > - static NeverDestroyed<Image*> resizeCornerImageHiRes(Image::loadPlatformResource("textAreaResizeCorner@2x").leakRef()); > + static NeverDestroyed<Image*> resizeCornerImageHiRes(&Image::loadPlatformResource("textAreaResizeCorner@2x").leakRef()); Don't we have 3x resources now? Maybe we should update this.
(In reply to Alex Christensen from comment #10) > Comment on attachment 309777 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=309777&action=review > > > Source/WebCore/platform/graphics/filters/FilterOperation.h:182 > > ASSERT_NOT_REACHED(); > > - return nullptr; > > + return *static_cast<FilterOperation*>(nullptr); > > This looks gross. It should return a RefPtr or > RELEASE_ASSERT_NOT_REACHED(). We shouldn't have to check a valid Ref for a > nullptr. Sure, I do not mind making it a release assertion. It is going to crash on the next line anyway. > > > Source/WebCore/rendering/RenderLayer.cpp:3717 > > if (renderer().document().deviceScaleFactor() >= 2) { > > - static NeverDestroyed<Image*> resizeCornerImageHiRes(Image::loadPlatformResource("textAreaResizeCorner@2x").leakRef()); > > + static NeverDestroyed<Image*> resizeCornerImageHiRes(&Image::loadPlatformResource("textAreaResizeCorner@2x").leakRef()); > > Don't we have 3x resources now? Maybe we should update this. Seems out of scope.
Created attachment 309793 [details] Patch
> > Don't we have 3x resources now? Maybe we should update this. > > Seems out of scope. Definitely out of scope.
Comment on attachment 309793 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=309793&action=review > Source/WebCore/platform/graphics/filters/FilterOperation.h:182 > + return *static_cast<FilterOperation*>(nullptr); I would prefer dereferencing this. Dereferencing nullptr feels really bad.
(In reply to Alex Christensen from comment #14) > Comment on attachment 309793 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=309793&action=review > > > Source/WebCore/platform/graphics/filters/FilterOperation.h:182 > > + return *static_cast<FilterOperation*>(nullptr); > > I would prefer dereferencing this. Dereferencing nullptr feels really bad. If I return |this|, then I would need to const_cast it. Note that dereferencing nullptr is the pattern used here already: Ref<Node> ShadowRoot::cloneNodeInternal(Document&, CloningOperation) { RELEASE_ASSERT_NOT_REACHED(); return *static_cast<Node*>(nullptr); // ShadowRoots should never be cloned. }
Comment on attachment 309793 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=309793&action=review >>> Source/WebCore/platform/graphics/filters/FilterOperation.h:182 >>> + return *static_cast<FilterOperation*>(nullptr); >> >> I would prefer dereferencing this. Dereferencing nullptr feels really bad. > > If I return |this|, then I would need to const_cast it. Note that dereferencing nullptr is the pattern used here already: > Ref<Node> ShadowRoot::cloneNodeInternal(Document&, CloningOperation) > { > RELEASE_ASSERT_NOT_REACHED(); > return *static_cast<Node*>(nullptr); // ShadowRoots should never be cloned. > } Yeah, I think returning casted nullptr is fine. clang would be eliminating this code anyway due to reachability.
Comment on attachment 309793 [details] Patch Clearing flags on attachment: 309793 Committed r216702: <http://trac.webkit.org/changeset/216702>
All reviewed patches have been landed. Closing bug.
Committed r216722: <http://trac.webkit.org/changeset/216722>