Coverity is complaining here about copying PageState by value in the parameter list. It's sort of a false positive, in that the PageState really does need to be copied here, so this is the best we can do. But pass by value and then WTFMove() is a pretty strange way to write it. Just passing by reference would be better. Then it will be copied into m_itemState.pageState.
Created attachment 373542 [details] Patch
Comment on attachment 373542 [details] Patch Looks good to me. I think you need to CC some reviewers or this will be unnoticed
Comment on attachment 373542 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=373542&action=review > Source/WebKit/ChangeLog:12 > + m_itemState.pageState.. I would tend to go with setPageState(PageState&&), except if it requires too much explicit PageState { } at call sites. Doing setPageState(PageState) is somehow better than setPageState(const&) since it probably accepts both const& and && as parameters.
Created attachment 373971 [details] Patch
(In reply to youenn fablet from comment #3) > > Source/WebKit/ChangeLog:12 > > + m_itemState.pageState.. > > I would tend to go with setPageState(PageState&&), except if it requires too > much explicit PageState { } at call sites. Sure. There's only one callsite.
Comment on attachment 373971 [details] Patch Clearing flags on attachment: 373971 Committed r247396: <https://trac.webkit.org/changeset/247396>
All reviewed patches have been landed. Closing bug.
<rdar://problem/53025386>