Changeset 134191 in webkit


Ignore:
Timestamp:
Nov 11, 2012, 11:09:44 PM (13 years ago)
Author:
rniwa@webkit.org
Message:

Multiple Layout Tests (e.g. fast/repaint/japanese-rl-selection-clear.html) is failing after r133840.
​https://bugs.webkit.org/show_bug.cgi?id=101547

Reviewed by Simon Fraser.

Source/WebCore:

I overlooked the fact when the selection is null, we still have to invalidate the caret rect that
previously existed. Revert the optimization added in r133840 to skip caret invalidation when new
selection is null, and add a special method to be called by FrameLoader prior to destruction instead.
This will let us avoid doing an extra layout upon destruction and not regress repaint tests.

Covered by existing tests.

  • editing/FrameSelection.cpp:

(WebCore::FrameSelection::setSelection): Added DoNotUpdateAppearance option.
(WebCore::FrameSelection::prepareForDestruction): Added.
(WebCore::FrameSelection::updateAppearance): Reverted the flawed optimization added in r133840.
Also, don't update style before updating selection unless text caret is disabled since we always
update the layout (including style) when text caret is enabled.

  • editing/FrameSelection.h:

(FrameSelection):

  • loader/FrameLoader.cpp:

(WebCore::FrameLoader::clear): Call prepareForDestruction instead of clear to avoid a layout.

LayoutTests:

Remove Chromium test expectations as these tests now pass.

  • platform/chromium/TestExpectations:
