Bug 238776 – [PGO] We should be able to build WebKit to collect PGO profiles easily

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Justin Michaud]

Let's make it possible for us to build WebKit with LLVM's pgo instrumentation with one flag.

[Justin Michaud]

Created attachment 456963 [details]
Patch

[Justin Michaud]

Created attachment 456965 [details]
Patch

[Justin Michaud]

Created attachment 456967 [details]
Patch

[Saam Barati]

Comment on attachment 456967 [details]
Patch

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

> Source/WTF/ChangeLog:9
> +        Then, follow the included directions to collect your raw PGO profiles!

where are the included directions?

> Source/WTF/wtf/GenerateProfiles.h:41
> +inline void registerProfileGenerationCallback(const char* name)

I think you want ALWAYS_INLINE here, since this code really needs to live in the dylib it's called from to get proper data?

> Source/WebKit/WebProcess/WebPage/WebPage.cpp:579
> +    WTF::registerProfileGenerationCallback<WebPage>("WebKit");

nit: "WebContentProcess" to be consistent w/ your other calls?

[Justin Michaud]

Comment on attachment 456967 [details]
Patch

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

>> Source/WTF/wtf/GenerateProfiles.h:41
>> +inline void registerProfileGenerationCallback(const char* name)
> 
> I think you want ALWAYS_INLINE here, since this code really needs to live in the dylib it's called from to get proper data?

Nice catch

>> Source/WebKit/WebProcess/WebPage/WebPage.cpp:579
>> +    WTF::registerProfileGenerationCallback<WebPage>("WebKit");
> 
> nit: "WebContentProcess" to be consistent w/ your other calls?

Hmm. *Process files contain symbols from multiple frameworks, but this should only be including symbols from WebKit.framework. Actually, I guess this would include symbols from every dylib we link against? I'll have to do some research on this one.

[Elliott Williams]

Comment on attachment 456967 [details]
Patch

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

> Source/JavaScriptCore/Configurations/JavaScriptCore.xcconfig:56
> +GENERATE_PROFILE_FLAGS_ON = -fprofile-instr-generate -DENABLE_LLVM_PROFILE_GENERATION=1;
> +USE_PROFILE_FLAGS_ON_GENERATE_OFF = -fprofile-instr-use=$(BUILT_PRODUCTS_DIR)/DerivedSources/JavaScriptCore/JavaScriptCore.profdata;
> +USE_PROFILE_FLAGS_ON_GENERATE_ = $(USE_PROFILE_FLAGS_ON_GENERATE_OFF)
> +
> +GENERATE_PROFILE_FLAGS = $(GENERATE_PROFILE_FLAGS_$(ENABLE_LLVM_PROFILE_GENERATION));
> +USE_PROFILE_FLAGS_ON = $(USE_PROFILE_FLAGS_ON_GENERATE_$(ENABLE_LLVM_PROFILE_GENERATION));
> +
>  PROFILE_DATA_FLAGS = $(PROFILE_DATA_FLAGS_$(CONFIGURATION)_$(WK_PLATFORM_NAME));
>  PROFILE_DATA_FLAGS_Release_macosx = $(PROFILE_DATA_FLAGS_Release_macosx$(WK_MACOS_1200));
> -PROFILE_DATA_FLAGS_Release_macosx_MACOS_SINCE_1200 = $(PROFILE_DATA_FLAGS_ENABLED);
> -PROFILE_DATA_FLAGS_Release_iphoneos = $(PROFILE_DATA_FLAGS_ENABLED);
> +PROFILE_DATA_FLAGS_Release_macosx_MACOS_SINCE_1200 = $(USE_PROFILE_FLAGS_ON);
> +PROFILE_DATA_FLAGS_Release_iphoneos = $(USE_PROFILE_FLAGS_ON);
>  PROFILE_DATA_FLAGS_Production_macosx = $(PROFILE_DATA_FLAGS_Production_macosx$(WK_MACOS_1200));
> -PROFILE_DATA_FLAGS_Production_macosx_MACOS_SINCE_1200 = $(PROFILE_DATA_FLAGS_ENABLED);
> -PROFILE_DATA_FLAGS_Production_iphoneos = $(PROFILE_DATA_FLAGS_ENABLED);
> -PROFILE_DATA_FLAGS_ENABLED = -fprofile-instr-use=$(BUILT_PRODUCTS_DIR)/DerivedSources/JavaScriptCore/JavaScriptCore.profdata;
> +PROFILE_DATA_FLAGS_Production_macosx_MACOS_SINCE_1200 = $(USE_PROFILE_FLAGS_ON);
> +PROFILE_DATA_FLAGS_Production_iphoneos = $(USE_PROFILE_FLAGS_ON);

