Bug 172780

Summary: [WebCore] Enable REQUEST_ANIMATION_FRAME_TIMER for all ports
Product: WebKit Reporter: Don Olmstead <don.olmstead>
Component: WebCore Misc.Assignee: Don Olmstead <don.olmstead>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, annulen, buildbot, commit-queue, dino, simon.fraser
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Description Don Olmstead 2017-05-31 15:23:44 PDT
All ports have USE_REQUEST_ANIMATION_FRAME_TIMER on. Remove all conditional compilation around it.
Comment 1 Don Olmstead 2017-05-31 17:00:28 PDT
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.
Comment 2 Build Bot 2017-05-31 17:03:23 PDT
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.
Comment 3 Don Olmstead 2017-05-31 17:09:02 PDT
Created attachment 311661 [details]
Patch

Fix style
Comment 4 Don Olmstead 2017-05-31 17:39:00 PDT
Created attachment 311665 [details]
Patch
Comment 5 Build Bot 2017-05-31 17:40:11 PDT
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.
Comment 6 Don Olmstead 2017-05-31 17:41:48 PDT
Created attachment 311666 [details]
Patch
Comment 7 Don Olmstead 2017-05-31 18:47:52 PDT
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.
Comment 8 Alex Christensen 2017-06-01 09:34:15 PDT
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.
Comment 9 Konstantin Tokarev 2017-06-01 16:09:05 PDT
Is it possible to !REQUEST_ANIMATION_FRAME_TIMER code, if I'll keep it working?
Comment 10 Konstantin Tokarev 2017-06-01 16:09:51 PDT
I've meant, to keep !REQUEST_ANIMATION_FRAME_TIMER code
Comment 11 Don Olmstead 2017-06-03 19:00:49 PDT
(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.
Comment 12 Alex Christensen 2017-06-05 13:20:04 PDT
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.
Comment 13 WebKit Commit Bot 2017-06-06 15:29:48 PDT
Comment on attachment 311666 [details]
Patch

Clearing flags on attachment: 311666

Committed r217862: <http://trac.webkit.org/changeset/217862>
Comment 14 WebKit Commit Bot 2017-06-06 15:29:50 PDT
All reviewed patches have been landed.  Closing bug.