Bug 172780 – [WebCore] Enable REQUEST_ANIMATION_FRAME_TIMER for all ports

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Don Olmstead]

All ports have USE_REQUEST_ANIMATION_FRAME_TIMER on. Remove all conditional compilation around it.

[Don Olmstead]

Created attachment 311659 [details]
Patch

Removing all references to USE(REQUEST_ANIMATION_FRAME_TIMER).

In Platform.h there's a conditional !USE(WINGDI) for its setting but no one is building with WINGDI and it should probably be removed in the future.

[Build Bot]

Attachment 311659 [details] did not pass style-queue:


ERROR: Source/WebCore/page/Chrome.h:88:  Missing space inside { }.  [whitespace/braces] [5]
Total errors found: 1 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Don Olmstead]

Created attachment 311661 [details]
Patch

Fix style

[Don Olmstead]

Created attachment 311665 [details]
Patch

[Build Bot]

Attachment 311665 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Don Olmstead]

Created attachment 311666 [details]
Patch

[Don Olmstead]

Comment on attachment 311666 [details]
Patch

Think this is ready to go. Only question I'm not sure of is if scheduleAnimation should just be removed from HostWindow/Chrome.

[Alex Christensen]

Comment on attachment 311666 [details]
Patch

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

> Source/WebCore/page/Chrome.h:88
> -    void scheduleAnimation() override;
> +    void scheduleAnimation() override { }

Yeah, this seems no longer needed, so let's just remove it.  Maybe in a followup patch if that requires more surgery.
And if it is needed and can be final instead of just override, let's do that.

[Konstantin Tokarev]

Is it possible to !REQUEST_ANIMATION_FRAME_TIMER code, if I'll keep it working?

[Konstantin Tokarev]

I've meant, to keep !REQUEST_ANIMATION_FRAME_TIMER code

[Don Olmstead]

(In reply to Konstantin Tokarev from comment #10)
> I've meant, to keep !REQUEST_ANIMATION_FRAME_TIMER code

Currently the !USE(REQUEST_ANIMATION_FRAME_TIMER) path does not compile. I found this out when compiling out the PS4 on trunk which did not have it set.

The only case where USE_REQUEST_ANIMATION_FRAME_TIMER was not turned on was if USE(WINGDI) was set. No one is using WINGDI and that should likely be removed as well. That is why I made the patch to remove it.

As we've been going through WebCore we've been encountering and fixing any of these issues. If someone wants to fix the !USE(REQUEST_ANIMATION_FRAME_TIMER) path that's fine. This just looks to remove a bunch of unused code and maybe more.

I don't know enough about what Qt WebKit requires so I'm not sure why this would be a problem for you.

[Alex Christensen]

Is there any fundamental problem preventing USE_REQUEST_ANIMATION_FRAME_TIMER on Qt?  We would like to drop as many of these legacy !USE_COMMONLY_IMPLEMENTED_THING code paths as possible.

[WebKit Commit Bot]

Comment on attachment 311666 [details]
Patch

Clearing flags on attachment: 311666

Committed r217862: <http://trac.webkit.org/changeset/217862>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.