RESOLVED FIXED Bug 131784
[Win] A few final cleanups to the DataObject classes 
https://bugs.webkit.org/show_bug.cgi?id=131784
Summary [Win] A few final cleanups to the DataObject classes
Brent Fulgham
Reported 2014-04-16 19:54:45 PDT
While working on Bug 131631, we noticed a few improvements that could be made once the Vector class had been updated with additional move-handling operators. 1. Use "someVector.last()" rather than "someVector[someVector.size() - 1]" 2. Use Vectors "takeLast()" method, rather than the hand-coded implementation in these classes.
Attachments
Patch (3.59 KB, patch)
2014-04-16 20:26 PDT, Brent Fulgham 		no flags
DetailsFormatted DiffDiff
Whoops! Left some unrelated debugging code in my tree. (4.00 KB, patch)
2014-04-17 10:55 PDT, Brent Fulgham 		no flags
DetailsFormatted DiffDiff
Patch (3.60 KB, patch)
2014-04-17 10:58 PDT, Brent Fulgham 		andersca: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Brent Fulgham
Comment 1 2014-04-16 20:26:41 PDT
Created attachment 229517 [details] Patch
Darin Adler
Comment 2 2014-04-17 10:29:26 PDT
Comment on attachment 229517 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=229517&action=review > Source/WebCore/platform/win/WCDataObject.cpp:361 > + m_formats[ptr] = std::move(m_formats.last()); > m_formats.removeLast(); Should be takeLast, not last/removeLast. > Source/WebCore/platform/win/WCDataObject.cpp:363 > + m_medium[ptr] = std::move(m_medium.last()); > m_medium.removeLast(); Should be takeLast, not last/removeLast. > Tools/DumpRenderTree/win/DRTDataObject.cpp:350 > + m_formats[position] = std::move(m_formats.last()); > m_formats.removeLast(); Should be takeLast, not last/removeLast. > Tools/DumpRenderTree/win/DRTDataObject.cpp:352 > + m_medium[position] = std::move(m_medium.last()); > m_medium.removeLast(); Should be takeLast, not last/removeLast.
Brent Fulgham
Comment 3 2014-04-17 10:55:27 PDT
Created attachment 229559 [details] Whoops! Left some unrelated debugging code in my tree.
Brent Fulgham
Comment 4 2014-04-17 10:56:08 PDT
Anders, do I need to add a std::move() in these uses of takeLast to ensure proper move semantics?
Brent Fulgham
Comment 5 2014-04-17 10:58:00 PDT
Created attachment 229560 [details] Patch
Brent Fulgham
Comment 6 2014-04-17 11:15:08 PDT
Committed r167442: <http://trac.webkit.org/changeset/167442>
Note You need to log in before you can comment on or make changes to this bug.