Bug 170828

Summary: Remove unused SharedBuffer constructor
Product: WebKit Reporter: Alex Christensen <achristensen>
Component: New BugsAssignee: Alex Christensen <achristensen>
Status: RESOLVED FIXED    
Severity: Normal CC: beidson, buildbot, darin, mmaxfield, ryanhaddad
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Alex Christensen
Reported 2017-04-13 14:41:24 PDT
Remove unused SharedBuffer constructor
Attachments
Patch (2.47 KB, patch)
2017-04-13 14:42 PDT, Alex Christensen 		no flags
DetailsFormatted DiffDiff
Patch (3.57 KB, patch)
2017-04-13 14:52 PDT, Alex Christensen 		no flags
DetailsFormatted DiffDiff
Patch (4.70 KB, patch)
2017-04-15 17:01 PDT, Alex Christensen 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Alex Christensen
Comment 1 2017-04-13 14:42:08 PDT
Created attachment 307027 [details] Patch
Alex Christensen
Comment 2 2017-04-13 14:52:03 PDT
Created attachment 307029 [details] Patch
Brady Eidson
Comment 3 2017-04-13 16:48:46 PDT
Comment on attachment 307029 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=307029&action=review > Source/WebCore/platform/SharedBuffer.cpp:230 > + // This copies data unnecessarily right now. We should just use the Vector instead of always copying it. I don't find any value-add from this comment.
Alex Christensen
Comment 4 2017-04-13 16:54:29 PDT
http://trac.webkit.org/r215350
Darin Adler
Comment 5 2017-04-13 18:22:14 PDT
Comment on attachment 307029 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=307029&action=review >> Source/WebCore/platform/SharedBuffer.cpp:230 >> + // This copies data unnecessarily right now. We should just use the Vector instead of always copying it. > > I don't find any value-add from this comment. I asked for a comment here after reviewing another bug. The comment I would write is slightly different. Something like this: // This takes its argument as a rvalue reference because we intend to have a future // version take ownership of the vector rather than copying.
Ryan Haddad
Comment 6 2017-04-14 09:53:07 PDT
(In reply to Alex Christensen from comment #4) > http://trac.webkit.org/r215350 I think this change broke the Windows build: https://build.webkit.org/builders/Apple%20Win%20Release%20%28Build%29/builds/674 C:\cygwin\home\buildbot\slave\win-release\build\Source\WebCore\platform\graphics\win\FontPlatformDataWin.cpp(81): error C2665: 'WebCore::SharedBuffer::create': none of the 4 overloads could convert all the argument types [C:\cygwin\home\buildbot\slave\win-release\build\WebKitBuild\Release\Source\WebCore\WebCore.vcxproj]
Ryan Haddad
Comment 7 2017-04-14 10:31:55 PDT
Reverted r215350 for reason: This change broke the Windows build. Committed r215364: <http://trac.webkit.org/changeset/215364>
Alex Christensen
Comment 8 2017-04-15 17:01:09 PDT
Created attachment 307207 [details] Patch
Alex Christensen
Comment 9 2017-04-15 17:10:16 PDT
http://trac.webkit.org/r215396
Alex Christensen
Comment 10 2017-04-15 23:31:09 PDT
https://trac.webkit.org/changeset/215398/webkit
Note You need to log in before you can comment on or make changes to this bug.