Issue 1623403002: Remove fetching stylesheets with unsupported type

Issue 1623403002: Remove fetching stylesheets with unsupported type (Closed)

Created:
4 years, 11 months ago by suzyh_UTC10 (ex-contributor)

Modified:
4 years, 11 months ago

Reviewers:
Yoav Weiss

CC:
blink-reviews, blink-reviews-html_chromium.org, chromium-reviews, dglazkov+blink, gavinp+prerender_chromium.org, kinuko+watch, Rick Byers, Yoav Weiss

Base URL:
https://chromium.googlesource.com/chromium/src.git@master

Target Ref:
refs/pending/heads/master

Project:
chromium

Visibility:
Public.

More Reviews

Description

Remove fetching stylesheets with unsupported type Having deprecated stylesheet links with unsupported MIME types in https://codereview.chromium.org/1501393003, this patch removes the capability, updates the affected tests, and removes the UseCounter and deprecation warning. This patch also updates the preload scanner to ensure that the resources are not preloaded, and adds a test to check for this. BUG=286682 Committed: https://crrev.com/9b6d756c09ace8f77dd2c359d891ff34c7d82a04 Cr-Commit-Position: refs/heads/master@{#371708}

Patch Set 1 #

Total comments: 8

Patch Set 2 : Response to review #

Created: 4 years, 11 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+69 lines, -26 lines)			Patch
A	third_party/WebKit/LayoutTests/http/tests/loading/download_only_supported_stylesheet_types.html	View	1	1 chunk	+27 lines, -0 lines	0 comments	Download
A	third_party/WebKit/LayoutTests/http/tests/loading/download_only_supported_stylesheet_types-expected.txt	View	1	1 chunk	+14 lines, -0 lines	0 comments	Download
M	third_party/WebKit/LayoutTests/loader/link-load-only-supported-stylesheet-types.html	View		1 chunk	+1 line, -5 lines	0 comments	Download
M	third_party/WebKit/LayoutTests/loader/link-load-only-supported-stylesheet-types-datauri.html	View		1 chunk	+1 line, -5 lines	0 comments	Download
A	third_party/WebKit/LayoutTests/loader/link-load-only-supported-stylesheet-types-datauri-expected.html	View		1 chunk	+8 lines, -0 lines	0 comments	Download
D	third_party/WebKit/LayoutTests/loader/link-load-only-supported-stylesheet-types-datauri-expected.txt	View		1 chunk	+0 lines, -4 lines	0 comments	Download
A	third_party/WebKit/LayoutTests/loader/link-load-only-supported-stylesheet-types-expected.html	View		1 chunk	+8 lines, -0 lines	0 comments	Download
D	third_party/WebKit/LayoutTests/loader/link-load-only-supported-stylesheet-types-expected.txt	View		1 chunk	+0 lines, -4 lines	0 comments	Download
M	third_party/WebKit/Source/core/frame/UseCounter.h	View		1 chunk	+0 lines, -1 line	0 comments	Download
M	third_party/WebKit/Source/core/frame/UseCounter.cpp	View		1 chunk	+0 lines, -3 lines	0 comments	Download
M	third_party/WebKit/Source/core/html/HTMLLinkElement.cpp	View		1 chunk	+1 line, -4 lines	0 comments	Download
M	third_party/WebKit/Source/core/html/parser/HTMLPreloadScanner.cpp	View	1	5 chunks	+9 lines, -0 lines	0 comments	Download

Messages

Total messages: 18 (8 generated)

Expand Messages | Collapse Messages | Show Generated Messages | Hide Generated Messages

commit-bot: I haz the power

Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1623403002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1623403002/1

4 years, 11 months ago (2016-01-25 06:02:31 UTC) #2

commit-bot: I haz the power

Dry run: No L-G-T-M from a valid reviewer yet. Only full committers are accepted. Even ...

4 years, 11 months ago (2016-01-25 06:02:33 UTC) #4

Yoav Weiss

LGTM % comment and nits :) https://codereview.chromium.org/1623403002/diff/1/third_party/WebKit/LayoutTests/http/tests/preload/download_only_supported_stylesheet_types.html File third_party/WebKit/LayoutTests/http/tests/preload/download_only_supported_stylesheet_types.html (right): https://codereview.chromium.org/1623403002/diff/1/third_party/WebKit/LayoutTests/http/tests/preload/download_only_supported_stylesheet_types.html#newcode1 third_party/WebKit/LayoutTests/http/tests/preload/download_only_supported_stylesheet_types.html:1: <!DOCTYPE html> Maybe ...

4 years, 11 months ago (2016-01-25 09:55:55 UTC) #7

suzyh_UTC10 (ex-contributor)

https://codereview.chromium.org/1623403002/diff/1/third_party/WebKit/LayoutTests/http/tests/preload/download_only_supported_stylesheet_types.html File third_party/WebKit/LayoutTests/http/tests/preload/download_only_supported_stylesheet_types.html (right): https://codereview.chromium.org/1623403002/diff/1/third_party/WebKit/LayoutTests/http/tests/preload/download_only_supported_stylesheet_types.html#newcode1 third_party/WebKit/LayoutTests/http/tests/preload/download_only_supported_stylesheet_types.html:1: <!DOCTYPE html> On 2016/01/25 at 09:55:54, Yoav Weiss wrote: ...

4 years, 11 months ago (2016-01-27 00:07:53 UTC) #8

https://codereview.chromium.org/1623403002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/http/tests/preload/download_only_supported_stylesheet_types.html
(right):

https://codereview.chromium.org/1623403002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/preload/download_only_supported_stylesheet_types.html:1:
<!DOCTYPE html>
On 2016/01/25 at 09:55:54, Yoav Weiss wrote:
> Maybe change the path for this test to be under http/tests/css or
http/tests/loading? the preload dir is originally destined at testing <link
rel=preload>

Done.

https://codereview.chromium.org/1623403002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/preload/download_only_supported_stylesheet_types.html:13:
<script src="../resources/slow-script.pl?delay=500"></script>
On 2016/01/25 at 09:55:54, Yoav Weiss wrote:
> Can you bump the delay up before the <link>s (in order to make sure they are
discovered by the preloadScanner and not by the parser?

Done.

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

https://codereview.chromium.org/1623403002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/parser/HTMLPreloadScanner.cpp:289:
m_linkTypeIsSupportedStyleSheet =
MIMETypeRegistry::isSupportedStyleSheetMIMEType(ContentType(attributeValue).type());
On 2016/01/25 at 09:55:54, Yoav Weiss wrote:
> I don't think we need two booleans here. You could change this to
m_linkTypeIsMissingOrSupportedStyleSheet, and initialize it to true.

Done.

https://codereview.chromium.org/1623403002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/parser/HTMLPreloadScanner.cpp:407: }
On 2016/01/25 at 09:55:54, Yoav Weiss wrote:
> Nit: Could you add a new line here, since you're already editing this area?

Done.

commit-bot: I haz the power

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

4 years, 11 months ago (2016-01-27 00:19:05 UTC) #10

commit-bot: I haz the power

Dry run: This issue passed the CQ dry run.

4 years, 11 months ago (2016-01-27 02:05:20 UTC) #12

commit-bot: I haz the power

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

4 years, 11 months ago (2016-01-27 03:37:34 UTC) #15

commit-bot: I haz the power

4 years, 11 months ago (2016-01-27 03:43:50 UTC) #18

Message was sent while issue was closed.

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

Expand Messages | Collapse Messages | Show Generated Messages | Hide Generated Messages