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
Don Olmstead
2017-05-31 15:23:44 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.
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.
Created attachment 311661 [details]
Patch
Fix style
Created attachment 311665 [details]
Patch
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.
Created attachment 311666 [details]
Patch
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 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. Is it possible to !REQUEST_ANIMATION_FRAME_TIMER code, if I'll keep it working? I've meant, to keep !REQUEST_ANIMATION_FRAME_TIMER code (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. 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 on attachment 311666 [details] Patch Clearing flags on attachment: 311666 Committed r217862: <http://trac.webkit.org/changeset/217862> All reviewed patches have been landed. Closing bug. |