Location:
trunk
Files:
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r134190 r134191  
     12012-11-09  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Multiple Layout Tests (e.g. fast/repaint/japanese-rl-selection-clear.html) is failing after r133840.
     4        https://bugs.webkit.org/show_bug.cgi?id=101547
     5
     6        Reviewed by Simon Fraser.
     7
     8        Remove Chromium test expectations as these tests now pass.
     9
     10        * platform/chromium/TestExpectations:
     11
    1122012-11-11  Dongwoo Joshua Im  <dw.im@samsung.com>
    213

  • trunk/LayoutTests/platform/chromium/TestExpectations

    r134190 r134191  
    28852885webkit.org/b/65009 [ Mac ] scrollbars/scrollbar-drag-thumb-with-large-content.html [ Failure ]
    28862886
    2887 #webkit.org/b/65124 [ Android Linux Win ] fast/repaint/japanese-rl-selection-clear.html [ Failure ]
    2888 #webkit.org/b/65124 [ Win ] fast/repaint/japanese-rl-selection-repaint.html [ Failure ]
    2889 #webkit.org/b/65124 [ Android Linux Win ] fast/repaint/japanese-rl-selection-repaint-in-regions.html [ Failure ]
     2887webkit.org/b/65124 [ Android Linux Win ] fast/repaint/japanese-rl-selection-clear.html [ Failure ]
     2888webkit.org/b/65124 [ Win ] fast/repaint/japanese-rl-selection-repaint.html [ Failure ]
     2889webkit.org/b/65124 [ Android Linux Win ] fast/repaint/japanese-rl-selection-repaint-in-regions.html [ Failure ]
    28902890webkit.org/b/65124 [ Android Linux Win ] fast/repaint/line-flow-with-floats-in-regions.html [ Failure ]
    28912891
    … …  
    41904190
    41914191webkit.org/b/101618 [ Win7 Debug ] http/tests/inspector/indexeddb/database-data.html [ Crash ]
    4192 
    4193 # Failures following r133840:
    4194 webkit.org/b/101547 fast/repaint/4774354.html [ ImageOnlyFailure ]
    4195 webkit.org/b/101547 fast/repaint/caret-outside-block.html [ ImageOnlyFailure ]
    4196 webkit.org/b/101547 fast/repaint/delete-into-nested-block.html [ ImageOnlyFailure ]
    4197 webkit.org/b/101547 fast/repaint/inline-outline-repaint.html [ ImageOnlyFailure ]
    4198 # The following test has prior expectations which were disabled. Re-enable when removing this:
    4199 webkit.org/b/101547 fast/repaint/japanese-rl-selection-clear.html [ ImageOnlyFailure Failure ]
    4200 # The following test has prior expectations which were disabled. Re-enable when removing this:
    4201 webkit.org/b/101547 fast/repaint/japanese-rl-selection-repaint-in-regions.html [ ImageOnlyFailure Failure ]
    4202 # The following test has prior expectations which were disabled. Re-enable when removing this:
    4203 webkit.org/b/101547 fast/repaint/japanese-rl-selection-repaint.html [ ImageOnlyFailure Failure ]
    4204 webkit.org/b/101547 fast/repaint/no-caret-repaint-in-non-content-editable-element.html [ ImageOnlyFailure ]
    4205 webkit.org/b/101547 fast/repaint/repaint-across-writing-mode-boundary.html [ ImageOnlyFailure ]
    4206 webkit.org/b/101547 fast/repaint/selection-after-delete.html [ ImageOnlyFailure ]
    4207 webkit.org/b/101547 fast/repaint/selection-rl.html [ ImageOnlyFailure ]
    4208 # The following test has prior expectations which were disabled. Re-enable when removing this:
    4209 webkit.org/b/101547 svg/custom/foreignObject-crash-on-hover.xml [ ImageOnlyFailure ]
    4210 # The following test has prior expectations which were disabled. Re-enable when removing this:
    4211 webkit.org/b/101547 svg/custom/hit-test-unclosed-subpaths.svg [ ImageOnlyFailure ]
    4212 # The following test has prior expectations which were disabled. Re-enable when removing this:
    4213 webkit.org/b/101547 svg/custom/hit-test-with-br.xhtml [ ImageOnlyFailure ]
    4214 
    4215 webkit.org/b/101794 fast/canvas/canvas-as-image-hidpi.html [ ImageOnlyFailure ]
    4216 webkit.org/b/101794 platform/chromium/virtual/gpu/fast/canvas/canvas-as-image-hidpi.html [ ImageOnlyFailure ]
    4217 webkit.org/b/101794 fast/canvas/canvas-resize-reset-pixelRatio.html [ Failure ]
    4218 webkit.org/b/101794 platform/chromium/virtual/gpu/fast/canvas/canvas-resize-reset-pixelRatio.html [ Failure ]

  • trunk/Source/WebCore/ChangeLog

    r134190 r134191  
     12012-11-09  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Multiple Layout Tests (e.g. fast/repaint/japanese-rl-selection-clear.html) is failing after r133840.
     4        https://bugs.webkit.org/show_bug.cgi?id=101547
     5
     6        Reviewed by Simon Fraser.
     7
     8        I overlooked the fact when the selection is null, we still have to invalidate the caret rect that
     9        previously existed. Revert the optimization added in r133840 to skip caret invalidation when new
     10        selection is null, and add a special method to be called by FrameLoader prior to destruction instead.
     11        This will let us avoid doing an extra layout upon destruction and not regress repaint tests.
     12
     13        Covered by existing tests.
     14
     15        * editing/FrameSelection.cpp:
     16        (WebCore::FrameSelection::setSelection): Added DoNotUpdateAppearance option.
     17        (WebCore::FrameSelection::prepareForDestruction): Added.
     18        (WebCore::FrameSelection::updateAppearance): Reverted the flawed optimization added in r133840.
     19        Also, don't update style before updating selection unless text caret is disabled since we always
     20        update the layout (including style) when text caret is enabled.
     21        * editing/FrameSelection.h:
     22        (FrameSelection):
     23        * loader/FrameLoader.cpp:
     24        (WebCore::FrameLoader::clear): Call prepareForDestruction instead of clear to avoid a layout.
     25
    1262012-11-11  Dongwoo Joshua Im  <dw.im@samsung.com>
    227

  • trunk/Source/WebCore/editing/FrameSelection.cpp

    r133939 r134191  
    295295    if (!s.isNone() && !(options & DoNotSetFocus))
    296296        setFocusedNodeIfNeeded();
    297    
    298     updateAppearance();
     297
     298    if (!(options & DoNotUpdateAppearance))
     299        updateAppearance();
    299300
    300301    // Always clear the x position used for vertical arrow navigation.
    … …  
    11501151    m_granularity = CharacterGranularity;
    11511152    setSelection(VisibleSelection());
     1153}
     1154
     1155void FrameSelection::prepareForDestruction()
     1156{
     1157    m_granularity = CharacterGranularity;
     1158
     1159#if ENABLE(TEXT_CARET)
     1160    m_caretBlinkTimer.stop();
     1161#endif
     1162
     1163    RenderView* view = m_frame->contentRenderer();
     1164    if (view)
     1165        view->clearSelection();
     1166
     1167    setSelection(VisibleSelection(), CloseTyping | ClearTypingStyle | DoNotUpdateAppearance);
    11521168}
    11531169
    … …  
    17281744{
    17291745#if ENABLE(TEXT_CARET)
    1730     bool caretRectChangedOrCleared = false;
    1731     if (isNonOrphanedCaret(m_selection)) {
    1732         m_frame->document()->updateLayout();
    1733         caretRectChangedOrCleared = recomputeCaretRect();
    1734     } else
    1735         caretRectChangedOrCleared = true;
     1746    m_frame->document()->updateLayout();
     1747    bool caretRectChangedOrCleared = recomputeCaretRect();
    17361748
    17371749    bool caretBrowsing = m_frame->settings() && m_frame->settings()->caretBrowsingEnabled();
    … …  
    17541766        }
    17551767    }
    1756 #endif
    1757 
     1768#else
    17581769    // We need to update style in case the node containing the selection is made display:none.
    17591770    m_frame->document()->updateStyleIfNeeded();
     1771#endif
    17601772
    17611773    RenderView* view = m_frame->contentRenderer();

  • trunk/Source/WebCore/editing/FrameSelection.h

    r133939 r134191  
    124124        SpellCorrectionTriggered = 1 << 3,
    125125        DoNotSetFocus = 1 << 4,
    126         DictationTriggered = 1 << 5
     126        DictationTriggered = 1 << 5,
     127        DoNotUpdateAppearance = 1 << 6,
    127128    };
    128129    typedef unsigned SetSelectionOptions; // Union of values in SetSelectionOption and EUserTriggered
    … …  
    154155    void selectAll();
    155156    void clear();
    156    
     157    void prepareForDestruction();
     158
    157159    // Call this after doing user-triggered selections to make it easy to delete the frame you entirely selected.
    158160    void selectFrameElementInParentIfFullySelected();

  • trunk/Source/WebCore/loader/FrameLoader.cpp

    r133868 r134191  
    571571    }
    572572
    573     m_frame->selection()->clear();
     573    m_frame->selection()->prepareForDestruction();
    574574    m_frame->eventHandler()->clear();
    575575    if (clearFrameView && m_frame->view())
Note: See TracChangeset for help on using the changeset viewer.