Title: [184576] trunk/Source/WebCore
Revision
184576
Author
bda...@apple.com
Date
2015-05-19 10:26:23 -0700 (Tue, 19 May 2015)

Log Message

Crash in WebCore::RenderLayer::updateScrollbarsAfterLayout
https://bugs.webkit.org/show_bug.cgi?id=145142

Reviewed by Simon Fraser.

I have not been able to reproduce this crash, but according to symbolication 
m_vBar is null. It seems like this crash was probably caused by 
http://trac.webkit.org/changeset/173668 which made it so that overflow:scroll 
behaves like overflow:auto when the scrollbars are overlay. I can see how you 
could encounter this crash with that change if the layout caused 
styleRequiresScrollbar() to return true when it used to return false. Then this 
code, by failing to null-check the scrollbars, assumes that 
styleRequiresScrollbar() could not have changed based on a layout. But it could 
change if the css changed the scrollbars to be custom or if the user managed 
switch to legacy style scrollbars at just the wrong time. Or I suppose it could 
also happen if the user has legacy scrollbars and the style switched from auto to 
scroll during the layout.

Anyway, we should null-check  the scrollbars. This is a speculative fix.
* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::updateScrollbarsAfterLayout):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (184575 => 184576)


--- trunk/Source/WebCore/ChangeLog	2015-05-19 17:06:23 UTC (rev 184575)
+++ trunk/Source/WebCore/ChangeLog	2015-05-19 17:26:23 UTC (rev 184576)
@@ -1,3 +1,27 @@
+2015-05-19  Beth Dakin  <bda...@apple.com>
+
+        Crash in WebCore::RenderLayer::updateScrollbarsAfterLayout
+        https://bugs.webkit.org/show_bug.cgi?id=145142
+
+        Reviewed by Simon Fraser.
+
+        I have not been able to reproduce this crash, but according to symbolication 
+        m_vBar is null. It seems like this crash was probably caused by 
+        http://trac.webkit.org/changeset/173668 which made it so that overflow:scroll 
+        behaves like overflow:auto when the scrollbars are overlay. I can see how you 
+        could encounter this crash with that change if the layout caused 
+        styleRequiresScrollbar() to return true when it used to return false. Then this 
+        code, by failing to null-check the scrollbars, assumes that 
+        styleRequiresScrollbar() could not have changed based on a layout. But it could 
+        change if the css changed the scrollbars to be custom or if the user managed 
+        switch to legacy style scrollbars at just the wrong time. Or I suppose it could 
+        also happen if the user has legacy scrollbars and the style switched from auto to 
+        scroll during the layout.
+
+        Anyway, we should null-check  the scrollbars. This is a speculative fix.
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::updateScrollbarsAfterLayout):
+
 2015-05-19  Hunseop Jeong  <hs85.je...@samsung.com>
 
         Use modern for-loops in WebCore/xml.

Modified: trunk/Source/WebCore/rendering/RenderLayer.cpp (184575 => 184576)


--- trunk/Source/WebCore/rendering/RenderLayer.cpp	2015-05-19 17:06:23 UTC (rev 184575)
+++ trunk/Source/WebCore/rendering/RenderLayer.cpp	2015-05-19 17:26:23 UTC (rev 184576)
@@ -3392,9 +3392,9 @@
     bool hasVerticalOverflow = this->hasVerticalOverflow();
 
     // If overflow requires a scrollbar, then we just need to enable or disable.
-    if (styleRequiresScrollbar(renderer().style(), HorizontalScrollbar))
+    if (m_hBar && styleRequiresScrollbar(renderer().style(), HorizontalScrollbar))
         m_hBar->setEnabled(hasHorizontalOverflow);
-    if (styleRequiresScrollbar(renderer().style(), VerticalScrollbar))
+    if (m_vBar && styleRequiresScrollbar(renderer().style(), VerticalScrollbar))
         m_vBar->setEnabled(hasVerticalOverflow);
 
     // Scrollbars with auto behavior may need to lay out again if scrollbars got added or removed.
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to