WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
Patch for landing
(4.52 KB, patch)
2012-06-22 16:02 PDT
,
Julien Chaffraix
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
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.
Abhishek Arya
Comment 3
2012-06-22 15:34:01 PDT
typo it is runWithKeyDown (
http://code.google.com/searchframe#OAMlx_jo-ck/src/third_party/WebKit/LayoutTests/fullscreen/full-screen-test.js&exact_package=chromium&q=runWithKeyDown&type=cs&l=12
)
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.
Top of Page
Format For Printing
XML
Clone This Bug