Changeset 216931 in webkit


Ignore:
Timestamp:
May 16, 2017 8:08:44 AM (7 years ago)
Author:
Claudio Saavedra
Message:

[GTK] Tests that always pass when run alone, but fail in the bots
​https://bugs.webkit.org/show_bug.cgi?id=168572

Reviewed by Michael Catanzaro.

PlatformWebView::viewSupportsOptions() is basically a comparison
that checks that the passed options are the ones supported by the
web view. There is no reason for them to be implemented for each
platform differently. In fact doing so causes issues each time a
new option is added, if the corresponding platform implementation
is not updated accordingly.

A consequence of not updating the viewSupportOptions()
implementations when new options are added is that tests that need
these options might fail if they are executed after a test that
didn't need the option, as the webview will be reused even if the
option is not supported. This cannot be spotted when running the
tests individually. See bug #165133 for other example of the same
problem.

Remove the platform implementations and make the comparison a
method of the TestOptions structure, so that the check is in one
Tools:

place. For the time being, only include in the comparison the
options that were checked in the mac platform, which seem to be
the only ones relevant this far (unless newer ones have also been
forgotten).

  • WebKitTestRunner/PlatformWebView.h:

(WTR::PlatformWebView::viewSupportsOptions): Use the method
defined below.

  • WebKitTestRunner/TestOptions.h:

(WTR::TestOptions::hasSameInitializationOptions): Added.

  • WebKitTestRunner/gtk/PlatformWebViewGtk.cpp:

(WTR::PlatformWebView::viewSupportsOptions): Deleted.

  • WebKitTestRunner/ios/PlatformWebViewIOS.mm:

(WTR::PlatformWebView::viewSupportsOptions): Deleted.

  • WebKitTestRunner/mac/PlatformWebViewMac.mm:

(WTR::PlatformWebView::viewSupportsOptions): Deleted.

  • WebKitTestRunner/wpe/PlatformWebViewWPE.cpp:

(WTR::PlatformWebView::viewSupportsOptions): Deleted.

LayoutTests:

place. For the time being include in the comparison the options
checked in the mac and ios platforms, which seem to be the only
ones relevant this far (unless newer ones have also been
forgotten).

  • platform/gtk/TestExpectations: Unskip an affected test.
