Bug 62762

Summary: ScrollableAreaSet should be moved from Page to FrameView
Product: WebKit Reporter: Beth Dakin <bdakin>
Component: Layout and RenderingAssignee: Anders Carlsson <andersca>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, bdakin, dglazkov, gns, sam, tkent, tonikitoo, webkit.review.bot, xan.lopez, zimmermann
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch bdakin: review+

Description Beth Dakin 2011-06-15 15:58:27 PDT
The ScrollableAreaSet that currently exists on Page should be moved to something that represents the top-level Document. We should consider hanging a structure off of Document to represent the top-level Document and moved this HashSet there. Even if we don't do that though, the HashSet of ScrollableAreas should move to a more appropriate class.
Comment 1 Anders Carlsson 2012-02-06 14:41:43 PST
Actually, it would be even better to move it to FrameView!
Comment 2 Anders Carlsson 2012-02-06 18:20:00 PST
Created attachment 125746 [details]
Patch
Comment 3 Early Warning System Bot 2012-02-06 19:21:47 PST
Comment on attachment 125746 [details]
Patch

Attachment 125746 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/11442110
Comment 4 Gyuyoung Kim 2012-02-06 19:25:07 PST
Comment on attachment 125746 [details]
Patch

Attachment 125746 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/11436109
Comment 5 WebKit Review Bot 2012-02-06 19:39:25 PST
Comment on attachment 125746 [details]
Patch

Attachment 125746 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11442118
Comment 6 Gustavo Noronha (kov) 2012-02-06 21:24:22 PST
Comment on attachment 125746 [details]
Patch

Attachment 125746 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/11442152
Comment 7 Sam Weinig 2012-02-06 21:42:18 PST
This should probably build on some platform.
Comment 8 Simon Fraser (smfr) 2012-02-07 00:43:32 PST
Comment on attachment 125746 [details]
Patch

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

> Source/WebCore/ChangeLog:3
> +        ScrollableAreaSet should be moved from Page to FrameView

Changelog needs a "why".
Comment 9 Anders Carlsson 2012-02-07 11:35:32 PST
Created attachment 125883 [details]
Patch
Comment 10 WebKit Review Bot 2012-02-07 11:57:50 PST
Comment on attachment 125883 [details]
Patch

Attachment 125883 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11453035
Comment 11 Anders Carlsson 2012-02-07 12:01:57 PST
Created attachment 125895 [details]
Patch
Comment 12 Anders Carlsson 2012-02-07 12:46:27 PST
Committed r106977: <http://trac.webkit.org/changeset/106977>
Comment 13 Nikolas Zimmermann 2012-02-08 00:53:01 PST
This made a SVG test crash:

nzimmermann ~/Coding/WebKit > WebKitBuild/Debug/DumpRenderTree LayoutTests/svg/dom/SVGStyledElement-pendingResource-crash.html
ASSERTION FAILED: scrollableArea->isOnActivePage()
/Users/nzimmermann/Coding/WebKit/Source/WebCore/page/FrameView.cpp(2609) : virtual void WebCore::FrameView::notifyPageThatContentAreaWillPaint() const
1   0x10cd68be1 WebCore::FrameView::notifyPageThatContentAreaWillPaint() const
2   0x10dac6e02 WebCore::ScrollView::repaintContentRectangle(WebCore::IntRect const&, bool)
3   0x10cd66425 WebCore::FrameView::repaintContentRectangle(WebCore::IntRect const&, bool)
4   0x10da12769 WebCore::RenderView::repaintViewRectangle(WebCore::IntRect const&, bool)
5   0x10d91a605 WebCore::RenderObject::repaintUsingContainer(WebCore::RenderBoxModelObject*, WebCore::IntRect const&, bool)
6   0x10d91a79d WebCore::RenderObject::repaint(bool)
7   0x10d922dcb WebCore::RenderObjectChildList::removeChildNode(WebCore::RenderObject*, WebCore::RenderObject*, bool)
8   0x10d9142ae WebCore::RenderObject::removeChild(WebCore::RenderObject*)
9   0x10d7df197 WebCore::RenderBlock::removeChild(WebCore::RenderObject*)
10  0x10d8afef6 WebCore::RenderObject::remove()
11  0x10d91f603 WebCore::RenderObject::willBeDestroyed()
12  0x10d85a66b WebCore::RenderBoxModelObject::willBeDestroyed()
13  0x10d842ae3 WebCore::RenderBox::willBeDestroyed()
14  0x10d930fb5 WebCore::RenderReplaced::willBeDestroyed()
15  0x10da1fc5c WebCore::RenderWidget::willBeDestroyed()
16  0x10da1fc9b WebCore::RenderWidget::destroy()
17  0x10d710a90 WebCore::Node::detach()
18  0x10c8581e4 WebCore::ContainerNode::detach()
19  0x10cc6b355 WebCore::Element::detach()
20  0x10cec8f49 WebCore::HTMLPlugInElement::detach()
21  0x10ceca7f6 WebCore::HTMLPlugInImageElement::detach()
22  0x10c857a8e WebCore::ContainerNode::removeChildren()
23  0x10ce5397f _ZN7WebCoreL27replaceChildrenWithFragmentEPNS_11HTMLElementEN3WTF10PassRefPtrINS_16DocumentFragmentEEERi
24  0x10ce536ad WebCore::HTMLElement::setInnerHTML(WTF::String const&, int&)
25  0x10d280925 WebCore::setJSHTMLElementInnerHTML(JSC::ExecState*, JSC::JSObject*, JSC::JSValue)
26  0x10d282aa1 bool JSC::lookupPut<WebCore::JSHTMLElement>(JSC::ExecState*, JSC::Identifier const&, JSC::JSValue, JSC::HashTable const*, WebCore::JSHTMLElement*, bool)
27  0x10d2824c8 void JSC::lookupPut<WebCore::JSHTMLElement, WebCore::JSElement>(JSC::ExecState*, JSC::Identifier const&, JSC::JSValue, JSC::HashTable const*, WebCore::JSHTMLElement*, JSC::PutPropertySlot&)
28  0x10d27f547 WebCore::JSHTMLElement::put(JSC::JSCell*, JSC::ExecState*, JSC::Identifier const&, JSC::JSValue, JSC::PutPropertySlot&)
29  0x10d267414 void JSC::lookupPut<WebCore::JSHTMLBodyElement, WebCore::JSHTMLElement>(JSC::ExecState*, JSC::Identifier const&, JSC::JSValue, JSC::HashTable const*, WebCore::JSHTMLBodyElement*, JSC::PutPropertySlot&)
30  0x10d2649b7 WebCore::JSHTMLBodyElement::put(JSC::JSCell*, JSC::ExecState*, JSC::Identifier const&, JSC::JSValue, JSC::PutPropertySlot&)
31  0x10b5c0b7d JSC::JSValue::put(JSC::ExecState*, JSC::Identifier const&, JSC::JSValue, JSC::PutPropertySlot&)
Comment 14 Anders Carlsson 2012-02-08 10:00:57 PST
(In reply to comment #13)
> This made a SVG test crash:
> 
> nzimmermann ~/Coding/WebKit > WebKitBuild/Debug/DumpRenderTree LayoutTests/svg/dom/SVGStyledElement-pendingResource-crash.html

Oops, fixed in<http://trac.webkit.org/changeset/107102>.
Comment 15 Antonio Gomes 2012-02-17 07:44:33 PST
Is there any missing work, Anders?
Comment 16 Anders Carlsson 2012-02-17 08:28:53 PST
(In reply to comment #15)
> Is there any missing work, Anders?

Nope, the assertion has been fixed! Closing.