I think this can be made simpler. How about:

    PROFILE_DATA_FLAGS_GENERATE_YES = -fprofile-instr-generate -DENABLE_LLVM_PROFILE_GENERATION=1;
    PROFILE_DATA_FLAGS_GENERATE_ = -fprofile-instr-use=$(BUILT_PRODUCTS_DIR)/DerivedSources/JavaScriptCore/JavaScriptCore.profdata;

Then, change the expansions from `$(PROFILE_DATA_FLAGS_ENABLED)` to `$($(PROFILE_DATA_FLAGS_GENERATE_$(ENABLE_LLVM_PROFILE_GENERATION))`.

> Tools/Scripts/webkitdirs.pm:1060
> +    appendToEnvironmentVariableList("ENABLE_LLVM_PROFILE_GENERATION", "ON") if defined $ENV{'ENABLE_LLVM_PROFILE_GENERATION'};

Are you sure this is necessary? Make is passing ENABLE_LLVM_PROFILE_GENERATION=YES to xcodebuild, so anything executed by Xcode will have the environment variable already set. Unless I'm missing something, this appears to be setting ENABLE_LLVM_PROFILE_GENERATION=ON when ENABLE_LLVM_PROFILE_GENERATION is already set.

> Tools/Scripts/webkitperl/FeatureList.pm:361
> +      { option => "llvm-profile-generation", desc => "Include LLVM's instrumentation to generate profiles for PGO",
> +      define => "ENABLE_LLVM_PROFILE_GENERATION", value => \$llvmProfileGenerationSupport },
> +

nit: indentation

> Makefile.shared:5
> +ifneq (,$(ENABLE_LLVM_PROFILE_GENERATION))
> +		export ENABLE_LLVM_PROFILE_GENERATION=ON
> +endif

If we can remove the line in webkitdirs.pm, then we can remove this too, and only pass the setting in XCODE_OPTIONS.

> Makefile.shared:9
> +ifneq (,$(ENABLE_LLVM_PROFILE_GENERATION))
> +		XCODE_OPTIONS += ENABLE_LLVM_PROFILE_GENERATION=ON
> +endif

nit: Using ifneq here means I could type `make ENABLE_LLVM_PROFILE_GENERATION=NO` and it would do the opposite and pass ENABLE_LLVM_PROFILE_GENERATION=ON. Can this be `ifeq ($(ENABLE_LLVM_PROFILE_GENERATION),YES)`?

[Justin Michaud]

Comment on attachment 456967 [details]
Patch

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

>> Source/JavaScriptCore/Configurations/JavaScriptCore.xcconfig:56
>> +PROFILE_DATA_FLAGS_Production_iphoneos = $(USE_PROFILE_FLAGS_ON);
> 
> I think this can be made simpler. How about:
> 
>     PROFILE_DATA_FLAGS_GENERATE_YES = -fprofile-instr-generate -DENABLE_LLVM_PROFILE_GENERATION=1;
>     PROFILE_DATA_FLAGS_GENERATE_ = -fprofile-instr-use=$(BUILT_PRODUCTS_DIR)/DerivedSources/JavaScriptCore/JavaScriptCore.profdata;
> 
> Then, change the expansions from `$(PROFILE_DATA_FLAGS_ENABLED)` to `$($(PROFILE_DATA_FLAGS_GENERATE_$(ENABLE_LLVM_PROFILE_GENERATION))`.

LDFlags are only used for generation, so there will always be a certain amount of duplication here. I have tried to make it simpler.

