Bug 61908

Summary: [chromium] PageSerializer::generateMHTML() should return a WebData
Product: WebKit Reporter: Jay Civelli <jcivelli>
Component: WebKit Misc.Assignee: Jay Civelli <jcivelli>
Status: RESOLVED WONTFIX    
Severity: Normal CC: fishd, schenney, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 62409    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing
none
Patch
none
Patch for landing webkit.review.bot: commit-queue-

Jay Civelli
Reported 2011-06-01 23:30:20 PDT
We need a WebSharedBuffer class that wraps the WebCore::SharedBuffer and PageSerializer::getSomeData() should return a WebSharedBuffer
Attachments
Patch (8.86 KB, patch)
2011-06-02 15:54 PDT, Jay Civelli 		no flags
DetailsFormatted DiffDiff
Patch (4.97 KB, patch)
2011-06-03 11:23 PDT, Jay Civelli 		no flags
DetailsFormatted DiffDiff
Patch for landing (5.06 KB, patch)
2011-06-09 13:55 PDT, Jay Civelli 		no flags
DetailsFormatted DiffDiff
Patch (5.97 KB, patch)
2011-06-09 17:25 PDT, Jay Civelli 		no flags
DetailsFormatted DiffDiff
Patch for landing (5.96 KB, patch)
2011-06-10 11:08 PDT, Jay Civelli 		webkit.review.bot: commit-queue-
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Jay Civelli
Comment 1 2011-06-02 15:54:10 PDT
Created attachment 95826 [details] Patch
Darin Fisher (:fishd, Google)
Comment 2 2011-06-03 10:03:25 PDT
Comment on attachment 95826 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=95826&action=review > Source/WebKit/chromium/public/WebSharedBuffer.h:69 > + WebPrivatePtr<WebCore::SharedBuffer> m_private; This class looks like WebData. Can we just use that? :-)
Jay Civelli
Comment 3 2011-06-03 11:23:33 PDT
Created attachment 95933 [details] Patch
Jay Civelli
Comment 4 2011-06-03 11:24:21 PDT
Comment on attachment 95826 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=95826&action=review >> Source/WebKit/chromium/public/WebSharedBuffer.h:69 >> + WebPrivatePtr<WebCore::SharedBuffer> m_private; > > This class looks like WebData. Can we just use that? :-) Doh! I missed that class somehow. Using it now.
Darin Fisher (:fishd, Google)
Comment 5 2011-06-03 11:33:28 PDT
Comment on attachment 95933 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=95933&action=review > Source/WebKit/chromium/public/WebData.h:90 > + WEBKIT_API size_t getSomeData(const char*& data, size_t position) const; why do you need this method when you already have direct access to the entire buffer via the "data" method?
Jay Civelli
Comment 6 2011-06-03 12:46:58 PDT
Comment on attachment 95933 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=95933&action=review >> Source/WebKit/chromium/public/WebData.h:90 >> + WEBKIT_API size_t getSomeData(const char*& data, size_t position) const; > > why do you need this method when you already have direct access to the entire buffer > via the "data" method? From SharedBuffer.h about data(): "Calling this function will force internal segmented buffers to be merged into a flat buffer. Use getSomeData() whenever possible for better performance."
Darin Fisher (:fishd, Google)
Comment 7 2011-06-08 16:34:25 PDT
Comment on attachment 95933 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=95933&action=review >>> Source/WebKit/chromium/public/WebData.h:90 >>> + WEBKIT_API size_t getSomeData(const char*& data, size_t position) const; >> >> why do you need this method when you already have direct access to the entire buffer >> via the "data" method? > > From SharedBuffer.h about data(): > "Calling this function will force internal segmented buffers to be merged into a flat buffer. Use getSomeData() whenever possible for better performance." OK, I see. I think it would help then to extend the comment above "data()" to explain why getSomeData() should be preferred (i.e., that it could require flattening the internal buffer).
Jay Civelli
Comment 8 2011-06-09 13:55:31 PDT
Created attachment 96637 [details] Patch for landing
WebKit Review Bot
Comment 9 2011-06-09 14:45:30 PDT
Comment on attachment 96637 [details] Patch for landing Clearing flags on attachment: 96637 Committed r88486: <http://trac.webkit.org/changeset/88486>
WebKit Review Bot
Comment 10 2011-06-09 14:45:35 PDT
All reviewed patches have been landed. Closing bug.
Jay Civelli
Comment 11 2011-06-09 17:17:22 PDT
Broke the Windows build (a test would not compile), patch got reverted.
Jay Civelli
Comment 12 2011-06-09 17:25:25 PDT
Created attachment 96670 [details] Patch
Jay Civelli
Comment 13 2011-06-10 11:08:26 PDT
Created attachment 96762 [details] Patch for landing
WebKit Review Bot
Comment 14 2011-06-10 11:49:17 PDT
The commit-queue encountered the following flaky tests while processing attachment 96762 [details]: http/tests/local/stylesheet-and-script-load-order.html bug 62450 (author: koivisto@iki.fi) The commit-queue is continuing to process your patch.
WebKit Review Bot
Comment 15 2011-06-10 11:50:41 PDT
Comment on attachment 96762 [details] Patch for landing Rejecting attachment 96762 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-02', '--port..." exit_code: 1 Last 500 characters of output: viewed" or "Rubber stamp" (case insensitive). 'svn update /mnt/git/webkit-commit-queue/Source/WebKit/chromium/third_party/skia/include --revision 1561 --ignore-externals' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' U /mnt/git/webkit-commit-queue/Source/WebKit/chromium/third_party/skia/include/core/SkScalar.h Updated to revision 1561. ________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' Updating webkit projects from gyp files... Full output: http://queues.webkit.org/results/8826464
Note You need to log in before you can comment on or make changes to this bug.