Fix preloadScanner double download issue with link preload and media

Fix preloadScanner double download issue with link preload and media

The preloadScanner was not checking the media attribute when preloading
<link rel=preload> based resources, due to a bug that bypassed that logic for preloads.

This CL fixes that and adds unit tests to make sure it stays fixed.

BUG=612443
Committed: https://crrev.com/dd9e1daef514feb5f0b1e0788f57b68b4adeec84
Cr-Commit-Position: refs/heads/master@{#394932}

[Yoav Weiss, 2016-05-17 13:53:13]

Description was changed from

==========
Fix preloadScanner double download issue with link preload and media

The preloadScanner was not checking the media attribute when preloading
<link rel=preload> based resources, due to a bug that bypassed that logic for
preloads.

This CL fixes that and adds unit tests to make sure it stays fixed.

BUG=
==========

to

==========
Fix preloadScanner double download issue with link preload and media

The preloadScanner was not checking the media attribute when preloading
<link rel=preload> based resources, due to a bug that bypassed that logic for
preloads.

This CL fixes that and adds unit tests to make sure it stays fixed.

BUG=612443
==========

[Yoav Weiss, 2016-05-17 13:53:13]

yoav@yoav.ws changed reviewers:
+ japhet@chromium.org

[Yoav Weiss, 2016-05-17 13:55:53]

Hey Nate!

Turns out this pesky `else` is preventing the "if media attribute doesn't match,
skip the preload" logic inside the preloadScanner. 

This makes sure that it doesn't do that, and adds tests to make sure the fix
sticks around.

Can you take a look?

[Nate Chapin, 2016-05-17 17:16:55]

https://codereview.chromium.org/1982263002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/html/parser/HTMLPreloadScanner.cpp (right):

https://codereview.chromium.org/1982263002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/parser/HTMLPreloadScanner.cpp:214: if
(isLinkRelPreload()) {
Which of these cases really should be mutually exclusive? It seems suspicious
that this patch makes preload and preconnect not mutually exclusive.

[Yoav Weiss, 2016-05-17 21:45:49]

https://codereview.chromium.org/1982263002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/html/parser/HTMLPreloadScanner.cpp (right):

https://codereview.chromium.org/1982263002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/parser/HTMLPreloadScanner.cpp:214: if
(isLinkRelPreload()) {
On 2016/05/17 17:16:55, Nate Chapin wrote:
> Which of these cases really should be mutually exclusive? It seems suspicious
> that this patch makes preload and preconnect not mutually exclusive.

Good point. I changed the conditions to represent the fact that preconnect is
mutual exclusive with link preload as well as regular preload checks.

[Nate Chapin, 2016-05-19 19:58:41]

lgtm

[Yoav Weiss, 2016-05-19 19:59:58]

The CQ bit was checked by yoav@yoav.ws

[commit-bot: I haz the power, 2016-05-19 20:00:34]

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

[commit-bot: I haz the power, 2016-05-20 00:41:07]

Description was changed from

==========
Fix preloadScanner double download issue with link preload and media

The preloadScanner was not checking the media attribute when preloading
<link rel=preload> based resources, due to a bug that bypassed that logic for
preloads.

This CL fixes that and adds unit tests to make sure it stays fixed.

BUG=612443
==========

to

==========
Fix preloadScanner double download issue with link preload and media

The preloadScanner was not checking the media attribute when preloading
<link rel=preload> based resources, due to a bug that bypassed that logic for
preloads.

This CL fixes that and adds unit tests to make sure it stays fixed.

BUG=612443
==========

[commit-bot: I haz the power, 2016-05-20 00:41:08]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-05-20 00:42:50]

Description was changed from

==========
Fix preloadScanner double download issue with link preload and media

The preloadScanner was not checking the media attribute when preloading
<link rel=preload> based resources, due to a bug that bypassed that logic for
preloads.

This CL fixes that and adds unit tests to make sure it stays fixed.

BUG=612443
==========

to

==========
Fix preloadScanner double download issue with link preload and media

The preloadScanner was not checking the media attribute when preloading
<link rel=preload> based resources, due to a bug that bypassed that logic for
preloads.

This CL fixes that and adds unit tests to make sure it stays fixed.

BUG=612443

Committed: https://crrev.com/dd9e1daef514feb5f0b1e0788f57b68b4adeec84
Cr-Commit-Position: refs/heads/master@{#394932}
==========

[commit-bot: I haz the power, 2016-05-20 00:42:51]

Patchset 2 (id:??) landed as
https://crrev.com/dd9e1daef514feb5f0b1e0788f57b68b4adeec84
Cr-Commit-Position: refs/heads/master@{#394932}