horizontalOverscrollBehavior(), verticalOverscrollBehavior() and scrollbarWidthStyle() inside of LocalFrameView have an if (scrollingObject && renderView()) condition before accessing the respective style values. The renderView() check may be redundant. scrollbarWidthStyle() has it as it's a copy of the overscrollBehavior functions. They appear to have it as a left over from when they accessed a function inside of renderView() which was subsequently removed. Check if this is needed for any of these functions and remove as appropriate.
Do you want to expand scope of this bug? I think we can add more checks in 'addScrollableArea' and 'containsScrollableArea' to also assert to confirm we have 'ScrollableArea'. bool LocalFrameView::addScrollableArea(ScrollableArea* scrollableArea) { ASSERT(scrollableArea); _________ bool LocalFrameView::containsScrollableArea(ScrollableArea* scrollableArea) const { ASSERT(scrollableArea); if (!m_scrollableAreas || !scrollableArea) return false; ^ Kind of early return, if we don't have scrollableArea. _____ I think it would make scroll code bit more robust as well. ________ If not, I am happy to separate PR, above compiles in my local build. :-)
(In reply to Ahmad Saleem from comment #1) > Do you want to expand scope of this bug? > > I think we can add more checks in 'addScrollableArea' and > 'containsScrollableArea' to also assert to confirm we have 'ScrollableArea'. > > > bool LocalFrameView::addScrollableArea(ScrollableArea* scrollableArea) > { > ASSERT(scrollableArea); > > > _________ > > bool LocalFrameView::containsScrollableArea(ScrollableArea* scrollableArea) > const > { > > ASSERT(scrollableArea); > if (!m_scrollableAreas || !scrollableArea) > return false; > > ^ Kind of early return, if we don't have scrollableArea. > > _____ > > I think it would make scroll code bit more robust as well. > > ________ > > > If not, I am happy to separate PR, above compiles in my local build. :-) Although 'containsScrollableArea' is not used much as well. (If we can also explore, if we can remove it).
Could you do this as a separate PR please. Not quite sure if this bug is actionable currently just something Darin spotted during a code review. Your changes look reasonable with my limited understanding.
<rdar://problem/112331229>