[Webkit-unassigned] [Bug 45056] New: Clean up issues from r66428.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=45056

           Summary: Clean up issues from r66428.
           Product: WebKit
           Version: 528+ (Nightly build)
          Platform: PC
        OS/Version: Windows XP
            Status: NEW
          Severity: Normal
          Priority: P2
         Component: Printing
        AssignedTo: webkit-unassigned at lists.webkit.org
        ReportedBy: thestig at chromium.org
                CC: darin at apple.com


Created an attachment (id=66255)
 --> (https://bugs.webkit.org/attachment.cgi?id=66255)
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.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.