Merge page serializers [4/12]

Support image documents, move frames test

Move WebPageNewSerializeTest.PageWithFrames,
WebPageNewSerializeTest.BlankFrames and
WebPageNewSerializeTest.SubFrameSerialization test to
PageSerializerTest.

Also adds support for ImageDocument in PageSerializer so frames that
points directly to an image is also supported.

--

This review is part of set aiming to merge the two existing page
serializers, WebPageSerializerImpl and PageSerializer into the latter
one.

In addition to this it moves all the old tests from
WebPageNewSerializerTest and WebPageSerializerTest to the newer
PageSerializerTest structure and splits out a test for MHTML into the
MHTMLTest file.

Note: This is a rebase and split of
https://codereview.chromium.org/68613003 from Opera which was reverted
due to failing tests. In addition to the rebase some smaller fixes was
done to accommodate to changes in the code from when it was first
landed.

BUG=328354
R=hajimehoshi@chromium.org

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=196861

[Tiger (Sony Mobile), 2015-06-09 09:38:50]

Please take a look. Thank you for reviewing.

[hajimehoshi, 2015-06-10 04:34:03]

lgtm

[Tiger (Sony Mobile), 2015-06-10 06:54:46]

gustav.tiger@sonymobile.com changed reviewers:
+ philipj@opera.com

[philipj_slow, 2015-06-10 09:09:30]

LGTM % nits

https://codereview.chromium.org/1167413004/diff/1/Source/core/page/PageSerial...
File Source/core/page/PageSerializer.cpp (right):

https://codereview.chromium.org/1167413004/diff/1/Source/core/page/PageSerial...
Source/core/page/PageSerializer.cpp:240: WTF::TextEncoding
textEncoding(document.charset());
Try document.encoding(), it's what backs document.charset() and is already the
correct type.

https://codereview.chromium.org/1167413004/diff/1/Source/core/page/PageSerial...
Source/core/page/PageSerializer.cpp:349: if (!image || !image->hasImage() ||
image->image() == Image::nullImage() || image->errorOccurred())
Would removing the |image->image() == Image::nullImage()| bit ever result in an
observable difference? I don't think so, because if |image->hasImage| or
|image->errorOccurred()|, then nullImage() will not be returned. Try removing?

[Tiger (Sony Mobile), 2015-06-10 09:38:08]

https://codereview.chromium.org/1167413004/diff/1/Source/core/page/PageSerial...
File Source/core/page/PageSerializer.cpp (right):

https://codereview.chromium.org/1167413004/diff/1/Source/core/page/PageSerial...
Source/core/page/PageSerializer.cpp:240: WTF::TextEncoding
textEncoding(document.charset());
On 2015/06/10 09:09:30, philipj wrote:
> Try document.encoding(), it's what backs document.charset() and is already the
> correct type.

Will fix

https://codereview.chromium.org/1167413004/diff/1/Source/core/page/PageSerial...
Source/core/page/PageSerializer.cpp:349: if (!image || !image->hasImage() ||
image->image() == Image::nullImage() || image->errorOccurred())
On 2015/06/10 09:09:30, philipj wrote:
> Would removing the |image->image() == Image::nullImage()| bit ever result in
an
> observable difference? I don't think so, because if |image->hasImage| or
> |image->errorOccurred()|, then nullImage() will not be returned. Try removing?

Yes, removing that part should be ok.

[philipj_slow, 2015-06-10 11:12:43]

The CQ bit was checked by philipj@opera.com

[philipj_slow, 2015-06-10 11:12:44]

lgtm

[philipj_slow, 2015-06-10 11:12:44]

The patchset sent to the CQ was uploaded after l-g-t-m from
hajimehoshi@chromium.org
Link to the patchset: https://codereview.chromium.org/1167413004/#ps20001
(title: "Fix nits")

[commit-bot: I haz the power, 2015-06-10 11:12:48]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1167413004/20001

[commit-bot: I haz the power, 2015-06-10 12:41:10]

Committed patchset #2 (id:20001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=196861