[Chromium] Add a signal for when the body is inserted in the document
Created attachment 186198 [details] Patch
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
What does inserting the body have to do with knowing we have enough to paint? That just means we hit either the <body> or any non-<head> tag, but it doesn't mean we have reasonable content. Is this emperically determined to help?
It is empirically determined to help. It's based on the comment & code here: https://code.google.com/searchframe#OAMlx_jo-ck/src/third_party/WebKit/Source/WebCore/loader/cache/CachedResourceLoader.cpp&exact_package=chromium&q=cachedresourceloader.cpp&type=cs&l=833
Err, the comment is a few lines up.
That heuristic is a little different since it's checking for the body element's renderer. That won't become non-null until we do the first style resolution (which is hopefully when doing the first layout). That's also a horrible layering violation for the loader to be checking for a render object :/
Yeah, I know it's not exact. Basically, we want to know that parser has made it through all of the parse blocking resources in the head before we start loading lower priority resources like images. We don't actually want to wait for the layout to finish before starting in.
Comment on attachment 186198 [details] Patch OK. I know the page cyclers are extremely sensitive to the timing of when that first style resolution+layout ends up happening, but maybe that's not as true for this. LGTM for the WebKit API parts. I don't know enough about the /html/ and /loader/ interactions to set the review? flag. Maybe japhet could do the honors?
Comment on attachment 186198 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=186198&action=review > Source/WebCore/html/HTMLBodyElement.cpp:190 > + if (Frame* frame = document()->frame()) > + frame->loader()->client()->dispatchDidInsertBody(); This is wrong. It will fire every time a body element is inserted anywhere, regardless of whether it's document.body. Instead what you want is a notification at the end of HTMLConstructionSite::insertHTMLBodyElement
(More specifically, when the task queued by that function executes.)
Created attachment 186474 [details] Patch
Thanks for the tip. That's a better location for it. I actually don't mind dispatching this message before it's actually inserted in the document. If the parser has gotten this far, it means nothing external will block the body from being inserted, which is what I care about. I've updated the function name to match (did -> will).
Comment on attachment 186474 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=186474&action=review > Source/WebCore/loader/FrameLoaderClient.h:375 > + virtual void dispatchWillInsertBody() { } I might add a comment about this only working for HTML document (e.g., not for SVG since they don't have a <body> element). > Source/WebKit/chromium/public/WebFrameClient.h:331 > + // The <body> tag is now present. I'd also mentioning that this makes sense only for HTML document here too.
This patch seems fine to me, but it would be good to have a unit test showing that we call the callback.
Created attachment 186517 [details] Patch
Comments updated and test added.
Comment on attachment 186517 [details] Patch Thanks!
Comment on attachment 186517 [details] Patch Clearing flags on attachment: 186517 Committed r141850: <http://trac.webkit.org/changeset/141850>
All reviewed patches have been landed. Closing bug.