Bug 60618 – WebView::performLayeredWindowUpdate() crashes with NULL pointer

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Heiner Wolf]

m_backingStoreBitmap can be NULL causing ->handle() to fail. This happens when I resize (quickly?) to (0,0). I am using a sizer-div and move it quickly to the topleft corner of the window and beyond. The sizer-div calls ::MoveWindow(). The crash does not always happen. Maybe it's a timing issue. If I resize slowly, then it does not crash. This lets me assume, that it only happens, when there comes a second ::MoveWindow(width=0,height=0) after sizing to (0,0). Maybe m_backingStoreBitmap is discarded for size (0,0). WebView::performLayeredWindowUpdate() should probably have a if(m_backingStoreBitmap!=NULL), anyway.

http://svn.webkit.org/repository/webkit/trunk
Revision: 85004

Method:
  void WebView::performLayeredWindowUpdate()
crashes when accessing
  m_backingStoreBitmap->handle()
with 
  m_backingStoreBitmap = 0


Stack trace:
>	WebKit_debug.dll!WebCore::RefCountedGDIHandle<HBITMAP__ *>::handle()  Line 51 + 0x3 bytes	C++
 	WebKit_debug.dll!WebView::performLayeredWindowUpdate()  Line 1001 + 0x12 bytes	C++
 	WebKit_debug.dll!WebView::WebViewWndProc(HWND__ * hWnd=0x000b0b78, unsigned int message=15, unsigned int wParam=0, long lParam=0)  Line 2130	C++
 	user32.dll!76bcc4e7() 	
 	[Frames below may be incorrect and/or missing, no symbols loaded for user32.dll]	
 	user32.dll!76bc5f9f() 	
 	user32.dll!76bcc590() 	
 	user32.dll!76bc4f0e() 	
 	user32.dll!76bc4f7d() 	
 	ntdll.dll!77816fee() 	
 	user32.dll!76bc4ec3() 	
 	user32.dll!76bc1be4() 	
 	user32.dll!76bbffcd() 	
 	WebKit_debug.dll!WebView::repaint(const WebCore::IntRect & windowRect={...}, bool contentChanged=false, bool immediate=true, bool repaintContentOnly=false)  Line 756 + 0xf bytes	C++
 	WebKit_debug.dll!WebChromeClient::invalidateWindow(const WebCore::IntRect & windowRect={...}, bool immediate=true)  Line 470	C++
 	WebKit_debug.dll!WebCore::Chrome::invalidateWindow(const WebCore::IntRect & updateRect={...}, bool immediate=true)  Line 71 + 0x20 bytes	C++
 	WebKit_debug.dll!WebCore::ScrollView::scrollContents(const WebCore::IntSize & scrollDelta={...})  Line 663 + 0x3f bytes	C++
 	WebKit_debug.dll!WebCore::ScrollView::scrollTo(const WebCore::IntSize & newOffset={...})  Line 366	C++
 	WebKit_debug.dll!WebCore::FrameView::scrollTo(const WebCore::IntSize & newOffset={...})  Line 2109	C++
 	WebKit_debug.dll!WebCore::ScrollView::setScrollOffset(const WebCore::IntPoint & offset={...})  Line 351 + 0x17 bytes	C++
 	WebKit_debug.dll!WebCore::ScrollableArea::setScrollOffsetFromAnimation(const WebCore::IntPoint & offset={...})  Line 133 + 0x13 bytes	C++
 	WebKit_debug.dll!WebCore::ScrollAnimator::notityPositionChanged()  Line 130	C++
 	WebKit_debug.dll!WebCore::ScrollAnimator::scrollToOffsetWithoutAnimation(const WebCore::FloatPoint & offset={...})  Line 80 + 0xf bytes	C++
 	WebKit_debug.dll!WebCore::ScrollableArea::scrollToOffsetWithoutAnimation(const WebCore::FloatPoint & offset={...})  Line 97 + 0x21 bytes	C++
 	WebKit_debug.dll!WebCore::ScrollView::updateScrollbars(const WebCore::IntSize & desiredOffset={...})  Line 592	C++
 	WebKit_debug.dll!WebCore::ScrollView::setScrollPosition(const WebCore::IntPoint & scrollPoint={...})  Line 402	C++
 	WebKit_debug.dll!WebCore::FrameView::setScrollPosition(const WebCore::IntPoint & scrollPoint={...})  Line 1457	C++
 	WebKit_debug.dll!WebCore::RenderLayer::scrollRectToVisible(const WebCore::IntRect & rect={...}, bool scrollToAnchor=false, const WebCore::ScrollAlignment & alignX={...}, const WebCore::ScrollAlignment & alignY={...})  Line 1482	C++
 	WebKit_debug.dll!WebCore::RenderLayer::autoscroll()  Line 1586	C++
 	WebKit_debug.dll!WebCore::RenderBox::autoscroll()  Line 660	C++
 	WebKit_debug.dll!WebCore::EventHandler::autoscrollTimerFired(WebCore::Timer<WebCore::EventHandler> * __formal=0x032a42b0)  Line 798 + 0x21 bytes	C++
 	WebKit_debug.dll!WebCore::Timer<WebCore::EventHandler>::fired()  Line 100 + 0x23 bytes	C++
 	WebKit_debug.dll!WebCore::ThreadTimers::sharedTimerFiredInternal()  Line 112 + 0xf bytes	C++
 	WebKit_debug.dll!WebCore::ThreadTimers::sharedTimerFired()  Line 91	C++
 	WebKit_debug.dll!WebCore::TimerWindowWndProc(HWND__ * hWnd=0x00040a4c, unsigned int message=49797, unsigned int wParam=0, long lParam=0)  Line 103 + 0x8 bytes	C++

