Bug 76135 - [Chromium] REGRESSION(102301): DC leak introduced in WebScreenInfoFactory
Summary: [Chromium] REGRESSION(102301): DC leak introduced in WebScreenInfoFactory
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit API (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows 7
: P1 Major
Assignee: David Levin
URL:
Keywords:
Depends on: 76203 76227 76237
Blocks:
  Show dependency treegraph
 
Reported: 2012-01-11 17:53 PST by Mark Larson (Google)
Modified: 2012-01-18 21:29 PST (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
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.)