>> Tools/Scripts/webkitdirs.pm:1060
>> +    appendToEnvironmentVariableList("ENABLE_LLVM_PROFILE_GENERATION", "ON") if defined $ENV{'ENABLE_LLVM_PROFILE_GENERATION'};
> 
> Are you sure this is necessary? Make is passing ENABLE_LLVM_PROFILE_GENERATION=YES to xcodebuild, so anything executed by Xcode will have the environment variable already set. Unless I'm missing something, this appears to be setting ENABLE_LLVM_PROFILE_GENERATION=ON when ENABLE_LLVM_PROFILE_GENERATION is already set.

This is needed for check-for-weak-vtables-and-externals

[Justin Michaud]

Created attachment 457133 [details]
Patch

[Saam Barati]

Comment on attachment 457133 [details]
Patch

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

> Source/JavaScriptCore/Configurations/JavaScriptCore.xcconfig:43
> +PROFILE_DATA_FLAGS_ENABLED = -fprofile-instr-use=$(BUILT_PRODUCTS_DIR)/DerivedSources/JavaScriptCore/JavaScriptCore.profdata;

why move this?

> Source/JavaScriptCore/Configurations/JavaScriptCore.xcconfig:55
> +PROFILE_GENERATE_OR_USE_CFLAGS_GENERATE_ = $(PROFILE_GENERATE_OR_USE_CFLAGS_GENERATE_)

Does this work? Assigning to yourself like this? Did you mean to have $(PROFILE_GENERATE_OR_USE_CFLAGS_GENERATE_OFF) on the RHS?

> Source/WebCore/Configurations/WebCoreTestSupport.xcconfig:66
> +OTHER_CFLAGS = $(inherited) // Workaround for rdar://91365594 (Clang hangs when building WebKit WebCoreTestSupport with PGO profile generation enabled)

Don't understand why we need to explicitly do this? Is it just a form of documentation?

> Source/WebCore/page/Frame.cpp:175
> +    WTF::registerProfileGenerationCallback<Frame>("WebCore");

Maybe ScriptController::initializeMainThread is the better place for this?

> Source/WebKit/WebProcess/WebPage/WebPage.cpp:579
> +    WTF::registerProfileGenerationCallback<WebPage>("WebKit");

Put this in InitializeWebKit2?

[Elliott Williams]

Comment on attachment 457133 [details]
Patch

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

> Source/JavaScriptCore/Configurations/JavaScriptCore.xcconfig:61
> +PROFILE_GENERATE_OR_USE_LDFLAGS_GENERATE_OFF = ;
> +PROFILE_GENERATE_OR_USE_LDFLAGS_GENERATE_ON = -fprofile-instr-generate -DENABLE_LLVM_PROFILE_GENERATION=1;
> +PROFILE_GENERATE_OR_USE_LDFLAGS_GENERATE_ = $(PROFILE_GENERATE_OR_USE_LDFLAGS_GENERATE_OFF)
> +PROFILE_GENERATE_OR_USE_LDFLAGS = $(PROFILE_GENERATE_OR_USE_LDFLAGS_GENERATE_$(ENABLE_LLVM_PROFILE_GENERATION));

No need to define empty build settings. You can just do:

PROFILE_GENERATE_OR_USE_LDFLAGS_GENERATE_ON = -fprofile-instr-generate -DENABLE_LLVM_PROFILE_GENERATION=1;
PROFILE_GENERATE_OR_USE_LDFLAGS = $(PROFILE_GENERATE_OR_USE_LDFLAGS_GENERATE_$(ENABLE_LLVM_PROFILE_GENERATION));

> Source/WebKit/Configurations/BaseXPCService.xcconfig:44
> +OTHER_CFLAGS = $(OTHER_CFLAGS_BASETARGET);

It might be easier to follow this if you keep all the OTHER_CFLAGS settings in BaseTarget.xcconfig, and set

    PROFILE_GENERATE_OR_USE_CFLAGS = ;

in any target which we intend to disable PGO in. Then you don't have this back-and-forth where OTHER_CFLAGS is getting evaluated using multiple files.

