<rdar://problem/61172738>

Created attachment 395197 [details]
Patch

Created attachment 395203 [details]
Patch

Created attachment 395212 [details]
Patch

Created attachment 395216 [details]
Patch

Comment on attachment 395216 [details]
Patch

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

Patch looks good. Check Windows.

> Source/WebCore/Modules/mediacontrols/MediaControlsHost.cpp:163
> +        return m_textTrackContainer.get();

Ok as is. No change needed. Looks like line is indented.

> Source/WebCore/html/HTMLMediaElement.cpp:4300
> +    if (m_creatingControls)

This is ok as-is. No change needed. Is this change concealing a bug revealed by this removal? If so, why is it ok to make this change? If not, proceed.

> Source/WebCore/html/HTMLMediaElement.cpp:4303
>      m_creatingControls = true;

Ok as-is. No change needed. In the future the optimal solution is to use SETforScope() class for this ivar toggling.

rendering/RenderThemeWin.cpp(846,30): error C3861: 'adjustMediaSliderThumbSize': identifier not found

Created attachment 395281 [details]
Patch for landing

Comment on attachment 395281 [details]
Patch for landing

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

> Source/WebCore/Modules/mediacontrols/MediaControlsHost.cpp:163
> +        return m_textTrackContainer.get();

Indentation mistake here.

Comment on attachment 395216 [details]
Patch

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

>> Source/WebCore/html/HTMLMediaElement.cpp:4300
>> +    if (m_creatingControls)
> 
> This is ok as-is. No change needed. Is this change concealing a bug revealed by this removal? If so, why is it ok to make this change? If not, proceed.

ensureUserAgentShadowRoot creates the media controls in the shadow DOM which can trigger scripts in the page and cause this to be called again recursively. In that case we don't need to call ensureUserAgentShadowRoot again.

>> Source/WebCore/html/HTMLMediaElement.cpp:4303
>>      m_creatingControls = true;
> 
> Ok as-is. No change needed. In the future the optimal solution is to use SETforScope() class for this ivar toggling.

I should have thought of that! I'll make that change in another patch, thanks.

Created attachment 395286 [details]
Patch for landing

Created attachment 395307 [details]
Patch for landing

Comment on attachment 395307 [details]
Patch for landing

The Windows test failures seem to be unrelated.

Committed r259435: <https://trac.webkit.org/changeset/259435>

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