[chromium] PageSerializer::generateMHTML() should return a WebData

• Component: WebKit Misc.
• Status: RESOLVED WONTFIX

[Jay Civelli]

We need a WebSharedBuffer class that wraps the WebCore::SharedBuffer and PageSerializer::getSomeData() should return a WebSharedBuffer

[Jay Civelli]

Created attachment 95826 [details]
Patch

[Darin Fisher (:fishd, Google)]

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]

Created attachment 95933 [details]
Patch

[Jay Civelli]

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 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 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 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]

Created attachment 96637 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 96637 [details]
Patch for landing

Clearing flags on attachment: 96637

Committed r88486: <http://trac.webkit.org/changeset/88486>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Jay Civelli]

Broke the Windows build (a test would not compile), patch got reverted.

[Jay Civelli]

Created attachment 96670 [details]
Patch

[Jay Civelli]

Created attachment 96762 [details]
Patch for landing

[WebKit Review Bot]

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: [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Review Bot]

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

[Stephen Chenney]

https://code.google.com/p/chromium/issues/detail?id=230570