33164 2010-01-04 11:47:08 -0800 [WinCairo] Improper Cairo Surface Type Sometimes Used 2010-01-04 12:29:55 -0800 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) PC Windows XP RESOLVED FIXED P2 Normal --- 1 bfulgham bfulgham webkit.review.bot oldest_to_newest 176781 0 bfulgham 2010-01-04 11:47:08 -0800 I tracked down the cause of an annoying ASSERTION hit during some WebKit operations when running under the WinCairo port. WebKit would assert when checking that the target bitsPerPixel was 32. This was caused by two cases: (1) Sometimes a device context to a 24-bits-per-pixel context was being used (for example, when printing). (2) Sometimes a new device context was passed that did not have a corresponding (existing) bitmap that could be selected from 'GetCurrentObject'. In the second case, the bitmap structure returned by GetObject was filled with garbage, causing the assertion and possibly causing construction of extremely large image surfaces (as the info.bmWidth/bmHeight field were filled in with uninitialized data). This patch corrects both problems by confirming that a valid bitmap handle is returned by GetCurrentObject, and falling back to the default cairo_win32_surface_create method, which constructs a standard GDI-compatible 24-bits-per-pixel surface from a raw device context handle. We only want to pay the price of a 32-bit cairo surface for rendering when its needed, otherwise we take no advantage of native GDI drawing operations inside Cairo. 176784 1 45811 bfulgham 2010-01-04 11:53:57 -0800 Created attachment 45811 Check bitmap validity, only construct 32-bit surface when needed. 176785 2 webkit.review.bot 2010-01-04 11:56:15 -0800 style-queue ran check-webkit-style on attachment 45811 without any errors. 176792 3 45811 aroben 2010-01-04 12:08:13 -0800 Comment on attachment 45811 Check bitmap validity, only construct 32-bit surface when needed. > + if (!GetObject(bitmap, sizeof(info), &info)) > + surface = cairo_win32_surface_create(hdc); Does this surface have an associated bitmap? Do we need to select it into the DC? Please add a link to this bug in the ChangeLog. r=me 176803 4 bfulgham 2010-01-04 12:24:47 -0800 (In reply to comment #3) > (From update of attachment 45811 [details]) > > + if (!GetObject(bitmap, sizeof(info), &info)) > > + surface = cairo_win32_surface_create(hdc); > > Does this surface have an associated bitmap? Do we need to select it into the > DC? > > Please add a link to this bug in the ChangeLog. > > r=me No, the created surface references whatever is currently in the DC. The only reason for the second path is to create a 32-bit image bitmap backing store so that alpha effects can be handled properly. 176805 5 bfulgham 2010-01-04 12:29:40 -0800 Landed in http://trac.webkit.org/changeset/52752. 45811 2010-01-04 11:53:57 -0800 2010-01-04 12:08:13 -0800 Check bitmap validity, only construct 32-bit surface when needed. context_fix.patch text/plain 2329 bfulgham SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA1Mjc1MSkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTUgQEAKKzIwMTAtMDEtMDQgIEJyZW50IEZ1bGdoYW0gIDxiZnVsZ2hhbUB3ZWJr aXQub3JnPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAg IENvcnJlY3QgZGVidWcgYXNzZXJ0aW9uIChhbmQgcG9zc2libGUgcnVudGltZSBlcnJvcnMpIGJ5 CisgICAgICAgIGNoZWNraW5nIHZhbGlkaXR5IG9mIGJpdG1hcCBzZXR0aW5ncyB1c2VkIHRvIGNy ZWF0ZQorICAgICAgICB0aGUgV2luZG93cyBDYWlybyBjb250ZXh0cy4KKworICAgICAgICAqIHBs YXRmb3JtL2dyYXBoaWNzL3dpbi9HcmFwaGljc0NvbnRleHRDYWlyb1dpbi5jcHA6CisgICAgICAg IChXZWJDb3JlOjpjcmVhdGVDYWlyb0NvbnRleHRXaXRoSERDKTogQ2hlY2sgdmFsaWRpdHkgb2Yg c2VsZWN0ZWQKKyAgICAgICAgICBPQkpfQklUTUFQIHJldHJpZXZlZCwgYW5kIG9ubHkgY3JlYXRl IDMyLWJpdCBjb250ZXh0IHdoZW4KKyAgICAgICAgICBhIHZhbGlkIGJpdG1hcCBpcyBwcm92aWRl ZC4KIDIwMTAtMDEtMDQgIERtaXRyeSBUaXRvdiAgPGRpbWljaEBjaHJvbWl1bS5vcmc+CiAKICAg ICAgICAgTm90IHJldmlld2VkLCByZXZlcnQgcjUyNzQ1IGFuZCByNTI3NDYgb24gYmVoYWxmIG9m IE5pa29sYXMgWmltbWVybWFubiwgYXMgZGlzY3Vzc2VkIG9uIElSQy4KSW5kZXg6IFdlYkNvcmUv cGxhdGZvcm0vZ3JhcGhpY3Mvd2luL0dyYXBoaWNzQ29udGV4dENhaXJvV2luLmNwcAo9PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09Ci0tLSBXZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL3dpbi9HcmFwaGljc0NvbnRleHRDYWly b1dpbi5jcHAJKHJldmlzaW9uIDUyNjg1KQorKysgV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy93 aW4vR3JhcGhpY3NDb250ZXh0Q2Fpcm9XaW4uY3BwCSh3b3JraW5nIGNvcHkpCkBAIC00MCwyMSAr NDAsMjYgQEAgc3RhdGljIGNhaXJvX3QqIGNyZWF0ZUNhaXJvQ29udGV4dFdpdGhIRAogewogICAg IC8vIFB1dCB0aGUgSERDIEluIGFkdmFuY2VkIG1vZGUgc28gaXQgd2lsbCBob25vciBhZmZpbmUg dHJhbnNmb3Jtcy4KICAgICBTZXRHcmFwaGljc01vZGUoaGRjLCBHTV9BRFZBTkNFRCk7Ci0gICAg CisKKyAgICBjYWlyb19zdXJmYWNlX3QqIHN1cmZhY2UgPSAwOworCiAgICAgSEJJVE1BUCBiaXRt YXAgPSBzdGF0aWNfY2FzdDxIQklUTUFQPihHZXRDdXJyZW50T2JqZWN0KGhkYywgT0JKX0JJVE1B UCkpOwogCiAgICAgQklUTUFQIGluZm87Ci0gICAgR2V0T2JqZWN0KGJpdG1hcCwgc2l6ZW9mKGlu Zm8pLCAmaW5mbyk7Ci0gICAgQVNTRVJUKGluZm8uYm1CaXRzUGl4ZWwgPT0gMzIpOworICAgIGlm ICghR2V0T2JqZWN0KGJpdG1hcCwgc2l6ZW9mKGluZm8pLCAmaW5mbykpCisgICAgICAgIHN1cmZh Y2UgPSBjYWlyb193aW4zMl9zdXJmYWNlX2NyZWF0ZShoZGMpOworICAgIGVsc2UgeworICAgICAg ICBBU1NFUlQoaW5mby5ibUJpdHNQaXhlbCA9PSAzMik7CiAKLSAgICBjYWlyb19zdXJmYWNlX3Qq IGltYWdlID0gY2Fpcm9faW1hZ2Vfc3VyZmFjZV9jcmVhdGVfZm9yX2RhdGEoKHVuc2lnbmVkIGNo YXIqKWluZm8uYm1CaXRzLAorICAgICAgICBzdXJmYWNlID0gY2Fpcm9faW1hZ2Vfc3VyZmFjZV9j cmVhdGVfZm9yX2RhdGEoKHVuc2lnbmVkIGNoYXIqKWluZm8uYm1CaXRzLAogICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBDQUlST19GT1JNQVRfQVJHQjMyLAog ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBpbmZvLmJtV2lk dGgsCiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIGluZm8u Ym1IZWlnaHQsCiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg IGluZm8uYm1XaWR0aEJ5dGVzKTsKKyAgICB9CiAKLSAgICBjYWlyb190KiBjb250ZXh0ID0gY2Fp cm9fY3JlYXRlKGltYWdlKTsKLSAgICBjYWlyb19zdXJmYWNlX2Rlc3Ryb3koaW1hZ2UpOworICAg IGNhaXJvX3QqIGNvbnRleHQgPSBjYWlyb19jcmVhdGUoc3VyZmFjZSk7CisgICAgY2Fpcm9fc3Vy ZmFjZV9kZXN0cm95KHN1cmZhY2UpOwogCiAgICAgcmV0dXJuIGNvbnRleHQ7CiB9Cg==