102148 2012-11-13 16:17:26 -0800 Automatically run small plugins 2012-11-14 00:31:42 -0800 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) Mac OS X 10.8 RESOLVED FIXED InRadar P2 Normal --- 98318 1 jonlee jonlee abarth andersca beidson bweinstein cevans cpu eric inferno mjs simon.fraser thorton webkit-bug-importer webkit.review.bot oldest_to_newest 766152 0 jonlee 2012-11-13 16:17:26 -0800 Plugins that are smaller than 1px x 1px should automatically run. 766153 1 webkit-bug-importer 2012-11-13 16:18:05 -0800 <rdar://problem/12695560> 766196 2 174030 jonlee 2012-11-13 17:08:31 -0800 Created attachment 174030 Patch 766213 3 174030 thorton 2012-11-13 17:20:14 -0800 Comment on attachment 174030 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=174030&action=review > Source/WebCore/rendering/RenderSnapshottedPlugIn.cpp:156 > + if (plugInImageElement()->displayState() < HTMLPlugInElement::Playing > + && (!width || !height || (width <= autoStartPlugInSizeThresholdWidth && height <= autoStartPlugInSizeThresholdHeight))) > + plugInImageElement()->setDisplayState(HTMLPlugInElement::Playing); Seems like this would let sites get around snapshotting/freezing by making their plugin 0x0 for the first layout and then the real size later. 766224 4 jonlee 2012-11-13 17:30:59 -0800 Filed bug 102157 to track this issue. 766226 5 eric 2012-11-13 17:32:21 -0800 Interesting. If I'm understanding this bug correctly (which I may not be!) this sounds like a different security decision than Chromium has made. 766228 6 eric 2012-11-13 17:33:10 -0800 (It's not clear to me that this change affects chromium. Just noting the possible divergence.) 766238 7 174030 darin 2012-11-13 17:43:26 -0800 Comment on attachment 174030 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=174030&action=review I’m OK with this change, although Tim’s point is an interesting one. > Source/WebCore/rendering/RenderEmbeddedObject.h:72 > + virtual void layout(); As you are moving this, please also add the OVERRIDE keyword. > Source/WebCore/rendering/RenderSnapshottedPlugIn.cpp:41 > +static int autoStartPlugInSizeThresholdWidth = 1; > +static int autoStartPlugInSizeThresholdHeight = 1; These should be const. Once they are const they need not be static (although there is no harm in marking them so, except perhaps to annoy Alexey). > Source/WebCore/rendering/RenderSnapshottedPlugIn.cpp:153 > + int width = rect.width(), height = rect.height(); We don’t do multiple variables on one line like this in WebKit. > Source/WebCore/rendering/RenderSnapshottedPlugIn.cpp:155 > + if (plugInImageElement()->displayState() < HTMLPlugInElement::Playing > + && (!width || !height || (width <= autoStartPlugInSizeThresholdWidth && height <= autoStartPlugInSizeThresholdHeight))) I suggest computing the content box size only after checking the displayState. > Source/WebCore/rendering/RenderSnapshottedPlugIn.h:57 > + virtual void layout(); Please add OVERRIDE. 766261 8 174030 jonlee 2012-11-13 18:04:08 -0800 Comment on attachment 174030 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=174030&action=review >> Source/WebCore/rendering/RenderEmbeddedObject.h:72 >> + virtual void layout(); > > As you are moving this, please also add the OVERRIDE keyword. Done. >> Source/WebCore/rendering/RenderSnapshottedPlugIn.cpp:41 >> +static int autoStartPlugInSizeThresholdHeight = 1; > > These should be const. Once they are const they need not be static (although there is no harm in marking them so, except perhaps to annoy Alexey). Always good to keep Alexey happy. >> Source/WebCore/rendering/RenderSnapshottedPlugIn.cpp:153 >> + int width = rect.width(), height = rect.height(); > > We don’t do multiple variables on one line like this in WebKit. Split. >> Source/WebCore/rendering/RenderSnapshottedPlugIn.cpp:155 >> + && (!width || !height || (width <= autoStartPlugInSizeThresholdWidth && height <= autoStartPlugInSizeThresholdHeight))) > > I suggest computing the content box size only after checking the displayState. Done. >> Source/WebCore/rendering/RenderSnapshottedPlugIn.h:57 >> + virtual void layout(); > > Please add OVERRIDE. Done. 766345 9 jonlee 2012-11-13 20:22:12 -0800 Committed r134526: <http://trac.webkit.org/changeset/134526> 174030 2012-11-13 17:08:31 -0800 2012-11-13 18:04:08 -0800 Patch bug-102148-20121113170631.patch text/plain 3964 jonlee U3VidmVyc2lvbiBSZXZpc2lvbjogMTM0NDgyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggYTY5YmZiNWY4ZWMyMDA3 YmJjZmU4MzBlODY2YzdjZDcxNjk3ZDlhMy4uOGYzNmNiYjhiY2I3ZDdkZDFhODNjN2Y5ZTgxYWQw MWRmNWIyMmY2MiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE5IEBACisyMDEyLTExLTEzICBKb24g TGVlICA8am9ubGVlQGFwcGxlLmNvbT4KKworICAgICAgICBBdXRvbWF0aWNhbGx5IHJ1biBzbWFs bCBwbHVnaW5zCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9p ZD0xMDIxNDgKKyAgICAgICAgPHJkYXI6Ly9wcm9ibGVtLzEyNjk1NTYwPgorCisgICAgICAgIFJl dmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICogcmVuZGVyaW5nL1JlbmRlckVt YmVkZGVkT2JqZWN0Lmg6IFByb21vdGUgbGF5b3V0KCkgdG8gcHJvdGVjdGVkLgorICAgICAgICAq IHJlbmRlcmluZy9SZW5kZXJTbmFwc2hvdHRlZFBsdWdJbi5jcHA6IEFkZCBjb25zdGFudHMgZm9y IHRocmVzaG9sZCBzaXplIGZvciBwbHVnaW5zIHRoYXQgd2lsbCBhdXRvLXN0YXJ0LgorICAgICAg ICAoV2ViQ29yZTo6UmVuZGVyU25hcHNob3R0ZWRQbHVnSW46OmxheW91dCk6IEFmdGVyIGxheW91 dCwgb2J0YWluIHRoZSB3aWR0aCBhbmQgaGVpZ2h0IG9mIHRoZSBlbGVtZW50LgorICAgICAgICBJ ZiBlaXRoZXIgZGltZW5zaW9uIGlzIDAsIG9yIHRoZSBvdmVyYWxsIHNpemUgb2YgdGhlIHBsdWdp biBpcyBzbWFsbGVyIHRoYXQgdGhlIHRocmVzaG9sZCBzaXplLCBtb3ZlIHRoZQorICAgICAgICBk aXNwbGF5IHN0YXRlIHRvIFBsYXlpbmcuIEFzc3VtaW5nIHdlIHdpbGwgYWx3YXlzIGxheW91dCBi ZWZvcmUgZmlyc3QgcGFpbnQsIGNoYW5naW5nIHRoZSBzdGF0ZSBoZXJlCisgICAgICAgIHNob3Vs ZCBiZSBzYWZlLgorICAgICAgICAqIHJlbmRlcmluZy9SZW5kZXJTbmFwc2hvdHRlZFBsdWdJbi5o OgorCiAyMDEyLTExLTEzICBBbmRyZWkgQnVjdXIgIDxhYnVjdXJAYWRvYmUuY29tPgogCiAgICAg ICAgIFtDU1MgUmVnaW9uc10gQWRkIFJlZ2lvbiBpbmZvIGZvciBSb290TGluZUJveGVzIGFuZCBw YWNrIHRoZSBwYWdpbmF0aW9uIGRhdGEKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3JlbmRl cmluZy9SZW5kZXJFbWJlZGRlZE9iamVjdC5oIGIvU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1Jl bmRlckVtYmVkZGVkT2JqZWN0LmgKaW5kZXggMTMzZjM0ZDkyZjhiNDk5MDcxOTRmZWViZmY4MmVj NDIyNDVjY2UyZS4uMmY1MGFmZDZlNDVkMTI1NDIxNzA1Nzg2Y2M4ZjQyMmI1MGQxYTU0ZCAxMDA2 NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlckVtYmVkZGVkT2JqZWN0LmgK KysrIGIvU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlckVtYmVkZGVkT2JqZWN0LmgKQEAg LTY4LDYgKzY4LDkgQEAgcHJvdGVjdGVkOgogICAgIFJlbmRlck9iamVjdENoaWxkTGlzdCogY2hp bGRyZW4oKSB7IHJldHVybiAmbV9jaGlsZHJlbjsgfQogI2VuZGlmCiAKK3Byb3RlY3RlZDoKKyAg ICB2aXJ0dWFsIHZvaWQgbGF5b3V0KCk7CisKIHByaXZhdGU6CiAgICAgdmlydHVhbCBjb25zdCBj aGFyKiByZW5kZXJOYW1lKCkgY29uc3QgeyByZXR1cm4gIlJlbmRlckVtYmVkZGVkT2JqZWN0Ijsg fQogICAgIHZpcnR1YWwgYm9vbCBpc0VtYmVkZGVkT2JqZWN0KCkgY29uc3QgeyByZXR1cm4gdHJ1 ZTsgfQpAQCAtNzYsNyArNzksNiBAQCBwcml2YXRlOgogICAgIHZpcnR1YWwgYm9vbCByZXF1aXJl c0xheWVyKCkgY29uc3Q7CiAjZW5kaWYKIAotICAgIHZpcnR1YWwgdm9pZCBsYXlvdXQoKTsKICAg ICB2aXJ0dWFsIHZvaWQgdmlld0NsZWFyZWQoKTsKIAogICAgIHZpcnR1YWwgYm9vbCBub2RlQXRQ b2ludChjb25zdCBIaXRUZXN0UmVxdWVzdCYsIEhpdFRlc3RSZXN1bHQmLCBjb25zdCBIaXRUZXN0 TG9jYXRpb24mIGxvY2F0aW9uSW5Db250YWluZXIsIGNvbnN0IExheW91dFBvaW50JiBhY2N1bXVs YXRlZE9mZnNldCwgSGl0VGVzdEFjdGlvbikgT1ZFUlJJREU7CmRpZmYgLS1naXQgYS9Tb3VyY2Uv V2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVyU25hcHNob3R0ZWRQbHVnSW4uY3BwIGIvU291cmNlL1dl YkNvcmUvcmVuZGVyaW5nL1JlbmRlclNuYXBzaG90dGVkUGx1Z0luLmNwcAppbmRleCBhN2E5ZjRh ZWUzNTkzZTUwYzZkNTc5MjMxZDg3M2UzMTBiNGE4NjcyLi5jODJlNWUwYjk0OWQwZTczZjgzOTBl YzMxMzU1YzRkNzVkNWJlODk2IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcv UmVuZGVyU25hcHNob3R0ZWRQbHVnSW4uY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL3JlbmRlcmlu Zy9SZW5kZXJTbmFwc2hvdHRlZFBsdWdJbi5jcHAKQEAgLTM3LDYgKzM3LDkgQEAKIAogbmFtZXNw YWNlIFdlYkNvcmUgewogCitzdGF0aWMgaW50IGF1dG9TdGFydFBsdWdJblNpemVUaHJlc2hvbGRX aWR0aCA9IDE7CitzdGF0aWMgaW50IGF1dG9TdGFydFBsdWdJblNpemVUaHJlc2hvbGRIZWlnaHQg PSAxOworCiBSZW5kZXJTbmFwc2hvdHRlZFBsdWdJbjo6UmVuZGVyU25hcHNob3R0ZWRQbHVnSW4o SFRNTFBsdWdJbkltYWdlRWxlbWVudCogZWxlbWVudCkKICAgICA6IFJlbmRlckVtYmVkZGVkT2Jq ZWN0KGVsZW1lbnQpCiAgICAgLCBtX3NuYXBzaG90UmVzb3VyY2UoUmVuZGVySW1hZ2VSZXNvdXJj ZTo6Y3JlYXRlKCkpCkBAIC0xNDMsNCArMTQ2LDE0IEBAIHZvaWQgUmVuZGVyU25hcHNob3R0ZWRQ bHVnSW46OmhhbmRsZUV2ZW50KEV2ZW50KiBldmVudCkKICAgICB9CiB9CiAKK3ZvaWQgUmVuZGVy U25hcHNob3R0ZWRQbHVnSW46OmxheW91dCgpCit7CisgICAgUmVuZGVyRW1iZWRkZWRPYmplY3Q6 OmxheW91dCgpOworICAgIExheW91dFJlY3QgcmVjdCA9IGNvbnRlbnRCb3hSZWN0KCk7CisgICAg aW50IHdpZHRoID0gcmVjdC53aWR0aCgpLCBoZWlnaHQgPSByZWN0LmhlaWdodCgpOworICAgIGlm IChwbHVnSW5JbWFnZUVsZW1lbnQoKS0+ZGlzcGxheVN0YXRlKCkgPCBIVE1MUGx1Z0luRWxlbWVu dDo6UGxheWluZworICAgICAgICAmJiAoIXdpZHRoIHx8ICFoZWlnaHQgfHwgKHdpZHRoIDw9IGF1 dG9TdGFydFBsdWdJblNpemVUaHJlc2hvbGRXaWR0aCAmJiBoZWlnaHQgPD0gYXV0b1N0YXJ0UGx1 Z0luU2l6ZVRocmVzaG9sZEhlaWdodCkpKQorICAgICAgICBwbHVnSW5JbWFnZUVsZW1lbnQoKS0+ c2V0RGlzcGxheVN0YXRlKEhUTUxQbHVnSW5FbGVtZW50OjpQbGF5aW5nKTsKK30KKwogfSAvLyBu YW1lc3BhY2UgV2ViQ29yZQpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL1Jl bmRlclNuYXBzaG90dGVkUGx1Z0luLmggYi9Tb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVy U25hcHNob3R0ZWRQbHVnSW4uaAppbmRleCA2YTEzYTNhM2ZmZmU4YmU1Y2YxYzMxZDAwNzlhOWJk NjZmODJiZDNhLi5kNzQyYWZjMjdkOGRkODk2NzJiMzQ4NzdlMjFkZDg5MDhjZDUzNjM3IDEwMDY0 NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVyU25hcHNob3R0ZWRQbHVnSW4u aAorKysgYi9Tb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVyU25hcHNob3R0ZWRQbHVnSW4u aApAQCAtNTQsNiArNTQsOCBAQCBwcml2YXRlOgogICAgIHZpcnR1YWwgdm9pZCBwYWludFJlcGxh Y2VkKFBhaW50SW5mbyYsIGNvbnN0IExheW91dFBvaW50JikgT1ZFUlJJREU7CiAgICAgdm9pZCBw YWludFJlcGxhY2VkU25hcHNob3QoUGFpbnRJbmZvJiwgY29uc3QgTGF5b3V0UG9pbnQmKTsKIAor ICAgIHZpcnR1YWwgdm9pZCBsYXlvdXQoKTsKKwogICAgIE93blB0cjxSZW5kZXJJbWFnZVJlc291 cmNlPiBtX3NuYXBzaG90UmVzb3VyY2U7CiB9OwogCg==