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

Eric Seidel (no email)
Reported 2010-08-31 16:58:30 PDT
Remove rendering types from SubframeLoader and split RenderEmbeddedObject::updateWidget in preparation for moving to DOM
Attachments
wip (34.10 KB, patch)
2010-08-31 17:30 PDT, Eric Seidel (no email) 		no flags
DetailsFormatted DiffDiff
Patch (38.42 KB, patch)
2010-08-31 23:17 PDT, Eric Seidel (no email) 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Eric Seidel (no email)
Comment 1 2010-08-31 17:30:34 PDT
Created attachment 66147 [details] wip
Eric Seidel (no email)
Comment 2 2010-08-31 23:17:08 PDT
Created attachment 66170 [details] Patch
Eric Seidel (no email)
Comment 3 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).
Adam Barth
Comment 4 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.
Dimitri Glazkov (Google)
Comment 5 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 :)
WebKit Commit Bot
Comment 6 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>
WebKit Commit Bot
Comment 7 2010-09-01 19:12:17 PDT
All reviewed patches have been landed. Closing bug.
WebKit Review Bot
Comment 8 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
Note You need to log in before you can comment on or make changes to this bug.