Bug 69040 - ScrollbarThemeComposite requires a ScrollView to draw scroll corner
Summary: ScrollbarThemeComposite requires a ScrollView to draw scroll corner
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alexey Proskuryakov
Depends on:
Reported: 2011-09-28 16:53 PDT by Alexey Proskuryakov
Modified: 2011-09-29 12:39 PDT (History)
2 users (show)

See Also:

proposed fix (8.04 KB, patch)
2011-09-28 17:05 PDT, Alexey Proskuryakov
simon.fraser: review+
Details | Formatted Diff | Diff
patch with Qt fix (8.49 KB, patch)
2011-09-29 11:01 PDT, Alexey Proskuryakov
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2011-09-28 16:53:40 PDT
Not every ScrollableArea is a ScrollView, much less a FrameView.

Talking to Page from platform code is obviously a layering violation, but I'm going to take the easy way out and just skip over failing code when there is no ScrollView.
Comment 1 Alexey Proskuryakov 2011-09-28 17:05:33 PDT
Created attachment 109095 [details]
proposed fix
Comment 2 Simon Fraser (smfr) 2011-09-28 17:08:05 PDT
Comment on attachment 109095 [details]
proposed fix

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

> Source/WebCore/platform/qt/ScrollbarThemeQt.cpp:248
> +    // FIXME: It's incorrect to do invalidation while painting.
>      if (context->updatingControlTints()) {
>         scrollView->invalidateRect(rect);

It's actually OK during the "updatingControlTints" paint phase. This is a special paint whose sole purpose is to invalidate things whose appearance changes based on the window's activation state.
Comment 3 Alexey Proskuryakov 2011-09-28 22:46:54 PDT
I need to make sure that Qt doesn't crash with null scrollView here.

Normally, it's something a caller does (e.g. in FrameView).
Comment 4 Alexey Proskuryakov 2011-09-29 11:01:37 PDT
Created attachment 109177 [details]
patch with Qt fix
Comment 5 Alexey Proskuryakov 2011-09-29 11:16:01 PDT
Comment on attachment 109177 [details]
patch with Qt fix

If this breaks something in Qt, platform/qt/ScrollbarThemeQt.cpp change alone can be reverted for now. But I'll need a good deal of detail about failures to fix (what calls ScrollbarThemeQt::paintScrollCorner() bypassing cross-platform code?)
Comment 6 WebKit Review Bot 2011-09-29 12:39:41 PDT
Comment on attachment 109177 [details]
patch with Qt fix

Clearing flags on attachment: 109177

Committed r96348: <http://trac.webkit.org/changeset/96348>
Comment 7 WebKit Review Bot 2011-09-29 12:39:46 PDT
All reviewed patches have been landed.  Closing bug.