Bug 209875 – Remove unused media controls code

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Eric Carlson]

Remove code for the, now unused, C++ based media controls.

[Radar WebKit Bug Importer]

<rdar://problem/61172738>

[Eric Carlson]

Created attachment 395197 [details]
Patch

[Eric Carlson]

Created attachment 395203 [details]
Patch

[Eric Carlson]

Created attachment 395212 [details]
Patch

[Eric Carlson]

Created attachment 395216 [details]
Patch

[Daniel Bates]

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.

[Darin Adler]

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

[Eric Carlson]

Created attachment 395281 [details]
Patch for landing

[Darin Adler]

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.

[Eric Carlson]

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.

[Eric Carlson]

Created attachment 395286 [details]
Patch for landing

[Eric Carlson]

Created attachment 395307 [details]
Patch for landing

[Eric Carlson]

Comment on attachment 395307 [details]
Patch for landing

The Windows test failures seem to be unrelated.

[EWS]

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

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