Bug 131784

Summary: [Win] A few final cleanups to the DataObject classes
Product: WebKit Reporter: Brent Fulgham <bfulgham>
Component: WebKit Misc.Assignee: Brent Fulgham <bfulgham>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, bfulgham, darin
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Bug Depends on: 131631, 131735    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Whoops! Left some unrelated debugging code in my tree.
none
Patch andersca: review+

Description Brent Fulgham 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.
Comment 1 Brent Fulgham 2014-04-16 20:26:41 PDT
Created attachment 229517 [details]
Patch
Comment 2 Darin Adler 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.
Comment 3 Brent Fulgham 2014-04-17 10:55:27 PDT
Created attachment 229559 [details]
Whoops! Left some unrelated debugging code in my tree.
Comment 4 Brent Fulgham 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?
Comment 5 Brent Fulgham 2014-04-17 10:58:00 PDT
Created attachment 229560 [details]
Patch
Comment 6 Brent Fulgham 2014-04-17 11:15:08 PDT
Committed r167442: <http://trac.webkit.org/changeset/167442>