Bug 43551

Created attachment 63577 [details] Patch

Created attachment 63680 [details] Patch

Created attachment 63695 [details] Patch

Comment on attachment 63695 [details] Patch The change log should explain what’s going on here.

Created attachment 63699 [details] Patch

Hi, mitz, Thank you for the review. I've added comment to WebCore/ChangeLog. Can you take another look? (In reply to comment #4) > (From update of attachment 63695 [details]) > The change log should explain what’s going on here.

Such a global change to WebKit’s resource loading behavior doesn’t seem like a reasonable approach to fixing this issue. I also don’t immediately see what makes 2 the magic number here. Is it not possible that the page would be printed after some resources have already been loaded a second time?

Hi, thanks again for the review. (In reply to comment #7) > Such a global change to WebKit’s resource loading behavior doesn’t seem like a reasonable approach to fixing this issue. Perhaps I should propose this patch under a different bug, something like: "Cache validation should be skipped for a resource when it is loaded for the second time (or later) for a same document. Currently, validation is skipped for the third load and later." > I also don’t immediately see what makes 2 the magic number here. The current behavior, i.e., no cache validation for the third time and later, is as questionable as my proposal. With a page similar to below, you should be able to see that WebKit doesn't request the image for the third time and later you click the button. (Please use a fresh instance of Safari to make sure nothing is cached at the beginning): <script> function addImage() { var newImage = document.createElement("img"); newImage.setAttribute("src", "cgi-bin/getlogo.py"); document.body.appendChild(newImage); } </script> <input type=button value=Change onclick="addImage()"/> Please see the server log (or make getlogo.py return different image every time). > Is it not possible that the page would be printed after some resources have already been loaded a second time? I'm not sure if I understand your question correctly. Do you mean we should repeat loading resources before the user requests printing a page (that way, reloading done for print would be the third one and skip the cache validation)? I'm afraid that would be inefficient. Or are you asking if my patch will break if the resource has been loaded two times before printing? It won't.

(In reply to comment #8) > Hi, thanks again for the review. > > (In reply to comment #7) > > Such a global change to WebKit’s resource loading behavior doesn’t seem like a reasonable approach to fixing this issue. > > Perhaps I should propose this patch under a different bug, something like: > "Cache validation should be skipped for a resource when it is loaded for the > second time (or later) for a same document. Currently, validation is skipped for > the third load and later." I think that would be a better, yes. I suggest that you re-title the bug. I’ve moved it to the Page Loading component where it belongs. You may also want to have a separate bug about the specific symptom you’re seeing when printing. The fact that preparing the page for printing sometimes causes resources to load is not limited to this case of reloading a font resource. A printing style sheet might specify fonts, images and other stylesheets that are not referenced by the screen style sheet, presumably leading to incomplete output if printing is synchronous and does not block on loading those resources. > > Is it not possible that the page would be printed after some resources have already been loaded a second time? > > I'm not sure if I understand your question correctly. > > Do you mean we should repeat loading resources before the user requests printing a page > (that way, reloading done for print would be the third one and skip the cache validation)? > I'm afraid that would be inefficient. > > Or are you asking if my patch will break if the resource has been loaded two times before printing? That was my question. > It won't. How come? I am not an expert on loading behavior, other than to know that there are intricate performance, correctness and compatibility considerations involved in this area, so I’ve CC:ed Brady Eidson on the bug.

Changed the summary line from: web font is reloaded in printing page and the relevant part can become blank

(In reply to comment #9) > (In reply to comment #8) > > Hi, thanks again for the review. > > > > (In reply to comment #7) > > > Such a global change to WebKit’s resource loading behavior doesn’t seem like a reasonable approach to fixing this issue. > > > > Perhaps I should propose this patch under a different bug, something like: > > "Cache validation should be skipped for a resource when it is loaded for the > > second time (or later) for a same document. Currently, validation is skipped for > > the third load and later." > > I think that would be a better, yes. I suggest that you re-title the bug. I’ve moved it to the Page Loading component where it belongs. Thank you. Changed the title. > > You may also want to have a separate bug about the specific symptom you’re seeing when printing. The fact that preparing the page for printing sometimes causes resources to load is not limited to this case of reloading a font resource. A printing style sheet might specify fonts, images and other stylesheets that are not referenced by the screen style sheet, presumably leading to incomplete output if printing is synchronous and does not block on loading those resources. Yes, I've been aware of it. The case I'm trying to save now with the patch is that the same web font is used both for screen and print. It won't save the case where different web fonts are used, for example, via @media print { @font-face ... }. I agree that if there is a way to either (1) print asynchronously or (2) block until all necessary resources are downloaded, we can solve the web font printing issue. Actually I tried to find such a way before I work on this patch. But I couldn't find a reasonable way. > > > > Is it not possible that the page would be printed after some resources have already been loaded a second time? > > > > I'm not sure if I understand your question correctly. > > > > Do you mean we should repeat loading resources before the user requests printing a page > > (that way, reloading done for print would be the third one and skip the cache validation)? > > I'm afraid that would be inefficient. > > > > Or are you asking if my patch will break if the resource has been loaded two times before printing? > > That was my question. > > > It won't. > > How come? > > I am not an expert on loading behavior, other than to know that there are intricate performance, correctness and compatibility considerations involved in this area, so I’ve CC:ed Brady Eidson on the bug. I admit that I don't understand why the current code skip cache validation for the third load and later. I'd love to hear from Brady.

This sounds almost exactly the same as bug 27971. When looking at that bug, I had a feeling that it was CSS machinery fault that a font was reloaded in the first place. I don't understand what page loading does wrong here. It seems wrong to skip revalidation. E.g. if we're dealing with a webcam that provides an image that's constantly updated, and the web page reloads it all the time, it should be requested from server.

Hi, Alexey, Thank you for the comments. (In reply to comment #12) > This sounds almost exactly the same as bug 27971. > > When looking at that bug, I had a feeling that it was CSS machinery fault that a font was reloaded in the first place. I don't understand what page loading does wrong here. > > It seems wrong to skip revalidation. E.g. if we're dealing with a webcam that provides an image that's constantly updated, and the web page reloads it all the time, it should be requested from server. Neither the current behavior (no validation at the third and later) or the proposed behavior (no validation at the second and later) are good in that sense. My proposal has an advantage: it fixes a subset of web font printing issue. I do agree that the proposal is a quick fix that is not very reliable.

Created attachment 63862 [details] Sorry, wrong patch.

Comment on attachment 63862 [details] Sorry, wrong patch. Uploaded to this bug by a mistake

Filed a bug for web font printing issue related to caching: https://bugs.webkit.org/show_bug.cgi?id=43704

Hi, Brady, Any comments on this? I think cache validation should happen every time, as Alexey pointed out. (I'd exclude the validations at screen/print switching time, though. But this is a different story.) I could understand limiting validation to only the first load for a page, trading correctness for the performance gain, I guess. I don't understand the current behavior, though: validation for the first and the second load, no validation there after. What is the rationale behind the current behavior?

Comment on attachment 63699 [details] Patch We need a test. Also DocLoader.cpp => CachedResourceLoader.cpp.

I'd like to release the ownership of this bug because I failed to get enough attention to my patch.

r74807 seems to have fixed the loading behavior.