[Adam Roben (:aroben)]

Heiner, is this happening in an application that is using layered window mode? If so, which application?

[Brent Fulgham]

Hi Adam,

(In reply to comment #1)
> Heiner, is this happening in an application that is using layered window mode? If so, which application?

I asked Heiner to file this bug.  It's happening in an application he's working on, and I wanted to open an issue to track a correction for this.

[Adam Roben (:aroben)]

Sounds good. I just wanted to make sure we weren't going down the layered window path by accident.

[Heiner Wolf]

I am working on my own application using WebKit.dll, etc. It is based on Brent's recent transparency patch. The transparency code is very new, has not seen may users, so it's no surprise, that some edge cases come up.

[Brent Fulgham]

Created attachment 103124 [details]
Patch

[Adam Roben (:aroben)]

Comment on attachment 103124 [details]
Patch

When do we expect this case to get hit? It would be worth mentioning it in the ChangeLog and/or a comment.

[Brent Fulgham]

(In reply to comment #6)
> (From update of attachment 103124 [details])
> When do we expect this case to get hit? It would be worth mentioning it in the ChangeLog and/or a comment.

The 'ensureBackingStore' method will not produce a backing store in cases where the window rect is zero height or zero width.  I think Heiner ran into this during certain resize operations; it may be that a paint message is getting processed after the window rect has collapsed to zero height (or width).

Based on his earlier comments (2011-05-11 03:26:14), it seems to trigger when the window is sized to (0,0) (which we know destroys the backing store), followed by a MoveWindow, which must be generating a paint.

It might be that this could be addressed by avoiding the second paint message getting to the 'performLayeredWindowUpdate', but it seemed easiest to just check for NULL backing store and bail out early if necessary.

Do you think deeper analysis of the drawing logic is warranted here?

[Adam Roben (:aroben)]

(In reply to comment #7)
> (In reply to comment #6)
> > (From update of attachment 103124 [details] [details])
> > When do we expect this case to get hit? It would be worth mentioning it in the ChangeLog and/or a comment.
> 
> The 'ensureBackingStore' method will not produce a backing store in cases where the window rect is zero height or zero width.  I think Heiner ran into this during certain resize operations; it may be that a paint message is getting processed after the window rect has collapsed to zero height (or width).
> 
> Based on his earlier comments (2011-05-11 03:26:14), it seems to trigger when the window is sized to (0,0) (which we know destroys the backing store), followed by a MoveWindow, which must be generating a paint.

Makes sense.

> It might be that this could be addressed by avoiding the second paint message getting to the 'performLayeredWindowUpdate', but it seemed easiest to just check for NULL backing store and bail out early if necessary.
> 
> Do you think deeper analysis of the drawing logic is warranted here?

I suggest we do as little work as possible when the window is 0-sized. The null-check seems fine.

[Brent Fulgham]

Committed r92621: <http://trac.webkit.org/changeset/92621>