Because RenderRegion can actually be part of other RenderFlowThreads we need to check that it is not actually trying to display a parent RenderFlowThread. Example: <div style="flow: 'flow1';"> <div style="content: from-flow:'flow1'></div> </div> Also we need to check for circular dependencies like the following one: <div style="flow: 'flow1';"> <div style="content: from-flow:'flow2'></div> </div> <div style="flow: 'flow2';"> <div style="content: from-flow:'flow1'></div> </div>
Created attachment 104044 [details] Patch
> Created an attachment (id=104044) [details] > Patch The patch creates a dependency tree and adds a region to a flow thread only if it adds no circular dependency to this tree. The patch also moves the layout of the RenderFlowThreads to a separate method called RenderView::layoutRenderFlowThreads, so that all threads are done at the end of the RenderView layout. It will also sort the regions, so that all the dependent RenderRegions are laid out before they are needed.
Comment on attachment 104044 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=104044&action=review > Source/WebCore/dom/NodeRenderingContext.cpp:373 > + if (newRenderer->isRenderRegion()) > + toRenderRegion(newRenderer)->attachRegion(); Seems like this would be better placed in renderer methods than over here. Maybe in RenderObjectChildList::append/insert/removeChildNode instead. Check out updateListMarkerNumbers call sites in RenderObjectChildList to see where you might attach and remove regions as they enter/leave the tree. > Source/WebCore/rendering/RenderBlock.cpp:1535 > + if (child->isRenderFlowThread()) > + return true; You've turned this into a virtual function call hit by every child you check, even non-positioned ones. This also only handles the insertion of positioned objects from blocks with block-level children. Changing to: if (child->isPositioned && !child->isRenderFlowThread()) would limit the virtual function call only to positioned children instead of making it for all children. However, you also didn't patch insertion from blocks with inline children that happens in RenderBlockLineLayout.cpp near the top of layoutInlineChildren. I think an easier way to handle this might be to just patch insertPositionedObject to ignore RenderFlowThreads, and that way you only have to patch one place.
Created attachment 104070 [details] Patch V2 Thanks for the tips. Updated the patch.
Comment on attachment 104070 [details] Patch V2 You'll need to tweak this after my landing from r93144. You'll want to ditch my m_hasRenderFlowThreads boolean now that you added the list, and you can redefine my hasRenderFlowThreads function on RenderView in terms of the list being non-empty. You'll also want to add isValid checks to the region walk I do in repaintIntoRegions. Sorry for the conflicts. Should be easy to update though.
Created attachment 104083 [details] Patch V3 Rebased.
Comment on attachment 104083 [details] Patch V3 View in context: https://bugs.webkit.org/attachment.cgi?id=104083&action=review Fix the one misspelled method name. Sorry I didn't catch it before. > Source/WebCore/ChangeLog:47 > + (WebCore::RenderRegion::dettachRegion): Typo. Should be detachRegion > Source/WebCore/rendering/RenderObjectChildList.cpp:106 > + toRenderRegion(oldChild)->dettachRegion(); detachRegion > Source/WebCore/rendering/RenderRegion.cpp:111 > +void RenderRegion::dettachRegion() detachRegion > Source/WebCore/rendering/RenderRegion.h:53 > + void dettachRegion(); detachRegion
Created attachment 104096 [details] Patch V4 Fixed it.
Comment on attachment 104096 [details] Patch V4 r=me
Comment on attachment 104096 [details] Patch V4 Rejecting attachment 104096 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-02', '--port..." exit_code: 2 Last 500 characters of output: omium-mac to mean Skia instead of CoreGraphics." Failed to merge in the changes. Patch failed at 0001 This patch prepares for chromium-mac to mean Skia instead of CoreGraphics. When you have resolved this problem run "git rebase --continue". If you would prefer to skip this patch, instead run "git rebase --skip". To restore the original branch and stop rebasing run "git rebase --abort". rebase refs/remotes/origin/master: command returned error: 1 Died at Tools/Scripts/update-webkit line 146. Full output: http://queues.webkit.org/results/9399008
Created attachment 104147 [details] Patch for landing
Comment on attachment 104147 [details] Patch for landing Rejecting attachment 104147 [details] from commit-queue. achicu@adobe.com does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py. - If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags. - If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed). The commit-queue restarts itself every 2 hours. After restart the commit-queue will correctly respect your committer rights.
Created attachment 104149 [details] Patch for landing
Comment on attachment 104149 [details] Patch for landing Rejecting attachment 104149 [details] from commit-queue. achicu@adobe.com does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py. - If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags. - If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed). The commit-queue restarts itself every 2 hours. After restart the commit-queue will correctly respect your committer rights.
Comment on attachment 104149 [details] Patch for landing Rejecting attachment 104149 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-02', '--port..." exit_code: 2 Last 500 characters of output: uto-merging Source/WebCore/platform/chromium/ScrollAnimatorChromiumMac.mm Failed to merge in the changes. Patch failed at 0001 2011-08-16 Andrey Kosyakov <caseq@chromium.org> When you have resolved this problem run "git rebase --continue". If you would prefer to skip this patch, instead run "git rebase --skip". To restore the original branch and stop rebasing run "git rebase --abort". rebase refs/remotes/origin/master: command returned error: 1 Died at Tools/Scripts/update-webkit line 146. Full output: http://queues.webkit.org/results/9401815
*** Bug 66129 has been marked as a duplicate of this bug. ***
Comment on attachment 104149 [details] Patch for landing Clearing flags on attachment: 104149 Committed r93307: <http://trac.webkit.org/changeset/93307>
All reviewed patches have been landed. Closing bug.