RESOLVED FIXED 134417
Add test to ensure that SVGDocumentExtensions::rebuildElements() doesn't rebuild invalid elements 
https://bugs.webkit.org/show_bug.cgi?id=134417
Summary Add test to ensure that SVGDocumentExtensions::rebuildElements() doesn't rebu...
Daniel Bates
Reported 2014-06-27 15:23:32 PDT
We should use Vector::swap() instead of std::move() to explicitly swap Vector objects in SVGDocumentExtensions::rebuildElements(). Currently we use std::move() to implicitly swap Vector objects. This is error prone as a const lvalue reference can bind to an rvalue so calling T t = std::move(t_1) <=> T t = t_1 <=> T t(t_1) where t_1 is const T&&.
Attachments
Patch and layout test (6.44 KB, patch)
2014-06-27 15:27 PDT, Daniel Bates 		no flags
DetailsFormatted DiffDiff
Layout test (3.49 KB, patch)
2014-07-03 16:26 PDT, Daniel Bates 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Daniel Bates
Comment 1 2014-06-27 15:24:00 PDT
<rdar://problem/17479854>
Daniel Bates
Comment 2 2014-06-27 15:27:30 PDT
Created attachment 234024 [details] Patch and layout test
Darin Adler
Comment 3 2014-06-27 16:50:17 PDT
Comment on attachment 234024 [details] Patch and layout test I don’t think this is an improvement.
Darin Adler
Comment 4 2014-06-27 16:51:17 PDT
(In reply to comment #0) > Currently we use std::move() to implicitly swap Vector objects. No, we use it to move a vector. There is no swapping going on; we move the vector from the object into a local variable.
Daniel Bates
Comment 5 2014-06-30 17:14:48 PDT
Comment on attachment 234024 [details] Patch and layout test Clearing review flag. After talking with Darin Adler and Anders Carlsson today (06/30), a better approach is to come up with a WTF::move() function that asserts we aren't called with a const value.
Daniel Bates
Comment 6 2014-07-01 16:27:44 PDT
(In reply to comment #5) > (From update of attachment 234024 [details]) > Clearing review flag. After talking with Darin Adler and Anders Carlsson today (06/30), a better approach is to come up with a WTF::move() function that asserts we aren't called with a const value. Filed bug #134500 to add WTF::move(). I'll repurpose this bug for adding the layout test, included in attachment #234024 [details].
Daniel Bates
Comment 7 2014-07-03 16:26:54 PDT
Created attachment 234378 [details] Layout test
David Kilzer (:ddkilzer)
Comment 8 2014-07-03 18:26:30 PDT
Comment on attachment 234378 [details] Layout test r=me
Daniel Bates
Comment 9 2014-07-07 10:52:54 PDT
Comment on attachment 234378 [details] Layout test Clearing flags on attachment: 234378 Committed r170847: <http://trac.webkit.org/changeset/170847>
Daniel Bates
Comment 10 2014-07-07 10:53:01 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.