[Webkit-unassigned] [Bug 159212] [Coordinated Graphics] Modernize and cleanup CompositingCoordinator

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Jul 4 01:29:17 PDT 2016


https://bugs.webkit.org/show_bug.cgi?id=159212

--- Comment #6 from Carlos Garcia Campos <cgarcia at igalia.com> ---
(In reply to comment #5)
> Comment on attachment 282417 [details]
> Rebased patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=282417&action=review
> 
> >>> Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/CompositingCoordinator.cpp:217
> >>> +void CompositingCoordinator::updateImageBacking(CoordinatedImageBackingID imageID, RefPtr<CoordinatedSurface>& coordinatedSurface)
> >> 
> >> I don't think this should be receiving a reference to RefPtr.
> > 
> > Could you elaborate? what's wrong with the reference and what should I do instead? just copying the RefPtr? I don't understand whey PassRefPtr is used there TBH.
> 
> Passing a reference to a non-const RefPtr object here, you'd expect the
> method to modify it. That's not what's happening here, and if it were, you'd
> expect the method to return a new RefPtr.

Ah, I see, I didn't notice it was non-const.

> A rvalue reference to the RefPtr would make sense, so you'd copyRef() it
> into the method call, and move it in this method to the std::make_pair()
> call.
> 
> >>> Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/CompositingCoordinator.cpp:286
> >>> +void CompositingCoordinator::createUpdateAtlas(uint32_t atlasID, RefPtr<CoordinatedSurface>& coordinatedSurface)
> >> 
> >> I don't think this should be receiving a reference to RefPtr.
> > 
> > Isn't that automatic? doesn't auto deduce it's a pointer?
> 
> It does, but it doesn't tell me that when I look at it. auto* provides more
> info, shows that it's not a RefPtr<> or an iterator or anything.

Ah, it's just for readability, I agree indeed. I'll submit an updated patch, thanks!

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20160704/ccfddaab/attachment.html>


More information about the webkit-unassigned mailing list