Changeset 75985 in webkit


Ignore:
Timestamp:
Jan 17, 2011 5:44:40 PM (13 years ago)
Author:
Adam Roben
Message:

Don't access the CACFLayerRef's sublayers directly from PlatformCALayer

There might be a secret extra sublayer (the tile parent layer) that
PlatformCALayer doesn't know about. When PlatformCALayer would
encounter this, it would try to use the tile parent layer's
PlatformCALayer wrapper, which was null, and then would crash. We now
ask PlatformCALayerWinInternal for the sublayer list, since that class
knows about the tile parent layer and can exclude it from the sublayer
list.

Covered by compositing/tiling/huge-layer-resize.html.

Fixes <​http://webkit.org/b/52597> Crash beneath
PlatformCALayer::adoptSublayers when switching out of tiling mode
(null-dereference of a PlatformCALayer)

Reviewed by Darin Adler and Chris Marrin.

LayoutTests:

Make compositing/tiling/huge-layer-resize.html faster, more reliable,
and more crashy (when there's a WebKit bug)

This test was trying to cause a layout/paint to happen by returning to
the event loop for a certain amount of time via setTimeout. But this
didn't always result in a layout/paint (at least on Windows). We now
force the layout/paint explicitly, which also lets us speed up the test
by removing the setTimeouts.

  • compositing/tiling/huge-layer-resize.html:

(testOnLoad): Changed to use recordLayerTree, which forces a
layout/paint, instead of hoping that setTimeout will do the trick.
(recordLayerTree): Forces a layout/paint, then dumps the layer tree.

Source/WebCore:

  • platform/graphics/ca/win/PlatformCALayerWin.cpp:

(PlatformCALayer::adoptSublayers):
(printLayer):
Changed to use PlatformCALayerWinInternal::getSublayers.

  • platform/graphics/ca/win/PlatformCALayerWinInternal.cpp:

(PlatformCALayerWinInternal::getSublayers): Added. Retrieves the list
of PlatformCALayers that represent our sublayers. Significantly, this
code knows about the tile parent layer and can thus exclude it.

  • platform/graphics/ca/win/PlatformCALayerWinInternal.h: Added

getSublayers.

Location:
trunk
Files:
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r75975 r75985  
     12011-01-17  Adam Roben  <aroben@apple.com>
     2
     3        Make compositing/tiling/huge-layer-resize.html faster, more reliable,
     4        and more crashy (when there's a WebKit bug)
     5
     6        This test was trying to cause a layout/paint to happen by returning to
     7        the event loop for a certain amount of time via setTimeout. But this
     8        didn't always result in a layout/paint (at least on Windows). We now
     9        force the layout/paint explicitly, which also lets us speed up the test
     10        by removing the setTimeouts.
     11
     12        Reviewed by Darin Adler and Chris Marrin.
     13
     14        Test for <http://webkit.org/b/52597> Crash beneath
     15        PlatformCALayer::adoptSublayers when switching out of tiling mode
     16        (null-dereference of a PlatformCALayer)
     17
     18        * compositing/tiling/huge-layer-resize.html:
     19        (testOnLoad): Changed to use recordLayerTree, which forces a
     20        layout/paint, instead of hoping that setTimeout will do the trick.
     21        (recordLayerTree): Forces a layout/paint, then dumps the layer tree.
     22
    1232011-01-17  Dan Bernstein  <mitz@apple.com>
    224

  • trunk/LayoutTests/compositing/tiling/huge-layer-resize.html

    r60004 r75985  
    2727    </style>
    2828    <script type="text/javascript" charset="utf-8">
    29         if (window.layoutTestController) {
     29        if (window.layoutTestController)
    3030            layoutTestController.dumpAsText();
    31             layoutTestController.waitUntilDone();
    32         }
    3331       
    34         result = "";
    35 
    3632        function testOnLoad()
    3733        {
    3834            // Small layer first
    39             window.setTimeout(function() {
    40                 if (window.layoutTestController)
    41                     result = "First (small layer):<br>" + layoutTestController.layerTreeAsText();
    42             }, 0);
    43            
     35            var result = recordLayerTree("First (small layer):<br>");
     36
    4437            // Huge layer second
    45             window.setTimeout(function() {
    46                 document.getElementById('container').style.height = "5000px";
    47                
    48                 // Let it render
    49                 window.setTimeout(function() {
    50                     if (window.layoutTestController)
    51                         result += "<br><br>Second (huge layer):<br>" + layoutTestController.layerTreeAsText();
    52                 }, 0);
    53             }, 100);
    54            
     38            document.getElementById('container').style.height = "5000px";
     39            result += recordLayerTree("<br><br>Second (huge layer):<br>");
     40
    5541            // Small layer third
    56             window.setTimeout(function() {
    57                 document.getElementById('container').style.height = "500px";
    58                
    59                 // Let it render
    60                 window.setTimeout(function() {
    61                     if (window.layoutTestController) {
    62                         result += "<br><br>Third (small layer):<br>" + layoutTestController.layerTreeAsText();
    63                         document.getElementById('layers').innerHTML = result;
    64                         layoutTestController.notifyDone();
    65                     }
    66                 }, 0);
    67             }, 200);
     42            document.getElementById('container').style.height = "500px";
     43            result += recordLayerTree("<br><br>Third (small layer):<br>");
     44
     45            document.getElementById('layers').innerHTML = result;
     46        }
     47
     48        function recordLayerTree(messagePrefix)
     49        {
     50            if (!window.layoutTestController)
     51                return "";
     52
     53            // Force a layout and a paint to make sure the compositing layers
     54            // have been updated.
     55            document.body.offsetLeft;
     56            layoutTestController.display();
     57
     58            return messagePrefix + layoutTestController.layerTreeAsText();
    6859        }
    6960     

  • trunk/Source/WebCore/ChangeLog

    r75982 r75985  
     12011-01-17  Adam Roben  <aroben@apple.com>
     2
     3        Don't access the CACFLayerRef's sublayers directly from PlatformCALayer
     4
     5        There might be a secret extra sublayer (the tile parent layer) that
     6        PlatformCALayer doesn't know about. When PlatformCALayer would
     7        encounter this, it would try to use the tile parent layer's
     8        PlatformCALayer wrapper, which was null, and then would crash. We now
     9        ask PlatformCALayerWinInternal for the sublayer list, since that class
     10        knows about the tile parent layer and can exclude it from the sublayer
     11        list.
     12
     13        Covered by compositing/tiling/huge-layer-resize.html.
     14
     15        Fixes <http://webkit.org/b/52597> Crash beneath
     16        PlatformCALayer::adoptSublayers when switching out of tiling mode
     17        (null-dereference of a PlatformCALayer)
     18
     19        Reviewed by Darin Adler and Chris Marrin.
     20
     21        * platform/graphics/ca/win/PlatformCALayerWin.cpp:
     22        (PlatformCALayer::adoptSublayers):
     23        (printLayer):
     24        Changed to use PlatformCALayerWinInternal::getSublayers.
     25
     26        * platform/graphics/ca/win/PlatformCALayerWinInternal.cpp:
     27        (PlatformCALayerWinInternal::getSublayers): Added. Retrieves the list
     28        of PlatformCALayers that represent our sublayers. Significantly, this
     29        code knows about the tile parent layer and can thus exclude it.
     30
     31        * platform/graphics/ca/win/PlatformCALayerWinInternal.h: Added
     32        getSublayers.
     33
    1342011-01-17  Naoki Takano  <takano.naoki@gmail.com>
    235

  • trunk/Source/WebCore/platform/graphics/ca/win/PlatformCALayerWin.cpp

    r75652 r75985  
    254254void PlatformCALayer::adoptSublayers(PlatformCALayer* source)
    255255{
    256     // Make a list of the sublayers from source
    257256    PlatformCALayerList sublayers;
    258     size_t n = source->sublayerCount();
    259     CFArrayRef sourceSublayers = CACFLayerGetSublayers(source->platformLayer());
    260 
    261     for (size_t i = 0; i < n; ++i) {
    262         CACFLayerRef layer = static_cast<CACFLayerRef>(const_cast<void*>(CFArrayGetValueAtIndex(sourceSublayers, i)));
    263         sublayers.append(platformCALayer(layer));
    264     }
     257    intern(source)->getSublayers(sublayers);
    265258
    266259    // Use setSublayers() because it properly nulls out the superlayer pointers.
    … …  
    691684        fprintf(stderr, "(sublayers\n");
    692685
    693         CFArrayRef sublayers = CACFLayerGetSublayers(layer->platformLayer());
    694         for (int i = 0; i < n; ++i) {
    695             PlatformCALayer* sublayer = PlatformCALayer::platformCALayer(const_cast<void*>(CFArrayGetValueAtIndex(sublayers, i)));
    696             printLayer(sublayer, indent + 2);
    697         }
     686        PlatformCALayerList sublayers;
     687        intern(layer)->getSublayers(sublayers);
     688        ASSERT(n == sublayers.size());
     689        for (int i = 0; i < n; ++i)
     690            printLayer(sublayers[i].get(), indent + 2);
    698691
    699692        printIndent(indent + 1);

  • trunk/Source/WebCore/platform/graphics/ca/win/PlatformCALayerWinInternal.cpp

    r75262 r75985  
    211211}
    212212
     213void PlatformCALayerWinInternal::getSublayers(PlatformCALayerList& list) const
     214{
     215    CFArrayRef sublayers = CACFLayerGetSublayers(owner()->platformLayer());
     216    if (!sublayers) {
     217        list.clear();
     218        return;
     219    }
     220
     221    size_t count = CFArrayGetCount(sublayers);
     222
     223    size_t layersToSkip = 0;
     224    if (owner()->layerType() == PlatformCALayer::LayerTypeWebTiledLayer) {
     225        // Exclude the tile parent layer.
     226        layersToSkip = 1;
     227    }
     228
     229    list.resize(count - layersToSkip);
     230    for (size_t arrayIndex = layersToSkip; arrayIndex < count; ++arrayIndex)
     231        list[arrayIndex - layersToSkip] = PlatformCALayer::platformCALayer(const_cast<void*>(CFArrayGetValueAtIndex(sublayers, arrayIndex)));
     232}
     233
    213234void PlatformCALayerWinInternal::removeAllSublayers()
    214235{

  • trunk/Source/WebCore/platform/graphics/ca/win/PlatformCALayerWinInternal.h

    r75262 r75985  
    5353
    5454    void setSublayers(const PlatformCALayerList&);
     55    void getSublayers(PlatformCALayerList&) const;
    5556    void removeAllSublayers();
    5657    void insertSublayer(PlatformCALayer*, size_t);
Note: See TracChangeset for help on using the changeset viewer.