Changeset 126591 in webkit


Ignore:
Timestamp:
Aug 24, 2012 9:39:43 AM (12 years ago)
Author:
jchaffraix@webkit.org
Message:

Crash in RenderTableCell::borderTop() due to custom scrollbars after r124168
https://bugs.webkit.org/show_bug.cgi?id=93903

Reviewed by Tony Chang.

Source/WebCore:

r124168 changed when we create scrollbars so that it happens (rightly) at style change time.
However the RenderScrollbar code assumes that any overflow: scroll RenderScrollbar would be
created at layout time as it directly tries to layout to scrollbar parts. The big issues with
the move is that the first style change operates on a detached renderer which means that we
could crash in some situation.

Test: scrollbars/custom-scrollbar-table-cell.html

  • rendering/RenderScrollbarPart.cpp:

(WebCore::RenderScrollbarPart::computeScrollbarWidth):
(WebCore::RenderScrollbarPart::computeScrollbarHeight):
Fixed the crash by using style information instead of calling the renderer. This is guaranteed
to be safe but it also means that custom scrollbars sizing is not right on table cells with
collapsing borders. The existing logic was querying layout information at style change so I
wouldn't bet on it working properly anyway.

LayoutTests:

  • scrollbars/custom-scrollbar-table-cell-expected.png: Added.
  • scrollbars/custom-scrollbar-table-cell-expected.txt: Added.
  • scrollbars/custom-scrollbar-table-cell.html: Added.
Location:
trunk
Files:
3 added
3 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r126589 r126591  
     12012-08-24  Julien Chaffraix  <jchaffraix@webkit.org>
     2
     3        Crash in RenderTableCell::borderTop() due to custom scrollbars after r124168
     4        https://bugs.webkit.org/show_bug.cgi?id=93903
     5
     6        Reviewed by Tony Chang.
     7
     8        * scrollbars/custom-scrollbar-table-cell-expected.png: Added.
     9        * scrollbars/custom-scrollbar-table-cell-expected.txt: Added.
     10        * scrollbars/custom-scrollbar-table-cell.html: Added.
     11
    1122012-08-24  Alexander Pavlov  <apavlov@chromium.org>
    213
  • trunk/Source/WebCore/ChangeLog

    r126590 r126591  
     12012-08-24  Julien Chaffraix  <jchaffraix@webkit.org>
     2
     3        Crash in RenderTableCell::borderTop() due to custom scrollbars after r124168
     4        https://bugs.webkit.org/show_bug.cgi?id=93903
     5
     6        Reviewed by Tony Chang.
     7
     8        r124168 changed when we create scrollbars so that it happens (rightly) at style change time.
     9        However the RenderScrollbar code assumes that any overflow: scroll RenderScrollbar would be
     10        created at layout time as it directly tries to layout to scrollbar parts. The big issues with
     11        the move is that the first style change operates on a detached renderer which means that we
     12        could crash in some situation.
     13
     14        Test: scrollbars/custom-scrollbar-table-cell.html
     15
     16        * rendering/RenderScrollbarPart.cpp:
     17        (WebCore::RenderScrollbarPart::computeScrollbarWidth):
     18        (WebCore::RenderScrollbarPart::computeScrollbarHeight):
     19        Fixed the crash by using style information instead of calling the renderer. This is guaranteed
     20        to be safe but it also means that custom scrollbars sizing is not right on table cells with
     21        collapsing borders. The existing logic was querying layout information at style change so I
     22        wouldn't bet on it working properly anyway.
     23
    1242012-08-24  Julien Chaffraix  <jchaffraix@webkit.org>
    225
  • trunk/Source/WebCore/rendering/RenderScrollbarPart.cpp

    r122264 r126591  
    9292        return;
    9393    RenderView* renderView = view();
    94     int visibleSize = m_scrollbar->owningRenderer()->width() - m_scrollbar->owningRenderer()->borderLeft() - m_scrollbar->owningRenderer()->borderRight();
     94    // FIXME: We are querying layout information but nothing guarantees that it's up-to-date, especially since we are called at style change.
     95    // FIXME: Querying the style's border information doesn't work on table cells with collapsing borders.
     96    int visibleSize = m_scrollbar->owningRenderer()->width() - m_scrollbar->owningRenderer()->style()->borderLeftWidth() - m_scrollbar->owningRenderer()->style()->borderRightWidth();
    9597    int w = calcScrollbarThicknessUsing(MainOrPreferredSize, style()->width(), visibleSize, renderView);
    9698    int minWidth = calcScrollbarThicknessUsing(MinSize, style()->minWidth(), visibleSize, renderView);
     
    108110        return;
    109111    RenderView* renderView = view();
    110     int visibleSize = m_scrollbar->owningRenderer()->height() -  m_scrollbar->owningRenderer()->borderTop() - m_scrollbar->owningRenderer()->borderBottom();
     112    // FIXME: We are querying layout information but nothing guarantees that it's up-to-date, especially since we are called at style change.
     113    // FIXME: Querying the style's border information doesn't work on table cells with collapsing borders.
     114    int visibleSize = m_scrollbar->owningRenderer()->height() -  m_scrollbar->owningRenderer()->style()->borderTopWidth() - m_scrollbar->owningRenderer()->style()->borderBottomWidth();
    111115    int h = calcScrollbarThicknessUsing(MainOrPreferredSize, style()->height(), visibleSize, renderView);
    112116    int minHeight = calcScrollbarThicknessUsing(MinSize, style()->minHeight(), visibleSize, renderView);
Note: See TracChangeset for help on using the changeset viewer.