> Tools/Scripts/webkitdirs.pm:1060
> +    appendToEnvironmentVariableList("ENABLE_LLVM_PROFILE_GENERATION", $ENV{'ENABLE_LLVM_PROFILE_GENERATION'}) if defined $ENV{'ENABLE_LLVM_PROFILE_GENERATION'};

Still confused about this. Xcode exports the target's build settings to any script phases, and AFAICT check-for-weak-vtables-and-externals is only run as a script phase. So we shouldn't need to manually define it here.

The reason why the existing WEBKIT_COVERAGE_BUILD variable is set here is because it's _not_ set in xcconfigs; it's derived from the active coverage settings.

> Tools/Scripts/webkitperl/FeatureList.pm:361
> +    { option => "llvm-profile-generation", desc => "Include LLVM's instrumentation to generate profiles for PGO",
> +      define => "ENABLE_LLVM_PROFILE_GENERATION", value => \$llvmProfileGenerationSupport },
> +

Here's something annoying: Feature flags appear in Xcode build settings as defined as their own name, _not_ as YES or NO. i.e. if I run `build-webkit --llvm-profile-generation`, this build setting is passed to Xcode:

    ENABLE_LLVM_PROFILE_GENERATION=ENABLE_LLVM_PROFILE_GENERATION

So you need to handle that case in the xcconfig expansions. Perhaps we should have Make match this behavior and transform YES into ENABLE_LLVM_PROFILE_GENERATION. Then, in xcconfigs, do something like:

    PROFILE_GENERATE_OR_USE_CFLAGS_ENABLE_LLVM_PROFILE_GENERATION = -fprofile-instr-generate -DENABLE_LLVM_PROFILE_GENERATION=1;
    PROFILE_GENERATE_OR_USE_CFLAGS_ = $(PROFILE_DATA_FLAGS);
    PROFILE_GENERATE_OR_USE_CFLAGS = $(PROFILE_GENERATE_OR_USE_CFLAGS_$(ENABLE_LLVM_PROFILE_GENERATION));

[Justin Michaud]

