WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
68099
[V8] Null v8::Context crash in V8DOMWindowShell::namedItemAdded()
https://bugs.webkit.org/show_bug.cgi?id=68099
Summary
[V8] Null v8::Context crash in V8DOMWindowShell::namedItemAdded()
Nate Chapin
Reported
2011-09-14 11:55:43 PDT
Original report at
http://code.google.com/p/chromium/issues/detail?id=96073
. For reasons that aren't clear, V8DOMWindowShell::namedItemAdded() is calling initContextIfNeeded(), but no v8::Context is getting initialized. We then crash when we try to enter the null context. We don't no why this is happening, so to start with we're going to see if it works to just exit early on null context. In addition, I'm going to include a bunch of INC_STATS logging to see if we can get some more data on what exactly is going wrong.
Attachments
patch
(1.90 KB, patch)
2011-09-14 13:19 PDT
,
Nate Chapin
webkit.review.bot
: commit-queue-
Details
Formatted Diff
Diff
patch - fixed inverted logic and attempted cr-mac build fix
(2.06 KB, patch)
2011-09-14 15:34 PDT
,
Nate Chapin
no flags
Details
Formatted Diff
Diff
Log where exactly the context initialization failed, use histograms
(5.53 KB, patch)
2011-09-29 11:41 PDT
,
Nate Chapin
no flags
Details
Formatted Diff
Diff
Remove logging and always check the result of initContextIfNeeded()
(8.00 KB, patch)
2011-10-04 13:53 PDT
,
Nate Chapin
abarth
: review+
webkit.review.bot
: commit-queue-
Details
Formatted Diff
Diff
Archive of layout-test-results from ec2-cr-linux-03
(32.55 KB, application/zip)
2011-10-04 14:24 PDT
,
WebKit Review Bot
no flags
Details
Archive of layout-test-results from ec2-cq-03
(32.55 KB, application/zip)
2011-10-04 15:56 PDT
,
WebKit Review Bot
no flags
Details
missed a case in CodeGeneratorV8.pm
(8.78 KB, patch)
2011-10-05 10:33 PDT
,
Nate Chapin
no flags
Details
Formatted Diff
Diff
Fix my mistakes in CodeGeneratorV8.pm
(8.80 KB, patch)
2011-10-10 15:14 PDT
,
Nate Chapin
no flags
Details
Formatted Diff
Diff
Show Obsolete
(7)
View All
Add attachment
proposed patch, testcase, etc.
Nate Chapin
Comment 1
2011-09-14 13:19:35 PDT
Created
attachment 107381
[details]
patch
WebKit Review Bot
Comment 2
2011-09-14 13:42:22 PDT
Comment on
attachment 107381
[details]
patch
Attachment 107381
[details]
did not pass cr-mac-ews (chromium): Output:
http://queues.webkit.org/results/9658674
Nate Chapin
Comment 3
2011-09-14 15:34:52 PDT
Created
attachment 107409
[details]
patch - fixed inverted logic and attempted cr-mac build fix
Adam Barth
Comment 4
2011-09-14 16:33:04 PDT
Comment on
attachment 107409
[details]
patch - fixed inverted logic and attempted cr-mac build fix View in context:
https://bugs.webkit.org/attachment.cgi?id=107409&action=review
> Source/WebCore/bindings/v8/V8DOMWindowShell.cpp:574 > + // FIXME: Temporary diagnostics as to why V8 sometimes crashes with a null context below.
Technically we should either skip the FIXME or re-word this to explain when we should remove this code.
WebKit Review Bot
Comment 5
2011-09-14 22:11:58 PDT
Comment on
attachment 107409
[details]
patch - fixed inverted logic and attempted cr-mac build fix Clearing flags on attachment: 107409 Committed
r95166
: <
http://trac.webkit.org/changeset/95166
>
WebKit Review Bot
Comment 6
2011-09-14 22:12:02 PDT
All reviewed patches have been landed. Closing bug.
Nate Chapin
Comment 7
2011-09-29 11:40:01 PDT
This got auto-closed by review bot....reopening and new logging patch incoming.
Nate Chapin
Comment 8
2011-09-29 11:41:07 PDT
Created
attachment 109180
[details]
Log where exactly the context initialization failed, use histograms
WebKit Review Bot
Comment 9
2011-09-29 12:45:09 PDT
Comment on
attachment 109180
[details]
Log where exactly the context initialization failed, use histograms Clearing flags on attachment: 109180 Committed
r96349
: <
http://trac.webkit.org/changeset/96349
>
WebKit Review Bot
Comment 10
2011-09-29 12:45:14 PDT
All reviewed patches have been landed. Closing bug.
Nate Chapin
Comment 11
2011-09-29 12:47:14 PDT
(In reply to
comment #10
)
> All reviewed patches have been landed. Closing bug.
Reopening what reviewbot closed (again).
Nate Chapin
Comment 12
2011-10-04 13:53:46 PDT
Created
attachment 109678
[details]
Remove logging and always check the result of initContextIfNeeded()
WebKit Review Bot
Comment 13
2011-10-04 14:23:58 PDT
Comment on
attachment 109678
[details]
Remove logging and always check the result of initContextIfNeeded()
Attachment 109678
[details]
did not pass chromium-ews (chromium-xvfb): Output:
http://queues.webkit.org/results/9942474
New failing tests: css1/basic/comments.html css1/basic/grouping.html canvas/philip/tests/2d.canvas.readonly.html http/tests/appcache/access-via-redirect.php http/tests/appcache/auth.html animations/animation-css-rule-types.html css1/basic/contextual_selectors.html animations/animation-direction.html http/tests/appcache/cyrillic-uri.html css1/basic/containment.html http/tests/appcache/credential-url.html http/tests/appcache/crash-when-navigating-away-then-back.html animations/animation-add-events-in-handler.html animations/animation-controller-drt-api.html canvas/philip/tests/2d.canvas.reference.html canvas/philip/tests/2d.clearRect+fillRect.basic.html css1/basic/class_as_selector.html canvas/philip/tests/2d.clearRect+fillRect.alpha0.5.html animations/animation-direction-normal.html canvas/philip/tests/2d.clearRect+fillRect.alpha0.html
WebKit Review Bot
Comment 14
2011-10-04 14:24:01 PDT
Created
attachment 109689
[details]
Archive of layout-test-results from ec2-cr-linux-03 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: ec2-cr-linux-03 Port: Chromium Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Adam Barth
Comment 15
2011-10-04 15:30:47 PDT
Comment on
attachment 109678
[details]
Remove logging and always check the result of initContextIfNeeded() This looks good. I'm not sure why the bot is complaining about all those test failures.
Nate Chapin
Comment 16
2011-10-04 15:34:26 PDT
Comment on
attachment 109678
[details]
Remove logging and always check the result of initContextIfNeeded() Yeah, it's really weird. I'm just gonna cq+ it and see if it happens again, since this isn't happening locally for me.
WebKit Review Bot
Comment 17
2011-10-04 15:56:08 PDT
Comment on
attachment 109678
[details]
Remove logging and always check the result of initContextIfNeeded() Rejecting
attachment 109678
[details]
from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: arRect+fillRect.basic.html = CRASH css1/basic/class_as_selector.html = CRASH css1/basic/comments.html = CRASH css1/basic/containment.html = CRASH css1/basic/contextual_selectors.html = CRASH css1/basic/grouping.html = CRASH http/tests/appcache/access-via-redirect.php = CRASH http/tests/appcache/auth.html = CRASH http/tests/appcache/crash-when-navigating-away-then-back.html = CRASH http/tests/appcache/credential-url.html = CRASH http/tests/appcache/cyrillic-uri.html = CRASH Full output:
http://queues.webkit.org/results/9954218
WebKit Review Bot
Comment 18
2011-10-04 15:56:11 PDT
Created
attachment 109708
[details]
Archive of layout-test-results from ec2-cq-03 The attached test failures were seen while running run-webkit-tests on the commit-queue. Bot: ec2-cq-03 Port: Chromium Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Nate Chapin
Comment 19
2011-10-05 10:33:08 PDT
Created
attachment 109819
[details]
missed a case in CodeGeneratorV8.pm The case in CodeGeneratorV8.pm apparently depended on initContextIfNeeded() returning false if a context already existed. Just checking for a context before calling initContextIfNeeded() should be sufficient in that case.
Adam Barth
Comment 20
2011-10-09 14:38:36 PDT
Comment on
attachment 109819
[details]
missed a case in CodeGeneratorV8.pm This patch is causing the cr-linux EWS to spin.
Nate Chapin
Comment 21
2011-10-10 15:14:53 PDT
Created
attachment 110413
[details]
Fix my mistakes in CodeGeneratorV8.pm Instead of only calling initContextIfNeeded() if it was actually needed in CodeGeneratorV8.pm, I just called it a different way. Silly me.
Adam Barth
Comment 22
2011-10-12 09:36:12 PDT
Comment on
attachment 110413
[details]
Fix my mistakes in CodeGeneratorV8.pm View in context:
https://bugs.webkit.org/attachment.cgi?id=110413&action=review
> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:2711 > - if (proxy->windowShell()->initContextIfNeeded()) { > + if (proxy->windowShell()->context().IsEmpty() && proxy->windowShell()->initContextIfNeeded()) {
Do you need to run-bindings-tests?
Nate Chapin
Comment 23
2011-10-12 09:41:44 PDT
Comment on
attachment 110413
[details]
Fix my mistakes in CodeGeneratorV8.pm Looks like run-bindings-tests still passes.
WebKit Review Bot
Comment 24
2011-10-12 11:44:55 PDT
Comment on
attachment 110413
[details]
Fix my mistakes in CodeGeneratorV8.pm Clearing flags on attachment: 110413 Committed
r97280
: <
http://trac.webkit.org/changeset/97280
>
WebKit Review Bot
Comment 25
2011-10-12 11:45:01 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