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 ...
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: James Robinson
Depends on:
Reported: 2012-03-14 20:29 PDT by James Robinson
Modified: 2012-03-14 22:07 PDT (History)
2 users (show)

See Also:

Patch (1.66 KB, patch)
2012-03-14 20:30 PDT, James Robinson
simon.fraser: review+
Details | Formatted Diff | Diff

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]
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]

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