Issue metadata
Sign in to add a comment
|
DevTools: [network] unused webfonts are shown as pending network requests
Reported by
fatimah....@gmail.com,
Jun 27 2016
|
||||||||||||||||||||||
Issue descriptionDevice name: Samsung S5 From "Settings > About Chrome" Application version: 53.0.2781.0 OS: Kitkat 4.4.4 URLs (if applicable): Any URL that would lead to fetching .ttf and .woff fonts. Example: https://m.washingtonpost.com Behavior in Android Browser (if applicable): The Remote Debugging Tool always shows that the .ttf and .woff fonts are "pending", and 0B are downloaded. I am not sure if the problem is with the Remote Debugging Tool's representation of what is happening or with Android Chrome, actually not fetching web objects. I verified that the problem occurs even when we have the appropriate fonts exist on the server. The same problem also occurs with the latest version of the Android Chrome Dev, version 53.0.2774.4 Steps to reproduce: (1) Connect the phone to the computer and run the Remote Debugging Tool. (2) Fetch: https://m.washingtonpost.com (3) Observe that .ttf and .woff fonts are always pending. Expected result: The fonts should get downloaded. Actual result: We always see that the fonts are pending.
,
Jun 27 2016
,
Jun 27 2016
,
Jun 28 2016
Fonts are shown as "pending" until the font is actually used by some texts on ToT/Dev. I can see the same behavior on Linux and Android on ToT, but can not on the stable channel. On the stable channel, resources that are shown as "pending" on ToT never appear in the DevTool network tab. fatimah.zarinni@gmail.com: Can you check if the font is actually used in the site, or create a minimized test scenario?
,
Jul 1 2016
,
Jul 1 2016
This affects both network panel and timeline panel. Attaches are screenshots of https://m.washingtonpost.com/ Toyoshim, do you know why fonts go through FrameFetchContext::dispatchWillSendRequest but apparently do not begin their network request then? This seems to be what is adding confusion to our instrumentation.
,
Jul 1 2016
Issue 625158 has been merged into this issue.
,
Jul 15 2016
hiroshige: in terms of core/fetch, is this an expected behavior change? This change may be related to recent refactoring on Resource.
,
Jul 15 2016
> but apparently do not begin their network request then? Blink's loader defers the loading of WebFonts until it is actually used (I expect this behavior hasn't changed recently). > why fonts go through FrameFetchContext::dispatchWillSendRequest A series of recent refactoring changed how WebFonts' deferred loading is implemented, and it might change whether/when dispatchWillSendRequest() is called for deferred WebFonts. FrameFetchContext::dispatchWillSendRequest() is called from ResourceFetcher::requestResource(). https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp?sq=package:chromium&dr=CSs&rcl=1468538294&l=469 Later in requestResource(), resourceNeedsLoad() is false for WebFonts and thus requestResource() returns without calling startLoad(). https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp?sq=package:chromium&dr=CSs&rcl=1468538294&l=518
,
Jul 15 2016
What is the expected behavior? Should FrameFetchContext::dispatchWillSendRequest() called only when (and just before) the actual load is starting?
,
Jul 15 2016
,
Jul 20 2016
This is annoying... Let me raise the priority. > Should FrameFetchContext::dispatchWillSendRequest() called only when (and just before) the actual load is starting? Yeah that sounds like the right thing to do (I don't understand full implications though). Hiroshige-san, could you create a patch?
,
Jul 28 2016
Regressed by: https://codereview.chromium.org/1889973002, affecting M-53. Creating a CL: https://codereview.chromium.org/2184823006/
,
Jul 28 2016
Moving this nonessential bug to the next milestone. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jul 29 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1b4209891f51609fe5aa84e13be6dd482ca734c7 commit 1b4209891f51609fe5aa84e13be6dd482ca734c7 Author: hiroshige <hiroshige@chromium.org> Date: Fri Jul 29 06:03:10 2016 Call FetchContext::dispatchWillSendRequest() only if an actual load is started Fixing a regression since [1]. [1] https://codereview.chromium.org/1889973002/ Since [1], FetchContext::dispatchWillSendRequest() was called from ResourceFetcher::willSendRequest() in the middle of ResourceFetcher::requestResource(), and might have modified Resource::m_resourceRequest. However, if the actual loading is deferred, e.g. when FontResource is not used, FetchContext::dispatchWillSendRequest() was called without actual loading, causing stale pending load reports in DevTools. This CL - Moves the ResourceFetcher::willSendRequest() call in requestResource() to ResourceFetcher::startLoad(). Now FetchContext::dispatchWillSendRequest() (called from startLoad()) modifies the local ResourceRequest in startLoad(), not modifying ResourceRequest::m_resourceRequest (which is the behavior before [1]). - Moves setAllowStoredCredentials() call in ResourceFetcher::willSendRequest() to its callsites, because this should be done even when the actual loading is deferred and should modify Resource::m_resourceRequest. - Adds a layout test. BUG= 623616 , 632580 Review-Url: https://codereview.chromium.org/2184823006 Cr-Commit-Position: refs/heads/master@{#408588} [add] https://crrev.com/1b4209891f51609fe5aa84e13be6dd482ca734c7/third_party/WebKit/LayoutTests/http/tests/inspector/network/font-face-expected.txt [add] https://crrev.com/1b4209891f51609fe5aa84e13be6dd482ca734c7/third_party/WebKit/LayoutTests/http/tests/inspector/network/font-face.html [add] https://crrev.com/1b4209891f51609fe5aa84e13be6dd482ca734c7/third_party/WebKit/LayoutTests/http/tests/inspector/network/resources/font-face.html [modify] https://crrev.com/1b4209891f51609fe5aa84e13be6dd482ca734c7/third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp
,
Aug 2 2016
Tested the issue on windows 7, Linux Ubuntu 14.04 and Mac 10.11.6 using chrome version 54.0.2816.0 with the below steps 1. Go to URL https://m.washingtonpost.com/ 2.Open dev tools,network panel. 3.Reload(F5) .Not observed any .ttf and .woff fonts pending Please find the attached screen cast for the same.Adding TE-Verified labels. Thanks,
,
Aug 2 2016
,
Aug 2 2016
Your change meets the bar and is auto-approved for M53 (branch: 2785)
,
Aug 2 2016
We're cutting M53 Beta RC today for release tomorrow. Please try to merge your change to M53 branch 2785 before 5:30 PM PT today so we can take it for this week beta. Thank you.
,
Aug 3 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e8573e904dd5ed11adb5eac18ac601963e90cc14 commit e8573e904dd5ed11adb5eac18ac601963e90cc14 Author: Hiroshige Hayashizaki <hiroshige@chromium.org> Date: Wed Aug 03 07:29:32 2016 Call FetchContext::dispatchWillSendRequest() only if an actual load is started Fixing a regression since [1]. [1] https://codereview.chromium.org/1889973002/ Since [1], FetchContext::dispatchWillSendRequest() was called from ResourceFetcher::willSendRequest() in the middle of ResourceFetcher::requestResource(), and might have modified Resource::m_resourceRequest. However, if the actual loading is deferred, e.g. when FontResource is not used, FetchContext::dispatchWillSendRequest() was called without actual loading, causing stale pending load reports in DevTools. This CL - Moves the ResourceFetcher::willSendRequest() call in requestResource() to ResourceFetcher::startLoad(). Now FetchContext::dispatchWillSendRequest() (called from startLoad()) modifies the local ResourceRequest in startLoad(), not modifying ResourceRequest::m_resourceRequest (which is the behavior before [1]). - Moves setAllowStoredCredentials() call in ResourceFetcher::willSendRequest() to its callsites, because this should be done even when the actual loading is deferred and should modify Resource::m_resourceRequest. - Adds a layout test. BUG= 623616 , 632580 Review-Url: https://codereview.chromium.org/2184823006 Cr-Commit-Position: refs/heads/master@{#408588} (cherry picked from commit 1b4209891f51609fe5aa84e13be6dd482ca734c7) Review URL: https://codereview.chromium.org/2207833002 . Cr-Commit-Position: refs/branch-heads/2785@{#480} Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382} [add] https://crrev.com/e8573e904dd5ed11adb5eac18ac601963e90cc14/third_party/WebKit/LayoutTests/http/tests/inspector/network/font-face-expected.txt [add] https://crrev.com/e8573e904dd5ed11adb5eac18ac601963e90cc14/third_party/WebKit/LayoutTests/http/tests/inspector/network/font-face.html [add] https://crrev.com/e8573e904dd5ed11adb5eac18ac601963e90cc14/third_party/WebKit/LayoutTests/http/tests/inspector/network/resources/font-face.html [modify] https://crrev.com/e8573e904dd5ed11adb5eac18ac601963e90cc14/third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp
,
Aug 3 2016
Closing as fixed: Regression since M-53 (Comment 13) and the fix is merged into M-53 (Comment 20).
,
Aug 3 2016
Thank you for fixing the issue ! Best regards, Fatima
,
Aug 4 2016
Tested the issue on windows 8.1, Linux Ubuntu 14.04 and Mac 10.11.5 using chrome version 53.0.3785.46 navigating to https://m.washingtonpost.com and reloading - Not observed any .ttf and .woff fonts pending Please find the screencast Adding TE-Verified labels
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by rsgav...@chromium.org
, Jun 27 2016