110495 2013-02-21 11:58:31 -0800 Plug-in snapshotting code always accepts first snapshot 2013-02-21 13:57:36 -0800 1 1 1 Unclassified WebKit Plug-ins 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 dino dino esprehn+autocc ojan.autocc webkit.review.bot oldest_to_newest 838659 0 dino 2013-02-21 11:58:31 -0800 An HTMLPlugInImageElement swaps to a different renderer when it receives a snapshot from its embedding API (currently only WK2). This is a regression - previously we'd take a number of snapshots waiting for an image that had useful data, and there was a common renderer for the plugin and snapshot. This is the first part of the fix, which is to have the embedder tell the plugin when it thinks it has enough to snapshot. Only then will we swap to the other renderer. A later fix will be to make sure that we show intermediate (if mostly empty) snapshots. 838667 1 189570 dino 2013-02-21 12:08:59 -0800 Created attachment 189570 Patch 838762 2 189570 thorton 2013-02-21 13:22:16 -0800 Comment on attachment 189570 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=189570&action=review > Source/WebKit2/WebProcess/Plugins/PluginView.cpp:74 > -static const unsigned maximumSnapshotRetries = 60; > +static const unsigned maximumSnapshotRetries = 4; I don't see an explanation for this change. > Source/WebKit2/WebProcess/Plugins/PluginView.cpp:1609 > + m_pluginElement->setDisplayState(HTMLPlugInElement::DisplayingSnapshot); When does the tear-down happen now? 838769 3 189570 dino 2013-02-21 13:28:02 -0800 Comment on attachment 189570 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=189570&action=review >> Source/WebKit2/WebProcess/Plugins/PluginView.cpp:74 >> +static const unsigned maximumSnapshotRetries = 4; > > I don't see an explanation for this change. I will add this to the changelog: I also reduced the number of snapshot attempts we will make before giving up. We don't want to sit around for 66 seconds displaying nothing. >> Source/WebKit2/WebProcess/Plugins/PluginView.cpp:1609 >> + m_pluginElement->setDisplayState(HTMLPlugInElement::DisplayingSnapshot); > > When does the tear-down happen now? Oh, that happens anyway, because as soon as I set the displayState it will swap renderers. It was the existing renderer that had the Widget, and the reference to the PluginView, and so we tear things down in the destructor. 838807 4 dino 2013-02-21 13:57:36 -0800 Committed r143650: <http://trac.webkit.org/changeset/143650> 189570 2013-02-21 12:08:59 -0800 2013-02-21 13:28:46 -0800 Patch bug-110495-20130221120519.patch text/plain 6227 dino U3VidmVyc2lvbiBSZXZpc2lvbjogMTQzNDc1CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMDJmZmU0NDFhNjNiYWJl NWFkNDBmOWM4ZmZjODIxYmMxMmUwOTRiYy4uNmY1ZDhiY2I5ODhiMTYxZTYwNzBjZjcxNmM0NTkx N2NjNjk0NDBhNSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDI2IEBACisyMDEzLTAyLTIxICBEZWFu IEphY2tzb24gIDxkaW5vQGFwcGxlLmNvbT4KKworICAgICAgICBQbHVnLWluIHNuYXBzaG90dGlu ZyBjb2RlIGFsd2F5cyBhY2NlcHRzIGZpcnN0IHNuYXBzaG90CisgICAgICAgIGh0dHBzOi8vYnVn cy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMTA0OTUKKworICAgICAgICBSZXZpZXdlZCBi eSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBXaGVuIHdlIGRldGVjdCBhIHBsdWdpbiB0aGF0 IGNhbiBiZSBzbmFwc2hvdHRlZCB3ZSBzdGFydCBjYXB0dXJpbmcKKyAgICAgICAgaW1hZ2VzIHVu dGlsIHdlIGZpbmQgb25lIHRoYXQgd2UgYmVsaWV2ZSBpc24ndCBibGFuaywgb3Igd2UgdGltZW91 dC4KKyAgICAgICAgSSBpbnRyb2R1Y2VkIGEgcmVncmVzc2lvbiByZWNlbnRseSB3aGVyZSB3ZSBz d2FwIHJlbmRlcmVycyBhcworICAgICAgICBzb29uIGFzIHRoZSBzbmFwc2hvdCBhcnJpdmVzLCB3 aGV0aGVyIG9yIG5vdCBpcyBpcyBibGFuay4KKworICAgICAgICBUaGUgZml4IHdhcyB0byBoYXZl IHRoZSBlbWJlZGRlciAoY3VycmVudGx5IG9ubHkgV0syKSBiZSB0aGUgb25lCisgICAgICAgIHdo byB0ZWxscyB0aGUgSFRNTFBsdWdJbkVsZW1lbnQgdG8gc3RhcnQgZGlzcGxheWluZyBzbmFwc2hv dHMuCisKKyAgICAgICAgKiBodG1sL0hUTUxQbHVnSW5FbGVtZW50Lmg6CisgICAgICAgIChXZWJD b3JlOjpIVE1MUGx1Z0luRWxlbWVudDo6c2V0RGlzcGxheVN0YXRlKTogTWFrZSB0aGlzIGEgdmly dHVhbCBmdW5jdGlvbi4KKyAgICAgICAgKiBodG1sL0hUTUxQbHVnSW5JbWFnZUVsZW1lbnQuY3Bw OgorICAgICAgICAoV2ViQ29yZTo6SFRNTFBsdWdJbkltYWdlRWxlbWVudDo6c2V0RGlzcGxheVN0 YXRlKTogT3ZlcnJpZGUgdG8gY2FsbCB0aGUKKyAgICAgICAgICAgIGJhc2UgY2xhc3MgdmVyc2lv biwgYnV0IHN3YXAgcmVuZGVyZXJzIGlmIHdlIHNob3VsZCBtb3ZlIHRvIHRoZSBzbmFwc2hvdC4K KyAgICAgICAgKFdlYkNvcmU6OkhUTUxQbHVnSW5JbWFnZUVsZW1lbnQ6OnVwZGF0ZVNuYXBzaG90 KTogRG9uJ3Qgc3dhcCByZW5kZXJlcnMgaGVyZSBhbnkgbW9yZS4KKyAgICAgICAgKiBodG1sL0hU TUxQbHVnSW5JbWFnZUVsZW1lbnQuaDogTmV3IHZpcnR1YWwgdmVyc2lvbiBvZiBzZXREaXNwbGF5 U3RhdGUuCisKIDIwMTMtMDItMjAgIFJvYmVydCBIb2dhbiAgPHJvYmVydEB3ZWJraXQub3JnPgog CiAgICAgICAgIGlucHV0IGVsZW1lbnQgd2l0aCBwbGFjZWhvbGRlciB0ZXh0IGFuZCB3aWR0aCBz ZXQgdG8gMTAwJSBvbiBmb2N1cyBjYXVzZXMgb3ZlcmZsb3cgZXZlbiBhZnRlciBsb3NpbmcgZm9j dXMKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQy L0NoYW5nZUxvZwppbmRleCBiYzc3ZWMyNTFmNDdkMmJhZTc1ZTFlYTAwOWZkZDczM2Y4YTUwZDRh Li5lYTdkYzIzNjg4MjUwZWQ3ZGY0ZWE0MDQ2OGIyOGYwMTc2N2RjYTExIDEwMDY0NAotLS0gYS9T b3VyY2UvV2ViS2l0Mi9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCkBA IC0xLDMgKzEsMjMgQEAKKzIwMTMtMDItMjEgIERlYW4gSmFja3NvbiAgPGRpbm9AYXBwbGUuY29t PgorCisgICAgICAgIFBsdWctaW4gc25hcHNob3R0aW5nIGNvZGUgYWx3YXlzIGFjY2VwdHMgZmly c3Qgc25hcHNob3QKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dp P2lkPTExMDQ5NQorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAg ICAgIFdoZW4gd2UgZGV0ZWN0IGEgcGx1Z2luIHRoYXQgY2FuIGJlIHNuYXBzaG90dGVkIHdlIHN0 YXJ0IGNhcHR1cmluZworICAgICAgICBpbWFnZXMgdW50aWwgd2UgZmluZCBvbmUgdGhhdCB3ZSBi ZWxpZXZlIGlzbid0IGJsYW5rLCBvciB3ZSB0aW1lb3V0LgorICAgICAgICBJIGludHJvZHVjZWQg YSByZWdyZXNzaW9uIHJlY2VudGx5IHdoZXJlIHdlIHN3YXAgcmVuZGVyZXJzIGFzCisgICAgICAg IHNvb24gYXMgdGhlIHNuYXBzaG90IGFycml2ZXMsIHdoZXRoZXIgb3Igbm90IGlzIGlzIGJsYW5r LgorCisgICAgICAgIFRoZSBmaXggd2FzIHRvIGhhdmUgdGhlIGVtYmVkZGVyIChjdXJyZW50bHkg b25seSBXSzIpIGJlIHRoZSBvbmUKKyAgICAgICAgd2hvIHRlbGxzIHRoZSBIVE1MUGx1Z0luRWxl bWVudCB0byBzdGFydCBkaXNwbGF5aW5nIHNuYXBzaG90cy4KKworICAgICAgICAqIFdlYlByb2Nl c3MvUGx1Z2lucy9QbHVnaW5WaWV3LmNwcDoKKyAgICAgICAgKFdlYktpdDo6UGx1Z2luVmlldzo6 cGx1Z2luU25hcHNob3RUaW1lckZpcmVkKTogV2hlbiB3ZSBmaW5kIGEKKyAgICAgICAgc25hcHNo b3QgdGhhdCB3ZSBsaWtlLCB0ZWxsIHRoZSBIVE1MUGx1Z0luRWxlbWVudCB0byBtb3ZlIHRvCisg ICAgICAgIHRoZSBzbmFwc2hvdCB2aWV3LgorCiAyMDEzLTAyLTIwICBIdWFuZyBEb25nc3VuZyAg PGx1eHRlbGxhQGNvbXBhbnkxMDAubmV0PgogCiAgICAgICAgIFtXSzJdIFJlbW92ZSBtX3ZpZXdw b3J0U2l6ZSBpbiBXZWJQYWdlLgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvaHRtbC9IVE1M UGx1Z0luRWxlbWVudC5oIGIvU291cmNlL1dlYkNvcmUvaHRtbC9IVE1MUGx1Z0luRWxlbWVudC5o CmluZGV4IDA4YTM5MWQzMWIzMDA5MjQ3Zjg5MDQ3M2M0NTliZGY0MWUxMjEwMWEuLmUzM2M1NTMw OTk0YzlhNjQyMjdjYTFjOGUzMGU4NzQwMmM2YTU0YWEgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJD b3JlL2h0bWwvSFRNTFBsdWdJbkVsZW1lbnQuaAorKysgYi9Tb3VyY2UvV2ViQ29yZS9odG1sL0hU TUxQbHVnSW5FbGVtZW50LmgKQEAgLTU1LDcgKzU1LDcgQEAgcHVibGljOgogICAgICAgICBQbGF5 aW5nCiAgICAgfTsKICAgICBEaXNwbGF5U3RhdGUgZGlzcGxheVN0YXRlKCkgY29uc3QgeyByZXR1 cm4gbV9kaXNwbGF5U3RhdGU7IH0KLSAgICB2b2lkIHNldERpc3BsYXlTdGF0ZShEaXNwbGF5U3Rh dGUgc3RhdGUpIHsgbV9kaXNwbGF5U3RhdGUgPSBzdGF0ZTsgfQorICAgIHZpcnR1YWwgdm9pZCBz ZXREaXNwbGF5U3RhdGUoRGlzcGxheVN0YXRlIHN0YXRlKSB7IG1fZGlzcGxheVN0YXRlID0gc3Rh dGU7IH0KICAgICB2aXJ0dWFsIHZvaWQgdXBkYXRlU25hcHNob3QoUGFzc1JlZlB0cjxJbWFnZT4p IHsgfQogICAgIHZpcnR1YWwgdm9pZCBkaXNwYXRjaFBlbmRpbmdNb3VzZUNsaWNrKCkgeyB9CiAK ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL2h0bWwvSFRNTFBsdWdJbkltYWdlRWxlbWVudC5j cHAgYi9Tb3VyY2UvV2ViQ29yZS9odG1sL0hUTUxQbHVnSW5JbWFnZUVsZW1lbnQuY3BwCmluZGV4 IGRhMmMxYjRkZGVhMmI4ODZlZDZhN2YzMzE2ODFjZTQwNThhMmFlYTEuLjUxNDUzZjQ0ZDJjNmMx MmE2YmViYTc4ZDdhODdlZTdmMTA2YjY4N2IgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL2h0 bWwvSFRNTFBsdWdJbkltYWdlRWxlbWVudC5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvaHRtbC9I VE1MUGx1Z0luSW1hZ2VFbGVtZW50LmNwcApAQCAtODIsNiArODIsMTMgQEAgSFRNTFBsdWdJbklt YWdlRWxlbWVudDo6fkhUTUxQbHVnSW5JbWFnZUVsZW1lbnQoKQogICAgICAgICBkb2N1bWVudCgp LT51bnJlZ2lzdGVyRm9yUGFnZUNhY2hlU3VzcGVuc2lvbkNhbGxiYWNrcyh0aGlzKTsKIH0KIAor dm9pZCBIVE1MUGx1Z0luSW1hZ2VFbGVtZW50OjpzZXREaXNwbGF5U3RhdGUoRGlzcGxheVN0YXRl IHN0YXRlKQoreworICAgIEhUTUxQbHVnSW5FbGVtZW50OjpzZXREaXNwbGF5U3RhdGUoc3RhdGUp OworICAgIGlmIChkaXNwbGF5U3RhdGUoKSA9PSBEaXNwbGF5aW5nU25hcHNob3QpCisgICAgICAg IG1fc3dhcFJlbmRlcmVyVGltZXIuc3RhcnRPbmVTaG90KDApOworfQorCiBSZW5kZXJFbWJlZGRl ZE9iamVjdCogSFRNTFBsdWdJbkltYWdlRWxlbWVudDo6cmVuZGVyRW1iZWRkZWRPYmplY3QoKSBj b25zdAogewogICAgIC8vIEhUTUxPYmplY3RFbGVtZW50IGFuZCBIVE1MRW1iZWRFbGVtZW50IG1h eSByZXR1cm4gYXJiaXRyYXJ5IHJlbmRlcmVycwpAQCAtMjgxLDkgKzI4OCw2IEBAIHZvaWQgSFRN TFBsdWdJbkltYWdlRWxlbWVudDo6dXBkYXRlU25hcHNob3QoUGFzc1JlZlB0cjxJbWFnZT4gaW1h Z2UpCiAgICAgICAgIHRvUmVuZGVyU25hcHNob3R0ZWRQbHVnSW4ocmVuZGVyZXIoKSktPnVwZGF0 ZVNuYXBzaG90KGltYWdlKTsKICAgICAgICAgcmV0dXJuOwogICAgIH0KLQotICAgIHNldERpc3Bs YXlTdGF0ZShEaXNwbGF5aW5nU25hcHNob3QpOwotICAgIG1fc3dhcFJlbmRlcmVyVGltZXIuc3Rh cnRPbmVTaG90KDApOwogfQogCiBzdGF0aWMgQXRvbWljU3RyaW5nIGNsYXNzTmFtZUZvclNoYWRv d1Jvb3QoY29uc3QgTm9kZSogbm9kZSkKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL2h0bWwv SFRNTFBsdWdJbkltYWdlRWxlbWVudC5oIGIvU291cmNlL1dlYkNvcmUvaHRtbC9IVE1MUGx1Z0lu SW1hZ2VFbGVtZW50LmgKaW5kZXggNmZjMWY0NjczNTg0NGRkZjNhYzJhNmRhN2RkMWVmOTI4ZDZm MzY3OS4uZTZhM2ZlOWZiNjU3NzNhNjYxYzZiN2RlNmE1M2E3MjAwYzQ0ZTFlZiAxMDA2NDQKLS0t IGEvU291cmNlL1dlYkNvcmUvaHRtbC9IVE1MUGx1Z0luSW1hZ2VFbGVtZW50LmgKKysrIGIvU291 cmNlL1dlYkNvcmUvaHRtbC9IVE1MUGx1Z0luSW1hZ2VFbGVtZW50LmgKQEAgLTUyLDYgKzUyLDgg QEAgcHVibGljOgogCiAgICAgUmVuZGVyRW1iZWRkZWRPYmplY3QqIHJlbmRlckVtYmVkZGVkT2Jq ZWN0KCkgY29uc3Q7CiAKKyAgICB2aXJ0dWFsIHZvaWQgc2V0RGlzcGxheVN0YXRlKERpc3BsYXlT dGF0ZSkgT1ZFUlJJREU7CisKICAgICB2aXJ0dWFsIHZvaWQgdXBkYXRlV2lkZ2V0KFBsdWdpbkNy ZWF0aW9uT3B0aW9uKSA9IDA7CiAKICAgICBjb25zdCBTdHJpbmcmIHNlcnZpY2VUeXBlKCkgY29u c3QgeyByZXR1cm4gbV9zZXJ2aWNlVHlwZTsgfQpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdDIv V2ViUHJvY2Vzcy9QbHVnaW5zL1BsdWdpblZpZXcuY3BwIGIvU291cmNlL1dlYktpdDIvV2ViUHJv Y2Vzcy9QbHVnaW5zL1BsdWdpblZpZXcuY3BwCmluZGV4IDYxYmI3YmZlMmRhYjM3YzQwNmUxOTA2 N2ZhZTBiMzAxODEwYTVkZWIuLjA0MjY5Mjk1ZGZiOTBmOWM0ZTJiY2RkOGE2NDI5ZmI3NmY2NGQw YjEgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQyL1dlYlByb2Nlc3MvUGx1Z2lucy9QbHVnaW5W aWV3LmNwcAorKysgYi9Tb3VyY2UvV2ViS2l0Mi9XZWJQcm9jZXNzL1BsdWdpbnMvUGx1Z2luVmll dy5jcHAKQEAgLTcxLDcgKzcxLDcgQEAgbmFtZXNwYWNlIFdlYktpdCB7CiAKIC8vIFRoaXMgc2lt dWxhdGVkIG1vdXNlIGNsaWNrIGRlbGF5IGluIEhUTUxQbHVnSW5JbWFnZUVsZW1lbnQuY3BwIHNo b3VsZCBnZW5lcmFsbHkgYmUgdGhlIHNhbWUgb3Igc2hvcnRlciB0aGFuIHRoaXMgZGVsYXkuCiBz dGF0aWMgY29uc3QgZG91YmxlIHBsdWdpblNuYXBzaG90VGltZXJEZWxheSA9IDEuMTsKLXN0YXRp YyBjb25zdCB1bnNpZ25lZCBtYXhpbXVtU25hcHNob3RSZXRyaWVzID0gNjA7CitzdGF0aWMgY29u c3QgdW5zaWduZWQgbWF4aW11bVNuYXBzaG90UmV0cmllcyA9IDQ7CiAKIGNsYXNzIFBsdWdpblZp ZXc6OlVSTFJlcXVlc3QgOiBwdWJsaWMgUmVmQ291bnRlZDxVUkxSZXF1ZXN0PiB7CiBwdWJsaWM6 CkBAIC0xNjA2LDggKzE2MDYsNyBAQCB2b2lkIFBsdWdpblZpZXc6OnBsdWdpblNuYXBzaG90VGlt ZXJGaXJlZChEZWZlcnJhYmxlT25lU2hvdFRpbWVyPFBsdWdpblZpZXc+KikKICAgICB9CiAjZW5k aWYKIAotICAgIGRlc3Ryb3lQbHVnaW5BbmRSZXNldCgpOwotICAgIG1fcGx1Z2luID0gMDsKKyAg ICBtX3BsdWdpbkVsZW1lbnQtPnNldERpc3BsYXlTdGF0ZShIVE1MUGx1Z0luRWxlbWVudDo6RGlz cGxheWluZ1NuYXBzaG90KTsKIH0KIAogYm9vbCBQbHVnaW5WaWV3OjpzaG91bGRBbHdheXNBdXRv U3RhcnQoKSBjb25zdAo=