Bug 61908 - [chromium] PageSerializer::generateMHTML() should return a WebData
Summary: [chromium] PageSerializer::generateMHTML() should return a WebData
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jay Civelli
URL:
Keywords:
Depends on: 62409
Blocks:
  Show dependency treegraph
 
Reported: 2011-06-01 23:30 PDT by Jay Civelli
Modified: 2013-04-11 13:59 PDT (History)
3 users (show)

See Also:


Attachments
Patch (8.86 KB, patch)
2011-06-02 15:54 PDT, Jay Civelli
no flags Details | Formatted Diff | Diff
Patch (4.97 KB, patch)
2011-06-03 11:23 PDT, Jay Civelli
no flags Details | Formatted Diff | Diff
Patch for landing (5.06 KB, patch)
2011-06-09 13:55 PDT, Jay Civelli
no flags Details | Formatted Diff | Diff
Patch (5.97 KB, patch)
2011-06-09 17:25 PDT, Jay Civelli
no flags Details | Formatted Diff | Diff
Patch for landing (5.96 KB, patch)
2011-06-10 11:08 PDT, Jay Civelli
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jay Civelli 2011-06-01 23:30:20 PDT
We need a WebSharedBuffer class that wraps the WebCore::SharedBuffer and PageSerializer::getSomeData() should return a WebSharedBuffer
Comment 1 Jay Civelli 2011-06-02 15:54:10 PDT
Created attachment 95826 [details]
Patch
Comment 2 Darin Fisher (:fishd, Google) 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? :-)
Comment 3 Jay Civelli 2011-06-03 11:23:33 PDT
Created attachment 95933 [details]
Patch
Comment 4 Jay Civelli 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.
Comment 5 Darin Fisher (:fishd, Google) 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?
Comment 6 Jay Civelli 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."
Comment 7 Darin Fisher (:fishd, Google) 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).
Comment 8 Jay Civelli 2011-06-09 13:55:31 PDT
Created attachment 96637 [details]
Patch for landing
Comment 9 WebKit Review Bot 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>
Comment 10 WebKit Review Bot 2011-06-09 14:45:35 PDT
All reviewed patches have been landed.  Closing bug.
Comment 11 Jay Civelli 2011-06-09 17:17:22 PDT
Broke the Windows build (a test would not compile), patch got reverted.
Comment 12 Jay Civelli 2011-06-09 17:25:25 PDT
Created attachment 96670 [details]
Patch
Comment 13 Jay Civelli 2011-06-10 11:08:26 PDT
Created attachment 96762 [details]
Patch for landing
Comment 14 WebKit Review Bot 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.
Comment 15 WebKit Review Bot 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