Bug 45056 - Clean up issues from r66428.
Summary: Clean up issues from r66428.
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Printing (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows XP
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-09-01 13:35 PDT by Lei Zhang
Modified: 2010-09-01 13:35 PDT (History)
1 user (show)

See Also:


Attachments
first attempt to address concerns from bug 43658. (5.96 KB, patch)
2010-09-01 13:35 PDT, Lei Zhang
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Lei Zhang 2010-09-01 13:35:24 PDT
Created attachment 66255 [details]
first attempt to address concerns from bug 43658.

This patch attempts to address issues raised in https://bugs.webkit.org/show_bug.cgi?id=43658#c16

(In reply to bug 43658 comment #16)
> I see multiple problems with this patch:
> 
> It’s not good how this patch spreads the feature over two classes. The code in both FrameLoader and DOMWindow is inelegant. All the code should be in one class, with the other class calling over as needed. That way you would not have to expose a printDeferred function.

I moved more of the logic out of DOMWindow and into MainResourceLoader.

> The name printDeferred is not a good boolean function or data member name. It sounds like a verb "print deferred". I would name it "should print when finished loading":
> 
>     m_shouldPrintWhenFinishedLoading

Done.

> Also, I think FrameLoader::finishedLoading may not be the right place to call this. That's the function called when the main resources is loaded, not when the entire page is loaded.

I looked into this - in MainResourceLoader::didFinishLoading(), we have:

frameLoader()->finishedLoading();
ResourceLoader::didFinishLoading();

If I try to handle the deferred print after ResourceLoader::didFinishLoading(), it's too late because the data structures we need have been destroyed. Doing it before frameLoader()->finishedLoading() is too early since FrameLoader::isLoading() still returns true. So either we should do it between frameLoader()->finishedLoading() and ResourceLoader::didFinishLoading(), or I need to do it inside frameLoader()->finishedLoading(). I am currently handling it between these two calls, but if you feel it belongs inside frameLoader()->finishedLoading(), I'll look in there to see what's the right place.