Bug 43551 - Resource cache is validated for the second load, while not for the third and later
Summary: Resource cache is validated for the second load, while not for the third and ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-08-05 04:45 PDT by Yuzo Fujishima
Modified: 2011-01-06 18:36 PST (History)
7 users (show)

See Also:


Attachments
Patch (11.23 KB, patch)
2010-08-05 05:00 PDT, Yuzo Fujishima
no flags Details | Formatted Diff | Diff
Patch (11.16 KB, patch)
2010-08-05 19:14 PDT, Yuzo Fujishima
no flags Details | Formatted Diff | Diff
Patch (3.76 KB, patch)
2010-08-05 22:59 PDT, Yuzo Fujishima
no flags Details | Formatted Diff | Diff
Patch (4.62 KB, patch)
2010-08-05 23:33 PDT, Yuzo Fujishima
abarth: review-
Details | Formatted Diff | Diff
Sorry, wrong patch. (11.13 KB, patch)
2010-08-08 23:49 PDT, Yuzo Fujishima
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yuzo Fujishima 2010-08-05 04:45:29 PDT
When web font downloading is not fast enough,
the part of the page to be rendered in the font
can be printed as blank.

For example, the following page is printed as blank if
downloading cgi-bin/getfont.py takes, say, seconds,
to complete: 

<style> 
span { 
    font-size: 64px; 
} 
@font-face { 
    font-family:myfont; 
    src: url(cgi-bin/getfont.py); 
} 
</style> 
<span style="font-size:64px; font-family:myfont"> 
This will not be printed on paper in the first try to print. 
</span>

This happens because web font is reloaded when the page is printed.
I think the reloading is unnecessary because the font is refreshed
when the page is rendered on screen.
(Except when different web fonts are used for print and screen using @media)

Note:
This is the main cause of http://code.google.com/p/chromium/issues/detail?id=42499
Comment 1 Yuzo Fujishima 2010-08-05 05:00:35 PDT
Created attachment 63577 [details]
Patch
Comment 2 Yuzo Fujishima 2010-08-05 19:14:35 PDT
Created attachment 63680 [details]
Patch
Comment 3 Yuzo Fujishima 2010-08-05 22:59:57 PDT
Created attachment 63695 [details]
Patch
Comment 4 mitz 2010-08-05 23:04:29 PDT
Comment on attachment 63695 [details]
Patch

The change log should explain what’s going on here.
Comment 5 Yuzo Fujishima 2010-08-05 23:33:40 PDT
Created attachment 63699 [details]
Patch
Comment 6 Yuzo Fujishima 2010-08-05 23:34:51 PDT
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.
Comment 7 mitz 2010-08-05 23:38:17 PDT
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?
Comment 8 Yuzo Fujishima 2010-08-06 00:08:37 PDT
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.
Comment 9 mitz 2010-08-06 00:16:16 PDT
(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.
Comment 10 Yuzo Fujishima 2010-08-06 00:26:20 PDT
Changed the summary line from:
  web font is reloaded in printing page and the relevant part can become blank
Comment 11 Yuzo Fujishima 2010-08-06 00:36:08 PDT
(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.
Comment 12 Alexey Proskuryakov 2010-08-06 04:40:11 PDT
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.
Comment 13 Yuzo Fujishima 2010-08-08 17:58:39 PDT
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.
Comment 14 Yuzo Fujishima 2010-08-08 23:49:25 PDT
Created attachment 63862 [details]
Sorry, wrong patch.
Comment 15 Yuzo Fujishima 2010-08-08 23:52:15 PDT
Comment on attachment 63862 [details]
Sorry, wrong patch.

Uploaded to this bug by a mistake
Comment 16 Yuzo Fujishima 2010-08-08 23:53:27 PDT
Filed a bug for web font printing issue related to caching:
https://bugs.webkit.org/show_bug.cgi?id=43704
Comment 17 Yuzo Fujishima 2010-09-07 21:50:45 PDT
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 18 Adam Barth 2010-10-10 12:47:03 PDT
Comment on attachment 63699 [details]
Patch

We need a test.  Also DocLoader.cpp => CachedResourceLoader.cpp.
Comment 19 Yuzo Fujishima 2010-11-21 22:29:37 PST
I'd like to release the ownership of this bug because I failed to get enough attention to my patch.
Comment 20 Yuzo Fujishima 2011-01-06 18:36:02 PST
r74807 seems to have fixed the loading behavior.