Bug 45007 - Remove rendering types from SubframeLoader and split RenderEmbeddedObject::updateWidget in preparation for moving to DOM
Summary: Remove rendering types from SubframeLoader and split RenderEmbeddedObject::up...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Eric Seidel (no email)
URL:
Keywords:
Depends on:
Blocks: 45049
  Show dependency tree / graph
 
Reported: 2010-08-31 16:58 PDT by Eric Seidel (no email)
Modified: 2010-09-02 02:23 PDT (History)
6 users (show)

See Also:

Attachments
wip (34.10 KB, patch)
2010-08-31 17:30 PDT, Eric Seidel (no email) 		no flags Details | Formatted Diff | Diff
Patch (38.42 KB, patch)
2010-08-31 23:17 PDT, Eric Seidel (no email) 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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