[webkit-reviews] review denied: [Bug 186393] Crash under Page::scrollingCoordinator() : [Attachment 342156] Patch

[bugzilla-daemon at webkit.org]

zalan <zalan at apple.com> has denied Antoine Quint <graouts at apple.com>'s request
for review:
Bug 186393: Crash under Page::scrollingCoordinator()
https://bugs.webkit.org/show_bug.cgi?id=186393

Attachment 342156: Patch

https://bugs.webkit.org/attachment.cgi?id=342156&action=review




--- Comment #4 from zalan <zalan at apple.com> ---
Comment on attachment 342156
  --> https://bugs.webkit.org/attachment.cgi?id=342156
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=342156&action=review

> Source/WebCore/page/Page.cpp:377
> +    if (!m_settings || !m_settings->scrollingCoordinatorEnabled())
> +	   return nullptr;

m_settings can't really be nullptr and as long as the Page::settings() returns
Settings& we should not add random nullptr checks (since the contract is that
as long as the page is valid, the settings is valid too )
Not sure way we would crash with m_scrollingCoordinator nullptr. If we end up
not constructing a scrollingCoordinator object, m_scrollingCoordinator.get()
would just return nullptr.
Also I think this stacktrace is about accessing an invalid Page object.