Add a few more edge cases to CSS serialization test.

Add a few more edge cases to CSS serialization test.

This change slightly modifies the input HTML file for the
FrameSerializerTest.CSS test to exercise a couple more cases that should
be dealt by FrameSerializer:
- Linked CSS style using a data URL.
- External CSS resource being references twice.
- Simulate the attemp to include a CSS already included in a previously
  serialized frame.

Note that these inclusions almost do not affect the test expectations as
they are edge cases that should not change the final serialized results.

This is a follow up of https://crrev.com/2397983002.

BUG=645685
Committed: https://crrev.com/195ccbed3bad6e043253cab1c150652c72be5e96
Cr-Commit-Position: refs/heads/master@{#425239}

[carlosk, 2016-10-11 01:36:50]

The CQ bit was checked by carlosk@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-11 01:37:05]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-11 02:29:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-11 02:29:22]

Dry run: This issue passed the CQ dry run.

[carlosk, 2016-10-11 22:56:36]

The CQ bit was checked by carlosk@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-11 22:57:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[carlosk, 2016-10-11 23:08:04]

Description was changed from

==========
Add a few more edge cases to CSS serialization test.

This change slightly modifies the input HTML file for the
FrameSerializerTest.CSS test to exercise a couple more cases that should
be dealt by FrameSerializer:
- Linked CSS style using a data URL.
- External CSS resource being references twice.

Note that this change does not affect any of the test expectations as
they are edge cases that should not affect the final serialized results.

This is a follow up of https://crrev.com/2397983002.

BUG=645685
==========

to

==========
Add a few more edge cases to CSS serialization test.

This change slightly modifies the input HTML file for the
FrameSerializerTest.CSS test to exercise a couple more cases that should
be dealt by FrameSerializer:
- Linked CSS style using a data URL.
- External CSS resource being references twice.
- Simulate the attemp to include a CSS already included in a previously
  serialized frame.

Note that these inclusions almost do not affect the test expectations as
they are edge cases that should not change the final serialized results.

This is a follow up of https://crrev.com/2397983002.

BUG=645685
==========

[carlosk, 2016-10-11 23:08:53]

carlosk@chromium.org changed reviewers:
+ dcheng@chromium.org, lukasza@chromium.org

[carlosk, 2016-10-11 23:08:54]

dcheng@, lukasza@: PTAL.

This adds a unit test version of the missing tests we've been discussing.

https://codereview.chromium.org/2410693002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/frame/FrameSerializer.cpp (left):

https://codereview.chromium.org/2410693002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/FrameSerializer.cpp:415: break;
This is not needed anymore as it's checked early in serializeCSSStyleSheet.

https://codereview.chromium.org/2410693002/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/web/tests/data/frameserializer/css/css_test_page.html
(right):

https://codereview.chromium.org/2410693002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/data/frameserializer/css/css_test_page.html:12:
href="data:text/css;charset=utf-8;base64,I2RpdkZ1Y2hzaWEgew0KICBiYWNrZ3JvdW5kLWltYWdlOiB1cmwoJ2RvX25vdF9zZXJpYWxpemUucG5nJyk7DQogIGJhY2tncm91bmQtY29sb3I6ICNGRjAwRkY7DQp9"
/>
I'm unsure of the correct HTML style, especially in this line.

[Łukasz Anforowicz, 2016-10-11 23:30:37]

Thank you for adding the tests.  LGTM (with 2 optional improvement ideas).

https://codereview.chromium.org/2410693002/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/web/tests/data/frameserializer/css/css_test_page.html
(right):

https://codereview.chromium.org/2410693002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/data/frameserializer/css/css_test_page.html:12:
href="data:text/css;charset=utf-8;base64,I2RpdkZ1Y2hzaWEgew0KICBiYWNrZ3JvdW5kLWltYWdlOiB1cmwoJ2RvX25vdF9zZXJpYWxpemUucG5nJyk7DQogIGJhY2tncm91bmQtY29sb3I6ICNGRjAwRkY7DQp9"
/>
On 2016/10/11 23:08:54, carlosk wrote:
> I'm unsure of the correct HTML style, especially in this line.

Hmmm... non-base64-encoded form of the data: URI is only marginally more
readable:

data:text/css;charset=utf-8,%23divFuchsia%20%7B%0D%0A%20%20background-image%3A%20url%28%27do_not_serialize.png%27%29%3B%0D%0A%20%20background-color%3A%20%23FF00FF%3B%0D%0A%7D

So, I guess I am fine with just having base64-encoded data URI above.

