Changeset 223914 in webkit


Ignore:
Timestamp:
Oct 24, 2017 1:11:35 PM (6 years ago)
Author:
commit-queue@webkit.org
Message:

Web Inspector: Layer mutations should be purely based on layerId, not based on nodeId
​https://bugs.webkit.org/show_bug.cgi?id=178554

Patch by Ross Kirsling <Ross Kirsling> on 2017-10-24
Reviewed by Devin Rousso.

Source/WebInspectorUI:

  • UserInterface/Controllers/LayerTreeManager.js:

(WI.LayerTreeManager.prototype.layerTreeMutations):
Looking for special cases involving nodeIds is incorrect, as nodeIds need not be unique in the layer list (such
as when an element and a pseudo-element thereof each give rise to a layer). A layer object marked "preserved" in
this way shares no data with its predecessor, meaning that no consumer can act upon this so-called preservation.
A preserved layer should be nothing more or less than a recycled layerId (the thing that *is* unique).

LayoutTests:

  • inspector/layers/layer-tree-manager-expected.txt:
  • inspector/layers/layer-tree-manager.html:
Location:
trunk
Files:
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r223913 r223914  
     12017-10-24  Ross Kirsling  <ross.kirsling@sony.com>
     2
     3        Web Inspector: Layer mutations should be purely based on layerId, not based on nodeId
     4        https://bugs.webkit.org/show_bug.cgi?id=178554
     5
     6        Reviewed by Devin Rousso.
     7
     8        * inspector/layers/layer-tree-manager-expected.txt:
     9        * inspector/layers/layer-tree-manager.html:
     10
    1112017-10-24  Adrian Perez de Castro  <aperez@igalia.com>
    212

  • trunk/LayoutTests/inspector/layers/layer-tree-manager-expected.txt

    r223428 r223914  
    77PASS: Array elements should be Layer instances.
    88
     9-- Running test case: LayerTreeManager.layerTreeMutations.overlappingNodeIds
     10PASS: Preserved layers should be correct.
     11PASS: Added layers should be correct.
     12PASS: Removed layers should be correct.
     13

  • trunk/LayoutTests/inspector/layers/layer-tree-manager.html

    r223428 r223914  
    1010    suite.addTestCase({
    1111        name: "LayerTreeManager.layersForNode.root",
    12         description: "Calling the LayerTreeManager#layersForNode method with the root node should the full layer list.",
     12        description: "Calling the LayerTreeManager#layersForNode method with the root node should return the full layer list.",
    1313        test(resolve, reject) {
    1414            WI.domTreeManager.requestDocument((node) => {
    … …  
    1919                });
    2020            });
     21        }
     22    });
     23
     24    suite.addTestCase({
     25        name: "LayerTreeManager.layerTreeMutations.overlappingNodeIds",
     26        description: "The LayerTreeManager#layerTreeMutations method should provide a diff purely based on layerIds, not based on nodeIds.",
     27        test(resolve, reject) {
     28            let previousLayers = [
     29                {layerId: "1", nodeId: 1},
     30                {layerId: "2", nodeId: 1, isGeneratedContent: true, pseudoElementId: "10"},
     31                {layerId: "3", nodeId: 1, isReflection: true},
     32                {layerId: "4", nodeId: 2}
     33            ];
     34
     35            let newLayers = [
     36                {layerId: "4", nodeId: 2},
     37                {layerId: "5", nodeId: 1},
     38                {layerId: "6", nodeId: 3}
     39            ];
     40
     41            let {preserved, additions, removals} = WI.layerTreeManager.layerTreeMutations(previousLayers, newLayers);
     42            InspectorTest.expectShallowEqual(preserved, previousLayers.slice(3), "Preserved layers should be correct.");
     43            InspectorTest.expectShallowEqual(additions, newLayers.slice(1), "Added layers should be correct.");
     44            InspectorTest.expectShallowEqual(removals, previousLayers.slice(0, 3), "Removed layers should be correct.");
     45            resolve();
    2146        }
    2247    });

  • trunk/Source/WebInspectorUI/ChangeLog

    r223867 r223914  
     12017-10-24  Ross Kirsling  <ross.kirsling@sony.com>
     2
     3        Web Inspector: Layer mutations should be purely based on layerId, not based on nodeId
     4        https://bugs.webkit.org/show_bug.cgi?id=178554
     5
     6        Reviewed by Devin Rousso.
     7
     8        * UserInterface/Controllers/LayerTreeManager.js:
     9        (WI.LayerTreeManager.prototype.layerTreeMutations):
     10        Looking for special cases involving nodeIds is incorrect, as nodeIds need not be unique in the layer list (such
     11        as when an element and a pseudo-element thereof each give rise to a layer). A layer object marked "preserved" in
     12        this way shares no data with its predecessor, meaning that no consumer can act upon this so-called preservation.
     13        A preserved layer should be nothing more or less than a recycled layerId (the thing that *is* unique).
     14
    1152017-10-23  Nikita Vasilyev  <nvasilyev@apple.com>
    216

  • trunk/Source/WebInspectorUI/UserInterface/Controllers/LayerTreeManager.js

    r223428 r223914  
    4747        console.assert(this.supported);
    4848
    49         if (isEmptyObject(previousLayers)) {
    50             return {
    51                 preserved: [],
    52                 additions: newLayers,
    53                 removals: []
    54             };
    55         }
     49        if (isEmptyObject(previousLayers))
     50            return {preserved: [], additions: newLayers, removals: []};
    5651
    57         function nodeIdForLayer(layer)
    58         {
    59             return layer.isGeneratedContent ? layer.pseudoElementId : layer.nodeId;
    60         }
     52        let previousLayerIds = new Set;
     53        let newLayerIds = new Set;
    6154
    62         var layerIdsInPreviousLayers = [];
    63         var nodeIdsInPreviousLayers = [];
    64         var nodeIdsForReflectionsInPreviousLayers = [];
     55        let preserved = [];
     56        let additions = [];
    6557
    66         previousLayers.forEach(function(layer) {
    67             layerIdsInPreviousLayers.push(layer.layerId);
     58        for (let layer of previousLayers)
     59            previousLayerIds.add(layer.layerId);
    6860
    69             var nodeId = nodeIdForLayer(layer);
    70             if (!nodeId)
    71                 return;
     61        for (let layer of newLayers) {
     62            newLayerIds.add(layer.layerId);
    7263
    73             if (layer.isReflection)
    74                 nodeIdsForReflectionsInPreviousLayers.push(nodeId);
    75             else
    76                 nodeIdsInPreviousLayers.push(nodeId);
    77         });
    78 
    79         var preserved = [];
    80         var additions = [];
    81 
    82         var layerIdsInNewLayers = [];
    83         var nodeIdsInNewLayers = [];
    84         var nodeIdsForReflectionsInNewLayers = [];
    85 
    86         newLayers.forEach(function(layer) {
    87             layerIdsInNewLayers.push(layer.layerId);
    88 
    89             var existed = layerIdsInPreviousLayers.includes(layer.layerId);
    90 
    91             var nodeId = nodeIdForLayer(layer);
    92             if (!nodeId)
    93                 return;
    94 
    95             if (layer.isReflection) {
    96                 nodeIdsForReflectionsInNewLayers.push(nodeId);
    97                 existed = existed || nodeIdsForReflectionsInPreviousLayers.includes(nodeId);
    98             } else {
    99                 nodeIdsInNewLayers.push(nodeId);
    100                 existed = existed || nodeIdsInPreviousLayers.includes(nodeId);
    101             }
    102 
    103             if (existed)
     64            if (previousLayerIds.has(layer.layerId))
    10465                preserved.push(layer);
    10566            else
    10667                additions.push(layer);
    107         });
     68        }
    10869
    109         var removals = previousLayers.filter(function(layer) {
    110             var nodeId = nodeIdForLayer(layer);
    111 
    112             if (layer.isReflection)
    113                 return !nodeIdsForReflectionsInNewLayers.includes(nodeId);
    114             else
    115                 return !nodeIdsInNewLayers.includes(nodeId) && !layerIdsInNewLayers.includes(layer.layerId);
    116         });
     70        let removals = previousLayers.filter((layer) => !newLayerIds.has(layer.layerId));
    11771
    11872        return {preserved, additions, removals};
Note: See TracChangeset for help on using the changeset viewer.