64044 2011-07-06 16:50:31 -0700 Don't call into Image::loadPlatformResource() for Chromium Autofill popup items without icons 2011-07-07 15:01:00 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED WONTFIX P2 Normal --- 1 isherman isherman abarth isherman tkent oldest_to_newest 433118 0 isherman 2011-07-06 16:50:31 -0700 Don't call into Image::loadPlatformResource() for Chromium Autofill popup items without icons 433119 1 99901 isherman 2011-07-06 16:54:04 -0700 Created attachment 99901 Patch 433150 2 99901 abarth 2011-07-06 18:27:48 -0700 Comment on attachment 99901 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=99901&action=review R- for lack of tests. > Source/WebCore/ChangeLog:8 > + No new tests because the changes are all in platform-specific Chromium Autofill code. That's not a reason not to have tests! > Source/WebCore/platform/chromium/PopupMenuChromium.cpp:1043 > + image = Image::loadPlatformResource(itemIcon.utf8().data()); Bad indent > Source/WebCore/platform/chromium/PopupMenuChromium.cpp:1192 > - RefPtr<Image> image(Image::loadPlatformResource(icon.utf8().data())); > + RefPtr<Image> image; > + if (!icon.isEmpty()) > + image = Image::loadPlatformResource(icon.utf8().data()); I don't really understand this pattern. Why doesn't loadPlatformResource just return a null ptr if icon is empty? > Source/WebCore/platform/chromium/PopupMenuChromium.cpp:1382 > + iconImage = Image::loadPlatformResource(icon.utf8().data()); Bad indent. 433720 3 isherman 2011-07-07 15:01:00 -0700 (In reply to comment #2) > > Source/WebCore/platform/chromium/PopupMenuChromium.cpp:1192 > > - RefPtr<Image> image(Image::loadPlatformResource(icon.utf8().data())); > > + RefPtr<Image> image; > > + if (!icon.isEmpty()) > > + image = Image::loadPlatformResource(icon.utf8().data()); > > I don't really understand this pattern. Why doesn't loadPlatformResource just return a null ptr if icon is empty? Fair enough, I'll make that change instead. 99901 2011-07-06 16:54:04 -0700 2011-07-06 18:27:47 -0700 Patch bug-64044-20110706165403.patch text/plain 3117 isherman U3VidmVyc2lvbiBSZXZpc2lvbjogOTA0NDQKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwppbmRleCBhMDc3ZDJlNjdiMGEwMTI1 ZGQwZjUzMzUwMGQ4YjhjNjY2ODVkNWNkLi5hMzRlODBiMDcxZmI2NDllZDIxOWVlMmQ2ZDQ0NTdj M2IyMTQxODZlIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvU291 cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTcgQEAKKzIwMTEtMDctMDYgIElseWEg U2hlcm1hbiAgPGlzaGVybWFuQGNocm9taXVtLm9yZz4KKworICAgICAgICBEb24ndCBjYWxsIGlu dG8gSW1hZ2U6OmxvYWRQbGF0Zm9ybVJlc291cmNlKCkgZm9yIENocm9taXVtIEF1dG9maWxsIHBv cHVwIGl0ZW1zIHdpdGhvdXQgaWNvbnMKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcv c2hvd19idWcuY2dpP2lkPTY0MDQ0CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BT ISkuCisKKyAgICAgICAgTm8gbmV3IHRlc3RzIGJlY2F1c2UgdGhlIGNoYW5nZXMgYXJlIGFsbCBp biBwbGF0Zm9ybS1zcGVjaWZpYyBDaHJvbWl1bSBBdXRvZmlsbCBjb2RlLgorCisgICAgICAgICog cGxhdGZvcm0vY2hyb21pdW0vUG9wdXBNZW51Q2hyb21pdW0uY3BwOgorICAgICAgICAoV2ViQ29y ZTo6UG9wdXBMaXN0Qm94OjpwYWludFJvdyk6CisgICAgICAgIChXZWJDb3JlOjpQb3B1cExpc3RC b3g6OmdldFJvd0hlaWdodCk6CisgICAgICAgIChXZWJDb3JlOjpQb3B1cExpc3RCb3g6OmxheW91 dCk6CisKIDIwMTEtMDctMDUgIEtlbnRhcm8gSGFyYSAgPGhhcmFrZW5AZ29vZ2xlLmNvbT4KIAog ICAgICAgICBUcmlnZ2VyIGEgJ2NoYW5nZScgZXZlbnQgd2hlbmV2ZXIgc2VsZWN0ZWQgZmlsZXMg YXJlIGNoYW5nZWQKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2Nocm9taXVt L1BvcHVwTWVudUNocm9taXVtLmNwcCBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2Nocm9taXVt L1BvcHVwTWVudUNocm9taXVtLmNwcAppbmRleCBlM2JhODZjYzRlZGNmN2YyYzBmNGI2ZTM5Y2Y1 NmRhMzFhYjE2ODY4Li5mMGUyMjliMGVlMWI1NTlhMjY3NzU3MzE5MjY5NjgzMTM0MTRmYTU2IDEw MDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9jaHJvbWl1bS9Qb3B1cE1lbnVDaHJv bWl1bS5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vY2hyb21pdW0vUG9wdXBNZW51 Q2hyb21pdW0uY3BwCkBAIC0xMDM4LDcgKzEwMzgsOSBAQCB2b2lkIFBvcHVwTGlzdEJveDo6cGFp bnRSb3coR3JhcGhpY3NDb250ZXh0KiBnYywgY29uc3QgSW50UmVjdCYgcmVjdCwgaW50IHJvd0lu ZAogICAgIGludCByZW1haW5pbmdXaWR0aCA9IHJvd1JlY3Qud2lkdGgoKSAtIHJpZ2h0UGFkZGlu ZzsKIAogICAgIC8vIERyYXcgdGhlIGljb24gaWYgYXBwbGljYWJsZS4KLSAgICBSZWZQdHI8SW1h Z2U+IGltYWdlKEltYWdlOjpsb2FkUGxhdGZvcm1SZXNvdXJjZShpdGVtSWNvbi51dGY4KCkuZGF0 YSgpKSk7CisgICAgUmVmUHRyPEltYWdlPiBpbWFnZTsKKyAgICBpZiAoIWl0ZW1JY29uLmlzRW1w dHkoKSkKKyAgICAgIGltYWdlID0gSW1hZ2U6OmxvYWRQbGF0Zm9ybVJlc291cmNlKGl0ZW1JY29u LnV0ZjgoKS5kYXRhKCkpOwogICAgIGlmIChpbWFnZSAmJiAhaW1hZ2UtPmlzTnVsbCgpKSB7CiAg ICAgICAgIEludFJlY3QgaW1hZ2VSZWN0ID0gaW1hZ2UtPnJlY3QoKTsKICAgICAgICAgcmVtYWlu aW5nV2lkdGggLT0gKGltYWdlUmVjdC53aWR0aCgpICsga0xhYmVsVG9JY29uUGFkZGluZyk7CkBA IC0xMTg1LDcgKzExODcsOSBAQCBpbnQgUG9wdXBMaXN0Qm94OjpnZXRSb3dIZWlnaHQoaW50IGlu ZGV4KQogICAgICAgICByZXR1cm4gc2VwYXJhdG9ySGVpZ2h0OwogCiAgICAgU3RyaW5nIGljb24g PSBtX3BvcHVwQ2xpZW50LT5pdGVtSWNvbihpbmRleCk7Ci0gICAgUmVmUHRyPEltYWdlPiBpbWFn ZShJbWFnZTo6bG9hZFBsYXRmb3JtUmVzb3VyY2UoaWNvbi51dGY4KCkuZGF0YSgpKSk7CisgICAg UmVmUHRyPEltYWdlPiBpbWFnZTsKKyAgICBpZiAoIWljb24uaXNFbXB0eSgpKQorICAgICAgaW1h Z2UgPSBJbWFnZTo6bG9hZFBsYXRmb3JtUmVzb3VyY2UoaWNvbi51dGY4KCkuZGF0YSgpKTsKIAog ICAgIGludCBmb250SGVpZ2h0ID0gZ2V0Um93Rm9udChpbmRleCkuZm9udE1ldHJpY3MoKS5oZWln aHQoKTsKICAgICBpbnQgaWNvbkhlaWdodCA9IChpbWFnZSAmJiAhaW1hZ2UtPmlzTnVsbCgpKSA/ IGltYWdlLT5yZWN0KCkuaGVpZ2h0KCkgOiAwOwpAQCAtMTM1NSw3ICsxMzU5LDcgQEAgdm9pZCBQ b3B1cExpc3RCb3g6OnNldE1heFdpZHRoQW5kTGF5b3V0KGludCBtYXhXaWR0aCkKIHZvaWQgUG9w dXBMaXN0Qm94OjpsYXlvdXQoKQogewogICAgIGJvb2wgaXNSaWdodEFsaWduZWQgPSBtX3BvcHVw Q2xpZW50LT5tZW51U3R5bGUoKS50ZXh0RGlyZWN0aW9uKCkgPT0gUlRMOwotICAgIAorCiAgICAg Ly8gU2l6ZSBvdXIgY2hpbGQgaXRlbXMuCiAgICAgaW50IGJhc2VXaWR0aCA9IDA7CiAgICAgaW50 IHBhZGRpbmdXaWR0aCA9IDA7CkBAIC0xMzczLDcgKzEzNzcsMTAgQEAgdm9pZCBQb3B1cExpc3RC b3g6OmxheW91dCgpCiAgICAgICAgIFN0cmluZyB0ZXh0ID0gbV9wb3B1cENsaWVudC0+aXRlbVRl eHQoaSk7CiAgICAgICAgIFN0cmluZyBsYWJlbCA9IG1fcG9wdXBDbGllbnQtPml0ZW1MYWJlbChp KTsKICAgICAgICAgU3RyaW5nIGljb24gPSBtX3BvcHVwQ2xpZW50LT5pdGVtSWNvbihpKTsKLSAg ICAgICAgUmVmUHRyPEltYWdlPiBpY29uSW1hZ2UoSW1hZ2U6OmxvYWRQbGF0Zm9ybVJlc291cmNl KGljb24udXRmOCgpLmRhdGEoKSkpOworICAgICAgICBSZWZQdHI8SW1hZ2U+IGljb25JbWFnZTsK KyAgICAgICAgaWYgKCFpY29uLmlzRW1wdHkoKSkKKyAgICAgICAgICBpY29uSW1hZ2UgPSBJbWFn ZTo6bG9hZFBsYXRmb3JtUmVzb3VyY2UoaWNvbi51dGY4KCkuZGF0YSgpKTsKKwogICAgICAgICBp bnQgd2lkdGggPSAwOwogICAgICAgICBpZiAoIXRleHQuaXNFbXB0eSgpKQogICAgICAgICAgICAg d2lkdGggPSBpdGVtRm9udC53aWR0aChUZXh0UnVuKHRleHQpKTsK