Bug 81187 - Platforms without USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR) don't need to query the page's displayID
Summary: Platforms without USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR) don't need to ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: James Robinson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-03-14 20:29 PDT by James Robinson
Modified: 2012-03-14 22:07 PDT (History)
2 users (show)

See Also:

Attachments
Patch (1.66 KB, patch)
2012-03-14 20:30 PDT, James Robinson 		simon.fraser: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description James Robinson 2012-03-14 20:29:30 PDT 
Platforms without USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR) don't need to query the page's displayID
Comment 1 James Robinson 2012-03-14 20:30:03 PDT 
Created attachment 131980 [details]
Patch
Comment 2 James Robinson 2012-03-14 20:37:36 PDT 
In r97405 Chris Marrin added a PlatformDisplayID parameter to ScriptedAnimationController::create(). This is only used if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR) is set, which is only set on mac and blackberry (?).  Unfortunately this code tries to grab the display ID from Document::page() whenever a callback is set, bug that's not correct - the page() on a Document might be NULL.  We have crash reports in chromium from this exact situation.  Having the display ID on a Document-owned object seems inherently a bit broken - Documents aren't always associated with a Page and can move between Pages.

I had a patch series underway in https://bugs.webkit.org/show_bug.cgi?id=74165 to move the controller up to the Page, which I think will make it clearer how to deal with issues like this.  I haven't had time to get around to this, but this patch avoids doing the unnecessary call (and resulting crash) on most platforms.
Comment 3 Simon Fraser (smfr) 2012-03-14 21:37:49 PDT 
Comment on attachment 131980 [details]
Patch

Can you please file a separate bug on page() possibly being null?
Comment 4 James Robinson 2012-03-14 22:00:40 PDT 
https://bugs.webkit.org/show_bug.cgi?id=81189
Comment 5 James Robinson 2012-03-14 22:07:39 PDT 
Committed r110821: <http://trac.webkit.org/changeset/110821>