99102 2012-10-11 14:55:17 -0700 SVGImage::data() returns NULL 2012-10-23 20:21:26 -0700 1 1 1 Unclassified WebKit SVG 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 jcivelli pdr jcivelli webkit.review.bot zimmermann oldest_to_newest 740283 0 jcivelli 2012-10-11 14:55:17 -0700 Calling SVGImage::data() returns NULL. It should return the actual SVG data. 741003 1 jcivelli 2012-10-12 10:08:46 -0700 One way to repro is to run the Chromium webkit_unit_tests, more specifically WebPageNewSerializeTest,SVGImageDontCrash. webkit_unit_tests --gtest_filter=WebPageNewSerializeTest.SVGImageDontCrash In PageSerializer::addImageToResources() there is a LOG_ERROR that's printed when the SVG image content is missing. 741086 2 ap 2012-10-12 11:53:07 -0700 Is there any user observable effect? 741125 3 jcivelli 2012-10-12 12:50:53 -0700 The only effect I am aware of is that MHTML generated for a page with SVG images will not include them (as it does not get the data). 747644 4 pdr 2012-10-22 09:56:20 -0700 @Jay, were you able to create a repro for this (even if just in c++)? 747650 5 jcivelli 2012-10-22 10:02:50 -0700 @Philip You can repro with the steps I described in comment #1. It's a unit-test that generate MHTML from a simple page with an SVG image. 748236 6 pdr 2012-10-22 20:51:51 -0700 It looks like the image returned from imageForRenderer() does not contain the copied data() but image->image()->data() does. I should have a patch up tomorrow for this. 748941 7 170227 pdr 2012-10-23 13:45:52 -0700 Created attachment 170227 First pass 749104 8 170227 eric 2012-10-23 16:16:34 -0700 Comment on attachment 170227 First pass LGTM. Do we know if there is a way to test this with LayoutTests instead of just a Chromium Unit Test? 749126 9 jcivelli 2012-10-23 16:43:28 -0700 LGTM @eric We can't test that from a Layout test as page serialization cannot be triggered from a page. 749136 10 170271 pdr 2012-10-23 17:10:43 -0700 Created attachment 170271 Add back null check per Jay's request 749153 11 170271 webkit.review.bot 2012-10-23 17:27:55 -0700 Comment on attachment 170271 Add back null check per Jay's request Rejecting attachment 170271 from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1 ERROR: /mnt/git/webkit-commit-queue/Source/WebCore/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive). Full output: http://queues.webkit.org/results/14529170 749155 12 170276 pdr 2012-10-23 17:30:00 -0700 Created attachment 170276 Updating reviewer 749268 13 170276 webkit.review.bot 2012-10-23 20:21:23 -0700 Comment on attachment 170276 Updating reviewer Clearing flags on attachment: 170276 Committed r132295: <http://trac.webkit.org/changeset/132295> 749269 14 webkit.review.bot 2012-10-23 20:21:26 -0700 All reviewed patches have been landed. Closing bug. 170227 2012-10-23 13:45:52 -0700 2012-10-23 17:10:43 -0700 First pass 99102.1.patch text/plain 1877 pdr SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDEzMjI1OCkKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE5IEBACisyMDEyLTEwLTIzICBQaGlsaXAg Um9nZXJzICA8cGRyQGdvb2dsZS5jb20+CisKKyAgICAgICAgQWRkIGNoZWNrIGZvciByYXcgZGF0 YSBpbiBQYWdlU2VyaWFsaXplcgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93 X2J1Zy5jZ2k/aWQ9OTkxMDIKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4K KworICAgICAgICBUaGUgaW1hZ2UgcmV0dXJuZWQgZnJvbSBpbWFnZUZvclJlbmRlcmVyKCkgZG9l cyBub3QgY29udGFpbiB0aGUgcmF3IFNWRyBkYXRhCisgICAgICAgIHNvIHRoaXMgcGF0Y2ggYWRk cyBhIGNoZWNrIGZvciBpbWFnZS0+aW1hZ2UoKS0+ZGF0YSgpIGJlZm9yZSB3cml0aW5nIFNWRwor ICAgICAgICBpbiBQYWdlU2VyaWFsaXplcjo6YWRkSW1hZ2VUb1Jlc291cmNlcy4KKworICAgICAg ICBDb3ZlcmVkIGJ5IGV4aXN0aW5nIHRlc3QgV2ViUGFnZU5ld1NlcmlhbGl6ZVRlc3QuU1ZHSW1h Z2VEb250Q3Jhc2guCisKKyAgICAgICAgKiBwYWdlL1BhZ2VTZXJpYWxpemVyLmNwcDoKKyAgICAg ICAgKFdlYkNvcmU6OlBhZ2VTZXJpYWxpemVyOjphZGRJbWFnZVRvUmVzb3VyY2VzKToKKwogMjAx Mi0xMC0yMyAgQWxlYyBGbGV0dCAgPGFsZWNmbGV0dEBjaHJvbWl1bS5vcmc+CiAKICAgICAgICAg SW5kZXhlZERCOiByZWZhY3RvciBiYWNrZW5kIHRvIHVzZSBJREIqTWV0YWRhdGEKSW5kZXg6IFNv dXJjZS9XZWJDb3JlL3BhZ2UvUGFnZVNlcmlhbGl6ZXIuY3BwCj09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJj ZS9XZWJDb3JlL3BhZ2UvUGFnZVNlcmlhbGl6ZXIuY3BwCShyZXZpc2lvbiAxMzIyMTUpCisrKyBT b3VyY2UvV2ViQ29yZS9wYWdlL1BhZ2VTZXJpYWxpemVyLmNwcAkod29ya2luZyBjb3B5KQpAQCAt Mjk4LDEyICsyOTgsMTAgQEAgdm9pZCBQYWdlU2VyaWFsaXplcjo6YWRkSW1hZ2VUb1Jlc291cmNl cwogICAgIGlmICghaW1hZ2UgfHwgaW1hZ2UtPmltYWdlKCkgPT0gSW1hZ2U6Om51bGxJbWFnZSgp KQogICAgICAgICByZXR1cm47CiAKLSAgICBSZWZQdHI8U2hhcmVkQnVmZmVyPiBkYXRhID0gaW1h Z2VSZW5kZXJlciA/IGltYWdlLT5pbWFnZUZvclJlbmRlcmVyKGltYWdlUmVuZGVyZXIpLT5kYXRh KCkgOiBpbWFnZS0+aW1hZ2UoKS0+ZGF0YSgpOwotICAgIGlmICghZGF0YSkgewotICAgICAgICAv LyBTVkcgaW1hZ2VzIGRvbid0IHJldHVybiBkYXRhIGF0IHRoaXMgcG9pbnQuIEJ1ZyA5OTEwMi4K LSAgICAgICAgTE9HX0VSUk9SKCJObyBkYXRhIGZvciBpbWFnZSAlcyIsIHVybC5zdHJpbmcoKS51 dGY4KCkuZGF0YSgpKTsKLSAgICAgICAgcmV0dXJuOwotICAgIH0KKyAgICBSZWZQdHI8U2hhcmVk QnVmZmVyPiBkYXRhID0gaW1hZ2VSZW5kZXJlciA/IGltYWdlLT5pbWFnZUZvclJlbmRlcmVyKGlt YWdlUmVuZGVyZXIpLT5kYXRhKCkgOiAwOworICAgIGlmICghZGF0YSkKKyAgICAgICAgZGF0YSA9 IGltYWdlLT5pbWFnZSgpLT5kYXRhKCk7CisKICAgICBTdHJpbmcgbWltZVR5cGUgPSBpbWFnZS0+ cmVzcG9uc2UoKS5taW1lVHlwZSgpOwogICAgIG1fcmVzb3VyY2VzLT5hcHBlbmQoUmVzb3VyY2Uo dXJsLCBtaW1lVHlwZSwgZGF0YSkpOwogICAgIG1fcmVzb3VyY2VVUkxzLmFkZCh1cmwpOwo= 170271 2012-10-23 17:10:43 -0700 2012-10-23 17:30:00 -0700 Add back null check per Jay's request 99102.2.patch text/plain 1884 pdr SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDEzMjI1OCkKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE5IEBACisyMDEyLTEwLTIzICBQaGlsaXAg Um9nZXJzICA8cGRyQGdvb2dsZS5jb20+CisKKyAgICAgICAgQWRkIGV4dHJhIGNoZWNrIGZvciBk YXRhKCkgaW4gUGFnZVNlcmlhbGl6ZXIuCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3Jn L3Nob3dfYnVnLmNnaT9pZD05OTEwMgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09Q UyEpLgorCisgICAgICAgIFRoZSBpbWFnZSByZXR1cm5lZCBmcm9tIGltYWdlRm9yUmVuZGVyZXIo KSBkb2VzIG5vdCBjb250YWluIHRoZSByYXcgU1ZHIGRhdGEKKyAgICAgICAgc28gdGhpcyBwYXRj aCBhZGRzIGEgY2hlY2sgZm9yIGltYWdlLT5pbWFnZSgpLT5kYXRhKCkgYmVmb3JlIHdyaXRpbmcg U1ZHCisgICAgICAgIGluIFBhZ2VTZXJpYWxpemVyOjphZGRJbWFnZVRvUmVzb3VyY2VzLgorCisg ICAgICAgIENvdmVyZWQgYnkgZXhpc3RpbmcgdGVzdCBXZWJQYWdlTmV3U2VyaWFsaXplVGVzdC5T VkdJbWFnZURvbnRDcmFzaC4KKworICAgICAgICAqIHBhZ2UvUGFnZVNlcmlhbGl6ZXIuY3BwOgor ICAgICAgICAoV2ViQ29yZTo6UGFnZVNlcmlhbGl6ZXI6OmFkZEltYWdlVG9SZXNvdXJjZXMpOgor CiAyMDEyLTEwLTIzICBBbGVjIEZsZXR0ICA8YWxlY2ZsZXR0QGNocm9taXVtLm9yZz4KIAogICAg ICAgICBJbmRleGVkREI6IHJlZmFjdG9yIGJhY2tlbmQgdG8gdXNlIElEQipNZXRhZGF0YQpJbmRl eDogU291cmNlL1dlYkNvcmUvcGFnZS9QYWdlU2VyaWFsaXplci5jcHAKPT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0g U291cmNlL1dlYkNvcmUvcGFnZS9QYWdlU2VyaWFsaXplci5jcHAJKHJldmlzaW9uIDEzMjIxNSkK KysrIFNvdXJjZS9XZWJDb3JlL3BhZ2UvUGFnZVNlcmlhbGl6ZXIuY3BwCSh3b3JraW5nIGNvcHkp CkBAIC0yOTgsMTIgKzI5OCwxNSBAQCB2b2lkIFBhZ2VTZXJpYWxpemVyOjphZGRJbWFnZVRvUmVz b3VyY2VzCiAgICAgaWYgKCFpbWFnZSB8fCBpbWFnZS0+aW1hZ2UoKSA9PSBJbWFnZTo6bnVsbElt YWdlKCkpCiAgICAgICAgIHJldHVybjsKIAotICAgIFJlZlB0cjxTaGFyZWRCdWZmZXI+IGRhdGEg PSBpbWFnZVJlbmRlcmVyID8gaW1hZ2UtPmltYWdlRm9yUmVuZGVyZXIoaW1hZ2VSZW5kZXJlcikt PmRhdGEoKSA6IGltYWdlLT5pbWFnZSgpLT5kYXRhKCk7CisgICAgUmVmUHRyPFNoYXJlZEJ1ZmZl cj4gZGF0YSA9IGltYWdlUmVuZGVyZXIgPyBpbWFnZS0+aW1hZ2VGb3JSZW5kZXJlcihpbWFnZVJl bmRlcmVyKS0+ZGF0YSgpIDogMDsKKyAgICBpZiAoIWRhdGEpCisgICAgICAgIGRhdGEgPSBpbWFn ZS0+aW1hZ2UoKS0+ZGF0YSgpOworCiAgICAgaWYgKCFkYXRhKSB7Ci0gICAgICAgIC8vIFNWRyBp bWFnZXMgZG9uJ3QgcmV0dXJuIGRhdGEgYXQgdGhpcyBwb2ludC4gQnVnIDk5MTAyLgogICAgICAg ICBMT0dfRVJST1IoIk5vIGRhdGEgZm9yIGltYWdlICVzIiwgdXJsLnN0cmluZygpLnV0ZjgoKS5k YXRhKCkpOwogICAgICAgICByZXR1cm47CiAgICAgfQorCiAgICAgU3RyaW5nIG1pbWVUeXBlID0g aW1hZ2UtPnJlc3BvbnNlKCkubWltZVR5cGUoKTsKICAgICBtX3Jlc291cmNlcy0+YXBwZW5kKFJl c291cmNlKHVybCwgbWltZVR5cGUsIGRhdGEpKTsKICAgICBtX3Jlc291cmNlVVJMcy5hZGQodXJs KTsK 170276 2012-10-23 17:30:00 -0700 2012-10-23 20:21:23 -0700 Updating reviewer 99102.3.patch text/plain 1881 pdr SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDEzMjI1OCkKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE5IEBACisyMDEyLTEwLTIzICBQaGlsaXAg Um9nZXJzICA8cGRyQGdvb2dsZS5jb20+CisKKyAgICAgICAgQWRkIGV4dHJhIGNoZWNrIGZvciBk YXRhKCkgaW4gUGFnZVNlcmlhbGl6ZXIuCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3Jn L3Nob3dfYnVnLmNnaT9pZD05OTEwMgorCisgICAgICAgIFJldmlld2VkIGJ5IEVyaWMgU2VpZGVs LgorCisgICAgICAgIFRoZSBpbWFnZSByZXR1cm5lZCBmcm9tIGltYWdlRm9yUmVuZGVyZXIoKSBk b2VzIG5vdCBjb250YWluIHRoZSByYXcgU1ZHIGRhdGEKKyAgICAgICAgc28gdGhpcyBwYXRjaCBh ZGRzIGEgY2hlY2sgZm9yIGltYWdlLT5pbWFnZSgpLT5kYXRhKCkgYmVmb3JlIHdyaXRpbmcgU1ZH CisgICAgICAgIGluIFBhZ2VTZXJpYWxpemVyOjphZGRJbWFnZVRvUmVzb3VyY2VzLgorCisgICAg ICAgIENvdmVyZWQgYnkgZXhpc3RpbmcgdGVzdCBXZWJQYWdlTmV3U2VyaWFsaXplVGVzdC5TVkdJ bWFnZURvbnRDcmFzaC4KKworICAgICAgICAqIHBhZ2UvUGFnZVNlcmlhbGl6ZXIuY3BwOgorICAg ICAgICAoV2ViQ29yZTo6UGFnZVNlcmlhbGl6ZXI6OmFkZEltYWdlVG9SZXNvdXJjZXMpOgorCiAy MDEyLTEwLTIzICBBbGVjIEZsZXR0ICA8YWxlY2ZsZXR0QGNocm9taXVtLm9yZz4KIAogICAgICAg ICBJbmRleGVkREI6IHJlZmFjdG9yIGJhY2tlbmQgdG8gdXNlIElEQipNZXRhZGF0YQpJbmRleDog U291cmNlL1dlYkNvcmUvcGFnZS9QYWdlU2VyaWFsaXplci5jcHAKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL1dlYkNvcmUvcGFnZS9QYWdlU2VyaWFsaXplci5jcHAJKHJldmlzaW9uIDEzMjIxNSkKKysr IFNvdXJjZS9XZWJDb3JlL3BhZ2UvUGFnZVNlcmlhbGl6ZXIuY3BwCSh3b3JraW5nIGNvcHkpCkBA IC0yOTgsMTIgKzI5OCwxNSBAQCB2b2lkIFBhZ2VTZXJpYWxpemVyOjphZGRJbWFnZVRvUmVzb3Vy Y2VzCiAgICAgaWYgKCFpbWFnZSB8fCBpbWFnZS0+aW1hZ2UoKSA9PSBJbWFnZTo6bnVsbEltYWdl KCkpCiAgICAgICAgIHJldHVybjsKIAotICAgIFJlZlB0cjxTaGFyZWRCdWZmZXI+IGRhdGEgPSBp bWFnZVJlbmRlcmVyID8gaW1hZ2UtPmltYWdlRm9yUmVuZGVyZXIoaW1hZ2VSZW5kZXJlciktPmRh dGEoKSA6IGltYWdlLT5pbWFnZSgpLT5kYXRhKCk7CisgICAgUmVmUHRyPFNoYXJlZEJ1ZmZlcj4g ZGF0YSA9IGltYWdlUmVuZGVyZXIgPyBpbWFnZS0+aW1hZ2VGb3JSZW5kZXJlcihpbWFnZVJlbmRl cmVyKS0+ZGF0YSgpIDogMDsKKyAgICBpZiAoIWRhdGEpCisgICAgICAgIGRhdGEgPSBpbWFnZS0+ aW1hZ2UoKS0+ZGF0YSgpOworCiAgICAgaWYgKCFkYXRhKSB7Ci0gICAgICAgIC8vIFNWRyBpbWFn ZXMgZG9uJ3QgcmV0dXJuIGRhdGEgYXQgdGhpcyBwb2ludC4gQnVnIDk5MTAyLgogICAgICAgICBM T0dfRVJST1IoIk5vIGRhdGEgZm9yIGltYWdlICVzIiwgdXJsLnN0cmluZygpLnV0ZjgoKS5kYXRh KCkpOwogICAgICAgICByZXR1cm47CiAgICAgfQorCiAgICAgU3RyaW5nIG1pbWVUeXBlID0gaW1h Z2UtPnJlc3BvbnNlKCkubWltZVR5cGUoKTsKICAgICBtX3Jlc291cmNlcy0+YXBwZW5kKFJlc291 cmNlKHVybCwgbWltZVR5cGUsIGRhdGEpKTsKICAgICBtX3Jlc291cmNlVVJMcy5hZGQodXJsKTsK