[Webkit-unassigned] [Bug 167201] Add Link Preload as an off-by-default experimental feature menu item.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=167201

Joseph Pecoraro <joepeck at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #299690|review?                     |review-
              Flags|                            |

--- Comment #5 from Joseph Pecoraro <joepeck at webkit.org> ---
Comment on attachment 299690
  --> https://bugs.webkit.org/attachment.cgi?id=299690
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=299690&action=review

Neat!

r- only because I think there is some additional cleanup you should do when switching to an experimental feature.

1. Remove the legacy way to enable the feature for tests (since test runners will auto-enable it now!)

    Source/WebCore/testing/InternalSettings.cpp
    187:    RuntimeEnabledFeatures::sharedFeatures().setLinkPreloadEnabled(m_linkPreloadEnabled);
    674:void InternalSettings::setLinkPreloadEnabled(bool enabled)
    676:    RuntimeEnabledFeatures::sharedFeatures().setLinkPreloadEnabled(enabled);

    Source/WebCore/testing/InternalSettings.h
    113:    static void setLinkPreloadEnabled(bool);
    191:        bool m_linkPreloadEnabled;

    Source/WebCore/testing/InternalSettings.idl
    86:    void setLinkPreloadEnabled(boolean enabled);

2. Remove uses of `internal.settings.setLinkPreloadEnabled` in tests:

    http/tests/fetch/redirectmode-and-preload.html
    http/tests/preload/delaying_onload_link_preload_after_discovery.html
    http/tests/preload/delaying_onload_link_preload_after_discovery_image.html
    http/tests/preload/download_resources.html
    http/tests/preload/download_resources_from_header_iframe.html
    http/tests/preload/download_resources_from_invalid_headers.html
    http/tests/preload/dynamic_adding_preload.html
    http/tests/preload/dynamic_remove_preload_href.html
    http/tests/preload/not_delaying_window_onload_before_discovery.html
    http/tests/preload/onerror_event.html
    http/tests/preload/onload_event.html
    http/tests/preload/resources/download_resources_from_header.php
    http/tests/preload/resources/invalid_resources_from_header.php
    http/tests/preload/single_download_preload_runner.html
    http/tests/security/cached-cross-origin-preloaded-css-stylesheet.html
    http/tests/security/cached-cross-origin-preloading-css-stylesheet.html

Some tests might need updates based on line numbers once you simplify.

3. Add to the experimental features test page a way to quick test the experimental features menu (can be tested in MiniBrowser).

    Websites/webkit.org/experimental-features.html

Hopefully there is a way to feature check this. Maybe checking for the attribute being null instead of undefined.

> Source/WebKit/mac/WebView/WebPreferencesPrivate.h:172
> +- (void)setLinkPreloadEnabled:(BOOL)flag;
> +- (BOOL)linkPreloadEnabled;

At the bottom we have been just using @property syntax. For example:

    @property (nonatomic) BOOL intersectionObserverEnabled;

> Source/WebKit/mac/WebView/WebView.mm:2901
> +    RuntimeEnabledFeatures::sharedFeatures().setLinkPreloadEnabled([preferences linkPreloadEnabled]);

This could be moved to dot syntax:

    preferences.linkPreloadEnabled

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20170125/116d87cb/attachment.html>