Bug 76135

Summary: [Chromium] REGRESSION(102301): DC leak introduced in WebScreenInfoFactory
Product: WebKit Reporter: Mark Larson (Google) <mal>
Component: WebKit APIAssignee: David Levin <levin>
Severity: Major CC: fsamuel, levin
Priority: P1    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Windows 7   
Bug Depends on: 76203, 76227, 76237    
Bug Blocks:    

Description Mark Larson (Google) 2012-01-11 17:53:06 PST
See also http://bugs.chromium.org/109768

http://trac.webkit.org/changeset/102301 introduces a leak of GDI handles in Chromium.

In some cases, extensions that call screen.width/height repeatedly can exhaust GDI handles very quickly leading to a browser crash.

I think all you need to do is introduce a ReleaseDC(0, hdc) call before you return in /Source/WebKit/chromium/src/win/WebScreenInfoFactory.cpp
Comment 1 David Levin 2012-01-11 18:14:05 PST
I've got it. It is a simple fix. I'm working on a test for it right now.
Comment 2 Fady Samuel 2012-01-12 07:44:45 PST
(In reply to comment #1)
> I've got it. It is a simple fix. I'm working on a test for it right now.

Sorry, forgot the ReleaseDC (I don't do a lot of Windows development lately). I'm not sure how one tests memory leaks in WebKit however.
Comment 3 David Levin 2012-01-18 21:29:16 PST
Well I was going to introduce a test by calling the screen methods about 1000 or so times which is really fast in practice.

It turns out that the DRT doesn't ever call the problematic function, so I was going to fix that but that function is different on every platform so it is a larger problem.

Ideally the real screen functions would be called from the test to ensure there were no crashes, but I;m satisfied with having put in a check into check-webkit-style which will flag if anyone calls GetDC directly so it is unlikely that a leak of this type will ever be introduced again.

The depends on bugs are about getting rid of current instances of GetDC. (They technically are only related so I'm going to resolve this bug -- They are fixed and just pending review also.)