One way to help the reader, might be to add a decoded/expanded form in a
comment.  I don't feel particularily strongly about this though...


I wonder if it might be worth it to add a subresource with a unique link to the
CSS in the data: URI (similar to how we test presence of pink_background.png
coming from the inline <style> tag).

[carlosk, 2016-10-12 00:41:01]

The CQ bit was checked by carlosk@chromium.org to run a CQ dry run

[carlosk, 2016-10-12 00:41:30]

Thanks.

https://codereview.chromium.org/2410693002/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/web/tests/data/frameserializer/css/css_test_page.html
(right):

https://codereview.chromium.org/2410693002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/data/frameserializer/css/css_test_page.html:12:
href="data:text/css;charset=utf-8;base64,I2RpdkZ1Y2hzaWEgew0KICBiYWNrZ3JvdW5kLWltYWdlOiB1cmwoJ2RvX25vdF9zZXJpYWxpemUucG5nJyk7DQogIGJhY2tncm91bmQtY29sb3I6ICNGRjAwRkY7DQp9"
/>
On 2016/10/11 23:30:37, Łukasz Anforowicz wrote:
> On 2016/10/11 23:08:54, carlosk wrote:
> > I'm unsure of the correct HTML style, especially in this line.
> 
> Hmmm... non-base64-encoded form of the data: URI is only marginally more
> readable:
> 
>
data:text/css;charset=utf-8,%23divFuchsia%20%7B%0D%0A%20%20background-image%3A%20url%28%27do_not_serialize.png%27%29%3B%0D%0A%20%20background-color%3A%20%23FF00FF%3B%0D%0A%7D
> 
> So, I guess I am fine with just having base64-encoded data URI above.

Replaced with an escaped sequence instead of base64.

> One way to help the reader, might be to add a decoded/expanded form in a
> comment.  I don't feel particularily strongly about this though...

Done that too.

> I wonder if it might be worth it to add a subresource with a unique link to
the
> CSS in the data: URI (similar to how we test presence of pink_background.png
> coming from the inline <style> tag).

I tried this with a relative URL and it didn't work. But it did indeed work
using an absolute URL to an image so I added this extra element too.

[commit-bot: I haz the power, 2016-10-12 00:41:38]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-12 02:22:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-12 02:22:16]

Dry run: This issue passed the CQ dry run.

[dcheng, 2016-10-13 17:07:34]

lgtm

[carlosk, 2016-10-14 00:11:04]

The CQ bit was checked by carlosk@chromium.org

[carlosk, 2016-10-14 00:11:04]

The patchset sent to the CQ was uploaded after l-g-t-m from lukasza@chromium.org
Link to the patchset: https://codereview.chromium.org/2410693002/#ps60001
(title: "Adds image link to data URL CSS contents.")

[commit-bot: I haz the power, 2016-10-14 00:11:30]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-14 01:15:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-14 01:15:44]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[carlosk, 2016-10-14 01:17:33]

The CQ bit was checked by carlosk@chromium.org

[commit-bot: I haz the power, 2016-10-14 01:17:50]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-14 02:19:28]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-10-14 02:22:56]

Description was changed from

==========
Add a few more edge cases to CSS serialization test.

This change slightly modifies the input HTML file for the
FrameSerializerTest.CSS test to exercise a couple more cases that should
be dealt by FrameSerializer:
- Linked CSS style using a data URL.
- External CSS resource being references twice.
- Simulate the attemp to include a CSS already included in a previously
  serialized frame.

Note that these inclusions almost do not affect the test expectations as
they are edge cases that should not change the final serialized results.

This is a follow up of https://crrev.com/2397983002.

BUG=645685
==========

to

==========
Add a few more edge cases to CSS serialization test.

This change slightly modifies the input HTML file for the
FrameSerializerTest.CSS test to exercise a couple more cases that should
be dealt by FrameSerializer:
- Linked CSS style using a data URL.
- External CSS resource being references twice.
- Simulate the attemp to include a CSS already included in a previously
  serialized frame.

Note that these inclusions almost do not affect the test expectations as
they are edge cases that should not change the final serialized results.

This is a follow up of https://crrev.com/2397983002.

BUG=645685

Committed: https://crrev.com/195ccbed3bad6e043253cab1c150652c72be5e96
Cr-Commit-Position: refs/heads/master@{#425239}
==========

[commit-bot: I haz the power, 2016-10-14 02:22:57]

Patchset 4 (id:??) landed as
https://crrev.com/195ccbed3bad6e043253cab1c150652c72be5e96
Cr-Commit-Position: refs/heads/master@{#425239}