Bug 45007

Summary: Remove rendering types from SubframeLoader and split RenderEmbeddedObject::updateWidget in preparation for moving to DOM
Product: WebKit Reporter: Eric Seidel (no email) <eric>
Component: New BugsAssignee: Eric Seidel (no email) <eric>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, commit-queue, eric, jamesr, simon.fraser, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Bug Depends on:    
Bug Blocks: 45049    
Attachments:
Description Flags
wip
none
Patch none

Description Eric Seidel (no email) 2010-08-31 16:58:30 PDT
Remove rendering types from SubframeLoader and split RenderEmbeddedObject::updateWidget in preparation for moving to DOM
Comment 1 Eric Seidel (no email) 2010-08-31 17:30:34 PDT
Created attachment 66147 [details]
wip
Comment 2 Eric Seidel (no email) 2010-08-31 23:17:08 PDT
Created attachment 66170 [details]
Patch
Comment 3 Eric Seidel (no email) 2010-08-31 23:18:45 PDT
I'm happy to divide this patch into smaller chunks if that would make review easier.

This is all prep work for getting rid of the lazyAttach codepath. :)  Once this is moved to the DOM, then the DOM can force the style update if needed to decide if a plugin should load or not.  Or we can remove the display: none causes no-load branch if its decided that's better for web compat (but that's a separate issue).
Comment 4 Adam Barth 2010-08-31 23:27:35 PDT
Comment on attachment 66170 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=66170&action=prettypatch

> WebCore/html/HTMLFrameOwnerElement.h:48
> +    RenderPart* renderPart() const;
This is called RenderPart?  Maybe it should be renamed to RenderFrame?

> WebCore/loader/SubframeLoader.cpp:107
> +    // FIXME: None of this code should use renderers!
> +    RenderEmbeddedObject* renderer = ownerElement->renderEmbeddedObject();
> +    ASSERT(renderer);
> +    if (!renderer)
> +        return false;
The image loading system talks to renderers too, btw.
Comment 5 Dimitri Glazkov (Google) 2010-09-01 14:40:44 PDT
Comment on attachment 66170 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=66170&action=prettypatch

> WebCore/rendering/RenderEmbeddedObject.cpp:369
> +        updateWidgetForMediaElement(mediaElement, onlyCreateNonNetscapePlugins);

I clearly started reviewing this in the wrong order :)
Comment 6 WebKit Commit Bot 2010-09-01 19:12:12 PDT
Comment on attachment 66170 [details]
Patch

Clearing flags on attachment: 66170

Committed r66631: <http://trac.webkit.org/changeset/66631>
Comment 7 WebKit Commit Bot 2010-09-01 19:12:17 PDT
All reviewed patches have been landed.  Closing bug.
Comment 8 WebKit Review Bot 2010-09-02 02:23:45 PDT
http://trac.webkit.org/changeset/66631 might have broken Leopard Intel Debug (Tests)
The following changes are on the blame list:
http://trac.webkit.org/changeset/66629
http://trac.webkit.org/changeset/66630
http://trac.webkit.org/changeset/66631