RESOLVED FIXED 89785
REGRESSION(r116446): Crash in RenderBoxModelObject::adjustedPositionRelativeToOffsetParent
https://bugs.webkit.org/show_bug.cgi?id=89785
Summary REGRESSION(r116446): Crash in RenderBoxModelObject::adjustedPositionRelativeT...
Julien Chaffraix
Reported 2012-06-22 14:37:08 PDT
Sample backtrace: 0x66f497aa [chrome.dll] - renderboxmodelobject.cpp:526] WebCore::RenderBoxModelObject::adjustedPositionRelativeToOffsetParent(WebCore::FractionalLayoutPoint const &) 0x66f4965c [chrome.dll] - renderbox.cpp:3839] WebCore::RenderBox::offsetLeft() 0x67554103 [chrome.dll] - rendervideo.cpp:317] WebCore::RenderVideo::offsetLeft() 0x672321ef [chrome.dll] - element.cpp:372] WebCore::Element::offsetLeft() 0x6768aad9 [chrome.dll] - v8element.cpp:74] WebCore::ElementV8Internal::offsetLeftAttrGetter 0x66df9893 [chrome.dll] - objects.cc:203] v8::internal::JSObject::GetPropertyWithCallback(v8::internal::Object *,v8::internal::Object *,v8::internal::String *) 0x66c3e9d1 [chrome.dll] - objects.cc:633] v8::internal::Object::GetProperty(v8::internal::Object *,v8::internal::LookupResult *,v8::internal::String *,PropertyAttributes *) 0x66dba41f [chrome.dll] - ic.cc:933] v8::internal::LoadIC::Load(v8::internal::InlineCacheState,v8::internal::Handle<v8::internal::Object>,v8::internal::Handle<v8::internal::String>) I haven't been able to find out how users are hitting the bug. It's also very infrequent which makes it difficult to find a good reproduction. The bug is fairly obvious once you look at the failing line: referencePoint.move(parent()->offsetForColumns(referencePoint)); We need to NULL-check parent() as is done in offsetParent().
Attachments
Proposed fix. NULL-check parent. (4.91 KB, patch)
2012-06-22 15:20 PDT, Julien Chaffraix
no flags
Patch for landing (4.52 KB, patch)
2012-06-22 16:02 PDT, Julien Chaffraix
no flags
Julien Chaffraix
Comment 1 2012-06-22 15:20:56 PDT
Created attachment 149119 [details] Proposed fix. NULL-check parent.
Abhishek Arya
Comment 2 2012-06-22 15:32:41 PDT
Comment on attachment 149119 [details] Proposed fix. NULL-check parent. View in context: https://bugs.webkit.org/attachment.cgi?id=149119&action=review > LayoutTests/fullscreen/full-screen-crash-offsetLeft.html:16 > + if (window.testRunner) { these four lines are already provided by full-screen-test.js. no need of them here. > LayoutTests/fullscreen/full-screen-crash-offsetLeft.html:28 > + if (window.testRunner) nit: you can just call endTest() function in full-screen-test.js > LayoutTests/fullscreen/full-screen-crash-offsetLeft.html:37 > + if (window.eventSender) nit: you can use the runKeyDown function if you want.
Tony Chang
Comment 4 2012-06-22 15:46:11 PDT
Comment on attachment 149119 [details] Proposed fix. NULL-check parent. View in context: https://bugs.webkit.org/attachment.cgi?id=149119&action=review > LayoutTests/fullscreen/full-screen-crash-offsetLeft.html:5 > + font-family: ahem; > + -webkit-font-smoothing: none; > + visibility: hidden; Nit: Are these styles needed?
Julien Chaffraix
Comment 5 2012-06-22 16:02:38 PDT
Created attachment 149127 [details] Patch for landing
Julien Chaffraix
Comment 6 2012-06-22 16:05:01 PDT
Comment on attachment 149119 [details] Proposed fix. NULL-check parent. View in context: https://bugs.webkit.org/attachment.cgi?id=149119&action=review I ended up rewriting the test to use the helpers. The output should be way better. You can have a look at the new patch to tell me if you are satisfied. Thanks for the review, Abhishek and Tony! >> LayoutTests/fullscreen/full-screen-crash-offsetLeft.html:5 >> + visibility: hidden; > > Nit: Are these styles needed? Removed. I thought they were needed but not really.
WebKit Review Bot
Comment 7 2012-06-22 16:29:49 PDT
Comment on attachment 149127 [details] Patch for landing Clearing flags on attachment: 149127 Committed r121072: <http://trac.webkit.org/changeset/121072>
WebKit Review Bot
Comment 8 2012-06-22 16:30:09 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.