(In reply to Saam Barati from comment #9)
> Comment on attachment 457133 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=457133&action=review
> 
> > Source/JavaScriptCore/Configurations/JavaScriptCore.xcconfig:43
> > +PROFILE_DATA_FLAGS_ENABLED = -fprofile-instr-use=$(BUILT_PRODUCTS_DIR)/DerivedSources/JavaScriptCore/JavaScriptCore.profdata;
> 
> why move this?
This makes the copy pasta block after easier to copy & paste.

> 
> > Source/JavaScriptCore/Configurations/JavaScriptCore.xcconfig:55
> > +PROFILE_GENERATE_OR_USE_CFLAGS_GENERATE_ = $(PROFILE_GENERATE_OR_USE_CFLAGS_GENERATE_)
> 
> Does this work? Assigning to yourself like this? Did you mean to have
> $(PROFILE_GENERATE_OR_USE_CFLAGS_GENERATE_OFF) on the RHS?

Copy pasta error

> 
> > Source/WebCore/Configurations/WebCoreTestSupport.xcconfig:66
> > +OTHER_CFLAGS = $(inherited) // Workaround for rdar://91365594 (Clang hangs when building WebKit WebCoreTestSupport with PGO profile generation enabled)
> 
> Don't understand why we need to explicitly do this? Is it just a form of
> documentation?
We need to overwrite the included definition, which enables PGO. Elliott's solution was better.

[Justin Michaud]

Created attachment 457290 [details]
Patch

[Justin Michaud]

Created attachment 457292 [details]
Patch

[Elliott Williams]

Comment on attachment 457292 [details]
Patch

Looks great! r=me

[Radar WebKit Bug Importer]

<rdar://problem/91595425>

[Saam Barati]

Comment on attachment 457292 [details]
Patch

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

> Source/JavaScriptCore/Configurations/JavaScriptCore.xcconfig:57
> +PROFILE_GENERATE_OR_USE_LDFLAGS = $(PROFILE_GENERATE_OR_USE_LDFLAGS_$(ENABLE_LLVM_PROFILE_GENERATION));

So the idea is we're run with the variable "ENABLE_LLVM_PROFILE_GENERATION" equal to the string "ENABLE_LLVM_PROFILE_GENERATION"? Why not just make it "YES"/"NO", like over strings?

> Source/WTF/wtf/GenerateProfiles.h:40
> +template<typename OnceFlagDiversifier>

I don't understand this template parameter.

> Source/WebCore/bindings/js/ScriptController.cpp:90
> +    WTF::registerProfileGenerationCallback<Frame>("WebCore");

I'd put this at the end, not the beginning, so we don't need to worry about what we can use in this function.

Also, "Frame"?

[Saam Barati]

Comment on attachment 457133 [details]
Patch

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

>> Tools/Scripts/webkitperl/FeatureList.pm:361
>> +
> 
> Here's something annoying: Feature flags appear in Xcode build settings as defined as their own name, _not_ as YES or NO. i.e. if I run `build-webkit --llvm-profile-generation`, this build setting is passed to Xcode:
> 
>     ENABLE_LLVM_PROFILE_GENERATION=ENABLE_LLVM_PROFILE_GENERATION
> 
> So you need to handle that case in the xcconfig expansions. Perhaps we should have Make match this behavior and transform YES into ENABLE_LLVM_PROFILE_GENERATION. Then, in xcconfigs, do something like:
> 
>     PROFILE_GENERATE_OR_USE_CFLAGS_ENABLE_LLVM_PROFILE_GENERATION = -fprofile-instr-generate -DENABLE_LLVM_PROFILE_GENERATION=1;
>     PROFILE_GENERATE_OR_USE_CFLAGS_ = $(PROFILE_DATA_FLAGS);
>     PROFILE_GENERATE_OR_USE_CFLAGS = $(PROFILE_GENERATE_OR_USE_CFLAGS_$(ENABLE_LLVM_PROFILE_GENERATION));

Can we just transform ENABLE_LLVM_PROFILE_GENERATION=ENABLE_LLVM_PROFILE_GENERATION into ENABLE_LLVM_PROFILE_GENERATION=ON?
Reasoning being:
PROFILE_GENERATE_OR_USE_CFLAGS_ON is much easier to read than PROFILE_GENERATE_OR_USE_CFLAGS_ENABLE_LLVM_PROFILE_GENERATION

[Saam Barati]

Comment on attachment 457133 [details]
Patch

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

>>> Tools/Scripts/webkitperl/FeatureList.pm:361
>>> +
>> 
>> Here's something annoying: Feature flags appear in Xcode build settings as defined as their own name, _not_ as YES or NO. i.e. if I run `build-webkit --llvm-profile-generation`, this build setting is passed to Xcode:
>> 
>>     ENABLE_LLVM_PROFILE_GENERATION=ENABLE_LLVM_PROFILE_GENERATION
>> 
>> So you need to handle that case in the xcconfig expansions. Perhaps we should have Make match this behavior and transform YES into ENABLE_LLVM_PROFILE_GENERATION. Then, in xcconfigs, do something like:
>> 
>>     PROFILE_GENERATE_OR_USE_CFLAGS_ENABLE_LLVM_PROFILE_GENERATION = -fprofile-instr-generate -DENABLE_LLVM_PROFILE_GENERATION=1;
>>     PROFILE_GENERATE_OR_USE_CFLAGS_ = $(PROFILE_DATA_FLAGS);
>>     PROFILE_GENERATE_OR_USE_CFLAGS = $(PROFILE_GENERATE_OR_USE_CFLAGS_$(ENABLE_LLVM_PROFILE_GENERATION));
> 
> Can we just transform ENABLE_LLVM_PROFILE_GENERATION=ENABLE_LLVM_PROFILE_GENERATION into ENABLE_LLVM_PROFILE_GENERATION=ON?
> Reasoning being:
> PROFILE_GENERATE_OR_USE_CFLAGS_ON is much easier to read than PROFILE_GENERATE_OR_USE_CFLAGS_ENABLE_LLVM_PROFILE_GENERATION

Actually, I take this back, this is actually easier to read, just a bit verbose.

But I think the reason it's a bit weird to read is that the variable name contains an OR in it. Maybe we could define one rule by running NOT_ on another. Probably there is some way to define the rules that don't rely on the OR semantics in the variable, but maybe it's not worth bike shedding over.

[Saam Barati]

Comment on attachment 457292 [details]
Patch

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

> Source/JavaScriptCore/runtime/VM.cpp:245
> +    WTF::registerProfileGenerationCallback<VM>("JavaScriptCore");

I'd put this in initializeThreading

> Source/WebKit/NetworkProcess/mac/NetworkProcessMac.mm:55
> +    WTF::registerProfileGenerationCallback<NetworkProcess>("NetworkProcess");

Why are these needed if we're in InitializeWebKit2?

[Justin Michaud]

(In reply to Saam Barati from comment #16)
> Comment on attachment 457292 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=457292&action=review
> 
> > Source/JavaScriptCore/Configurations/JavaScriptCore.xcconfig:57
> > +PROFILE_GENERATE_OR_USE_LDFLAGS = $(PROFILE_GENERATE_OR_USE_LDFLAGS_$(ENABLE_LLVM_PROFILE_GENERATION));
> 
> So the idea is we're run with the variable "ENABLE_LLVM_PROFILE_GENERATION"
> equal to the string "ENABLE_LLVM_PROFILE_GENERATION"? Why not just make it
> "YES"/"NO", like over strings?

Elliott mentioned this above. We don't get a choice here, Xcode picked this convention. The command line still uses ON.

> 
> > Source/WTF/wtf/GenerateProfiles.h:40
> > +template<typename OnceFlagDiversifier>
> 
> I don't understand this template parameter.
> 
> > Source/WebCore/bindings/js/ScriptController.cpp:90
> > +    WTF::registerProfileGenerationCallback<Frame>("WebCore");
> 
> I'd put this at the end, not the beginning, so we don't need to worry about
> what we can use in this function.
> 
> Also, "Frame"?

This is spectacularly wrong, it is in the Windows port. The cocoa version is obj c, so I am searching for a different place to put this.

> > Source/WebKit/NetworkProcess/mac/NetworkProcessMac.mm:55
> > +    WTF::registerProfileGenerationCallback<NetworkProcess>("NetworkProcess");
> Why are these needed if we're in InitializeWebKit2?

See ^

------

Finding a place for the WebKit profiling call is spectacularly important, and I will try to move the JSC one too. After that I think it should be ready to land for real 100% no whammies.

[Justin Michaud]

Created attachment 457488 [details]
Patch

[Wenson Hsieh]

Comment on attachment 457488 [details]
Patch

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

> Source/WebCore/bindings/js/ScriptController.cpp:95
> +    WTF::registerProfileGenerationCallback<Frame>("WebCore");

Is there a reason why we use existing classes here (and in JSC), but use a named enum type in WebKit2?

[Saam Barati]

Comment on attachment 457488 [details]
Patch

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

> Source/WTF/wtf/GenerateProfiles.h:43
> +template<typename OnceFlagDiversifier>

Why do we need this template parameter?

[Justin Michaud]

(In reply to Wenson Hsieh from comment #22)
> Comment on attachment 457488 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=457488&action=review
> 
> > Source/WebCore/bindings/js/ScriptController.cpp:95
> > +    WTF::registerProfileGenerationCallback<Frame>("WebCore");
> 
> Is there a reason why we use existing classes here (and in JSC), but use a
> named enum type in WebKit2?

(In reply to Saam Barati from comment #23)
> Comment on attachment 457488 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=457488&action=review
> 
> > Source/WTF/wtf/GenerateProfiles.h:43
> > +template<typename OnceFlagDiversifier>
> 
> Why do we need this template parameter?

We just need some unique name to make the static once_flag different for each dylib. Anything will work.

[Justin Michaud]

Created attachment 457571 [details]
Patch

[EWS]

Committed r292870 (249641@main): <https://commits.webkit.org/249641@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 457571 [details].