This is a regression from bug 43779, changeset 66223. It has been noticed in Chromium; see the Chromium issue 54972 at http://code.google.com/p/chromium/issues/detail?id=54972 I am submitting a straightforward patch that fixes the crash, although I am not entirely sure this is the right fix. Input from reviewers will be highly appreciated. My understanding of the changeset 66223 is that RenderImage objects must have an ImageResource, so setImageResource must be called either inside the constructor (as it's the case with RenderMedia), or immediately after the constructor (as it's the case with the renderer associated with HTMLImageElement, HTMLInputElement, HTMLObjectElement, etc.). This, however, doesn't work well in the case of RenderObject or RenderObjectChildList: inside RenderObject::createObject, as well as inside RenderObjectChildList::updateBeforeAfterContent, setImageResource is called only after setting the image style. This leads to a crash when the style is zoomed, because RenderImage::setStyle (or more precisely, RenderImage::intrinsicSizeChanged) assumes that an ImageResource object exists. Swapping setStyle and setImageResource doesn't work any better, because setImageResource also assumes that a style exists! I am setting the severity/priority level to Major/P1, because it is crashing Chromium badly. I will submit, shortly, my very simple patch and layout test.
Created attachment 68179 [details] Fix and layout test As I mentioned in the previous comment, perhaps a little ambiguously: Swapping the order of calls to setStyle/setImageResource inside RenderObject::createObject and RenderObjectChildList::updateBeforeAfterContent does not solve the lack of initialization, because initializing a style and initializing an image resource are cyclically dependent on each other. I am, therefore, relying on a simple if-check instead, to avoid this issue altogether.
Comment on attachment 68179 [details] Fix and layout test View in context: https://bugs.webkit.org/attachment.cgi?id=68179&action=review > WebCore/rendering/RenderImage.h:66 > + if (m_imageResource.get()) No need to use .get(). r=me. If you don't have commit access, you'll need to upload a new version, that I can cq+.
Created attachment 68226 [details] Fix and layout test, comply with reviewer's comments
Comment on attachment 68226 [details] Fix and layout test, comply with reviewer's comments Rejecting patch 68226 from commit-queue. Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--exit-after-n-failures=1', '--wait-for-httpd', '--quiet']" exit_code: 1 Running build-dumprendertree Compiling Java tests make: Nothing to be done for `default'. Running tests from /Projects/CommitQueue/LayoutTests Testing 21333 test cases. css3/style-zoomed-image.html -> failed Exiting early after 1 failures. 1581 tests run. 48.40s total testing time 1580 test cases (99%) succeeded 1 test case (<1%) had incorrect layout 1 test case (<1%) had stderr output Full output: http://queues.webkit.org/results/4001076
(In reply to comment #4) Did the layout test fail on Mac? I do not understand why. I am running the trybot, again, to get more details on why it happened. I do not know, at this point, whether it's my fault and it needs more fixing on Mac. The patch is urgently needed on Chromium; should I add an exclusion, for the time being?
The commit queue is saying that this test failed on mac. I'll see if I can repro.
I rerun my patch with the try bots, thinking that I might have done something wrong previously, but I received successful results this time as well. I am really confused by what is happening, and I would really appreciate some help. Is there a way to retry this, e.g. by putting the patch back into the commit queue?
(In reply to comment #7) > I rerun my patch with the try bots, thinking that I might have done something wrong previously, but I received successful results this time as well. I am really confused by what is happening, and I would really appreciate some help. Is there a way to retry this, e.g. by putting the patch back into the commit queue? What platform are you testing on? The failure the commit queue is seeing is on the mac port (i.e. Safari), not chromium-mac.
The failure is due to trailing whitespace differences. I'm guessing you were testing with new-run-webkit-tests, which ignores trailing '\n's, I can fix and land by hand if you'd like.
Created attachment 68320 [details] Fix + layout test + fixed "expected" file James, I'm so grateful for finding this out for me. Thank you! I don't know how I lost the trailing '\n', probably while copying+pasting, and their lack didn't seem to cause any issue in the Chromium test runs. When I resubmitted the job to the trybots, all of them (mac, linux, windows) were happy as well. I already resubmitting the fixed patch, could you please review and commit-queue it? (Or, should I have submitted only the '\n\n' change this time?)
Comment on attachment 68320 [details] Fix + layout test + fixed "expected" file As I said, new-run-webkit-tests (which chromium uses) ignores trailing newline diffs when running tests. run-webkit-tests (which the mac port uses) does not. That's why you did not see a failure but the commit queue did.
Comment on attachment 68320 [details] Fix + layout test + fixed "expected" file Clearing flags on attachment: 68320 Committed r68004: <http://trac.webkit.org/changeset/68004>
All reviewed patches have been landed. Closing bug.