Bug 199535

Summary: WebBackForwardListItem::setPageState should receive pageState by reference
Product: WebKit Reporter: Michael Catanzaro <mcatanzaro>
Component: WebKit2Assignee: Michael Catanzaro <mcatanzaro>
Status: RESOLVED FIXED    
Severity: Trivial CC: achristensen, clopez, commit-queue, mcatanzaro, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: PC   
OS: Linux   
Bug Depends on:    
Bug Blocks: 104114    
Attachments:
Description Flags
Patch
none
Patch none

Michael Catanzaro
Reported 2019-07-05 15:04:28 PDT
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.
Attachments
Patch (2.07 KB, patch)
2019-07-05 15:05 PDT, Michael Catanzaro 		no flags
DetailsFormatted DiffDiff
Patch (2.69 KB, patch)
2019-07-11 16:07 PDT, Michael Catanzaro 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Michael Catanzaro
Comment 1 2019-07-05 15:05:38 PDT
Created attachment 373542 [details] Patch
Carlos Alberto Lopez Perez
Comment 2 2019-07-11 05:58:13 PDT
Comment on attachment 373542 [details] Patch Looks good to me. I think you need to CC some reviewers or this will be unnoticed
youenn fablet
Comment 3 2019-07-11 09:48:29 PDT
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.
Michael Catanzaro
Comment 4 2019-07-11 16:07:45 PDT
Created attachment 373971 [details] Patch
Michael Catanzaro
Comment 5 2019-07-11 16:08:33 PDT
(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.
WebKit Commit Bot
Comment 6 2019-07-12 13:58:49 PDT
Comment on attachment 373971 [details] Patch Clearing flags on attachment: 373971 Committed r247396: <https://trac.webkit.org/changeset/247396>
WebKit Commit Bot
Comment 7 2019-07-12 13:58:50 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 8 2019-07-12 13:59:19 PDT
<rdar://problem/53025386>
Note You need to log in before you can comment on or make changes to this bug.