Bug 210570 - Captured ThreadedScrollingTree should check its m_scrollingCoordinator before calling its methods
Summary: Captured ThreadedScrollingTree should check its m_scrollingCoordinator before...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Scrolling (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Said Abou-Hallawa
URL:
Keywords: InRadar
: 210987 (view as bug list)
Depends on:
Blocks:
 
Reported: 2020-04-15 14:01 PDT by Said Abou-Hallawa
Modified: 2020-04-26 18:49 PDT (History)
9 users (show)

See Also:


Attachments
Patch (2.29 KB, patch)
2020-04-15 14:05 PDT, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Patch (1.97 KB, patch)
2020-04-15 20:40 PDT, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Said Abou-Hallawa 2020-04-15 14:01:12 PDT
Before <https://trac.webkit.org/changeset/258679>, m_scrollingCoordinator was protected. But this was changed in this revision such that "*this" is now protected. This change was done because we needed to call strongThis->removeWheelEventTestCompletionDeferralForReason() also.

In <https://trac.webkit.org/changeset/258753>, the call to strongThis->removeWheelEventTestCompletionDeferralForReason() was removed. But the protection capture code was not changed back.

Protecting "*this" does not guarantee the protection of m_scrollingCoordinator since it is a RefPtr. And this RefPtr can be nullified in ThreadedScrollingTree::invalidate().
Comment 1 Said Abou-Hallawa 2020-04-15 14:05:00 PDT
Created attachment 396570 [details]
Patch
Comment 2 Said Abou-Hallawa 2020-04-15 14:07:41 PDT
<rdar://problem/61691082>
Comment 3 Simon Fraser (smfr) 2020-04-15 14:57:15 PDT
Comment on attachment 396570 [details]
Patch

If we've cleared the m_scrollingCoordinator on the ThreadedScrolling tree, I don't think we need to keep the scrollingCoordinator alive in order to call scheduleUpdateScrollPositionAfterAsyncScroll. We should just null-check m_scrollingCoordinator in the block.
Comment 4 Said Abou-Hallawa 2020-04-15 20:40:14 PDT
Created attachment 396612 [details]
Patch
Comment 5 EWS 2020-04-16 11:04:31 PDT
Committed r260199: <https://trac.webkit.org/changeset/260199>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 396612 [details].
Comment 6 Alexey Proskuryakov 2020-04-26 18:49:10 PDT
*** Bug 210987 has been marked as a duplicate of this bug. ***