Location:
trunk
Files:
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r216926 r216931  
     12017-05-16  Claudio Saavedra  <csaavedra@igalia.com>
     2
     3        [GTK] Tests that always pass when run alone, but fail in the bots
     4        https://bugs.webkit.org/show_bug.cgi?id=168572
     5
     6        Reviewed by Michael Catanzaro.
     7
     8        PlatformWebView::viewSupportsOptions() is basically a comparison
     9        that checks that the passed options are the ones supported by the
     10        web view. There is no reason for them to be implemented for each
     11        platform differently. In fact doing so causes issues each time a
     12        new option is added, if the corresponding platform implementation
     13        is not updated accordingly.
     14
     15        A consequence of not updating the viewSupportOptions()
     16        implementations when new options are added is that tests that need
     17        these options might fail if they are executed after a test that
     18        didn't need the option, as the webview will be reused even if the
     19        option is not supported. This cannot be spotted when running the
     20        tests individually. See bug #165133 for other example of the same
     21        problem.
     22
     23        Remove the platform implementations and make the comparison a
     24        method of the TestOptions structure, so that the check is in one
     25        place. For the time being include in the comparison the options
     26        checked in the mac and ios platforms, which seem to be the only
     27        ones relevant this far (unless newer ones have also been
     28        forgotten).
     29
     30        * platform/gtk/TestExpectations: Unskip an affected test.
     31
    1322017-05-16  Romain Bellessort  <romain.bellessort@crf.canon.fr>
    233

  • trunk/LayoutTests/platform/gtk/TestExpectations

    r216919 r216931  
    34503450webkit.org/b/168557 imported/blink/compositing/draws-content/webgl-simple-background.html [ ImageOnlyFailure ]
    34513451
    3452 webkit.org/b/168572 intersection-observer/intersection-observer-entry-interface.html [ Failure ]
    3453 webkit.org/b/168572 js/dom/regress-157246.html [ Failure ]
    3454 
    34553452webkit.org/b/168719 fast/css/paint-order-shadow.html [ ImageOnlyFailure ]
    34563453

  • trunk/Tools/ChangeLog

    r216914 r216931  
     12017-05-16  Claudio Saavedra  <csaavedra@igalia.com>
     2
     3        [GTK] Tests that always pass when run alone, but fail in the bots
     4        https://bugs.webkit.org/show_bug.cgi?id=168572
     5
     6        Reviewed by Michael Catanzaro.
     7
     8        PlatformWebView::viewSupportsOptions() is basically a comparison
     9        that checks that the passed options are the ones supported by the
     10        web view. There is no reason for them to be implemented for each
     11        platform differently. In fact doing so causes issues each time a
     12        new option is added, if the corresponding platform implementation
     13        is not updated accordingly.
     14
     15        A consequence of not updating the viewSupportOptions()
     16        implementations when new options are added is that tests that need
     17        these options might fail if they are executed after a test that
     18        didn't need the option, as the webview will be reused even if the
     19        option is not supported. This cannot be spotted when running the
     20        tests individually. See bug #165133 for other example of the same
     21        problem.
     22
     23        Remove the platform implementations and make the comparison a
     24        method of the TestOptions structure, so that the check is in one
     25        place. For the time being, only include in the comparison the
     26        options that were checked in the mac platform, which seem to be
     27        the only ones relevant this far (unless newer ones have also been
     28        forgotten).
     29
     30        * WebKitTestRunner/PlatformWebView.h:
     31        (WTR::PlatformWebView::viewSupportsOptions): Use the method
     32        defined below.
     33        * WebKitTestRunner/TestOptions.h:
     34        (WTR::TestOptions::hasSameInitializationOptions): Added.
     35        * WebKitTestRunner/gtk/PlatformWebViewGtk.cpp:
     36        (WTR::PlatformWebView::viewSupportsOptions): Deleted.
     37        * WebKitTestRunner/ios/PlatformWebViewIOS.mm:
     38        (WTR::PlatformWebView::viewSupportsOptions): Deleted.
     39        * WebKitTestRunner/mac/PlatformWebViewMac.mm:
     40        (WTR::PlatformWebView::viewSupportsOptions): Deleted.
     41        * WebKitTestRunner/wpe/PlatformWebViewWPE.cpp:
     42        (WTR::PlatformWebView::viewSupportsOptions): Deleted.
     43
    1442017-05-15  Yusuke Suzuki  <utatane.tea@gmail.com>
    245

  • trunk/Tools/WebKitTestRunner/PlatformWebView.h

    r216497 r216931  
    9696    void addToWindow();
    9797
    98     bool viewSupportsOptions(const TestOptions&) const;
     98    bool viewSupportsOptions(const TestOptions& options) const { return m_options.hasSameInitializationOptions(options); }
    9999
    100100    PlatformImage windowSnapshotImage();

  • trunk/Tools/WebKitTestRunner/TestOptions.h

    r213676 r216931  
    5353   
    5454    TestOptions(const std::string& pathOrURL);
     55
     56    // Add here options that can only be set upon PlatformWebView
     57    // initialization and make sure it's up to date when adding new
     58    // options to this struct. Otherwise, tests using those options
     59    // might fail if WTR is reusing an existing PlatformWebView.
     60    bool hasSameInitializationOptions(const TestOptions& options) const
     61    {
     62        if (useThreadedScrolling != options.useThreadedScrolling
     63            || overrideLanguages != options.overrideLanguages
     64            || useMockScrollbars != options.useMockScrollbars
     65            || needsSiteSpecificQuirks != options.needsSiteSpecificQuirks
     66            || useCharacterSelectionGranularity != options.useCharacterSelectionGranularity
     67            || enableIntersectionObserver != options.enableIntersectionObserver
     68            || enableModernMediaControls != options.enableModernMediaControls
     69            || enablePointerLock != options.enablePointerLock
     70            || enableCredentialManagement != options.enableCredentialManagement)
     71            return false;
     72
     73        return true;
     74    }
    5575};
    5676

  • trunk/Tools/WebKitTestRunner/gtk/PlatformWebViewGtk.cpp

    r215176 r216931  
    154154}
    155155
    156 bool PlatformWebView::viewSupportsOptions(const TestOptions&) const
    157 {
    158     return true;
    159 }
    160 
    161156void PlatformWebView::dismissAllPopupMenus()
    162157{

  • trunk/Tools/WebKitTestRunner/ios/PlatformWebViewIOS.mm

    r216462 r216931  
    355355}
    356356
    357 bool PlatformWebView::viewSupportsOptions(const TestOptions& options) const
    358 {
    359     if (m_options.overrideLanguages != options.overrideLanguages
    360         || m_options.needsSiteSpecificQuirks != options.needsSiteSpecificQuirks
    361         || m_options.useCharacterSelectionGranularity != options.useCharacterSelectionGranularity
    362         || m_options.enableIntersectionObserver != options.enableIntersectionObserver
    363         || m_options.enableModernMediaControls != options.enableModernMediaControls
    364         || m_options.enablePointerLock != options.enablePointerLock
    365         || m_options.enableCredentialManagement != options.enableCredentialManagement)
    366         return false;
    367 
    368     return true;
    369 }
    370 
    371357void PlatformWebView::setNavigationGesturesEnabled(bool enabled)
    372358{

  • trunk/Tools/WebKitTestRunner/mac/PlatformWebViewMac.mm

    r215176 r216931  
    283283}
    284284
    285 bool PlatformWebView::viewSupportsOptions(const TestOptions& options) const
    286 {
    287     if (m_options.useThreadedScrolling != options.useThreadedScrolling
    288         || m_options.overrideLanguages != options.overrideLanguages
    289         || m_options.useMockScrollbars != options.useMockScrollbars
    290         || m_options.needsSiteSpecificQuirks != options.needsSiteSpecificQuirks
    291         || m_options.enableIntersectionObserver != options.enableIntersectionObserver
    292         || m_options.enableModernMediaControls != options.enableModernMediaControls
    293         || m_options.enablePointerLock != options.enablePointerLock
    294         || m_options.enableCredentialManagement != options.enableCredentialManagement)
    295         return false;
    296 
    297     return true;
    298 }
    299 
    300285void PlatformWebView::changeWindowScaleIfNeeded(float newScale)
    301286{

  • trunk/Tools/WebKitTestRunner/wpe/PlatformWebViewWPE.cpp

    r216497 r216931  
    129129}
    130130
    131 bool PlatformWebView::viewSupportsOptions(const TestOptions&) const
    132 {
    133     return true;
    134 }
    135 
    136131void PlatformWebView::forceWindowFramesChanged()
    137132{
Note: See TracChangeset for help on using the changeset viewer.