Bug 100347 - A mistake in WebCore::JavaScriptCallFrame::evaluate which will cause assert failed
Summary: A mistake in WebCore::JavaScriptCallFrame::evaluate which will cause assert f...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 99801
Blocks:
  Show dependency treegraph
 
Reported: 2012-10-25 01:10 PDT by Peter Wang
Modified: 2014-04-24 16:45 PDT (History)
9 users (show)

See Also:


Attachments
Patch (1.57 KB, patch)
2012-10-25 01:15 PDT, Peter Wang
no flags Details | Formatted Diff | Diff
Patch (1.60 KB, patch)
2012-10-25 18:36 PDT, Peter Wang
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Peter Wang 2012-10-25 01:10:00 PDT
When WebCore::JavaScriptCallFrame::evaluate is invoked in worker context, this statement
      JSLockHolder lock(JSDOMWindowBase::commonJSGlobalData());
will cause assert, since there is "ASSERT(isMainThread());" in JSDOMWindowBase::commonJSGlobalData.
Comment 1 Peter Wang 2012-10-25 01:15:15 PDT
Created attachment 170581 [details]
Patch
Comment 2 Peter Wang 2012-10-25 03:24:17 PDT
it comes from <http://trac.webkit.org/changeset/121381>, a small neglect.
Comment 3 Mark Lam 2012-10-25 08:26:47 PDT
Comment on attachment 170581 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=170581&action=review

> Source/WebCore/ChangeLog:11
> +        No new test cae.

Typo in this line ("cae" ==> "case").  Just delete the line as it doesn't add any info.

Otherwise, LGTM, but need r+ from someone else.
Comment 4 Filip Pizlo 2012-10-25 08:49:34 PDT
(In reply to comment #3)
> (From update of attachment 170581 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=170581&action=review
> 
> > Source/WebCore/ChangeLog:11
> > +        No new test cae.
> 
> Typo in this line ("cae" ==> "case").  Just delete the line as it doesn't add any info.

In WebCore changelogs we usually have a line about test cases. Peter, please fix the typo before landing. My preference would be to keep this line. 

> 
> Otherwise, LGTM, but need r+ from someone else.
Comment 5 Peter Wang 2012-10-25 18:16:07 PDT
(In reply to comment #4)
> (In reply to comment #3)
> > (From update of attachment 170581 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=170581&action=review
> > 
> > > Source/WebCore/ChangeLog:11
> > > +        No new test cae.
> > 
> > Typo in this line ("cae" ==> "case").  Just delete the line as it doesn't add any info.
> 
> In WebCore changelogs we usually have a line about test cases. Peter, please fix the typo before landing. My preference would be to keep this line. 
> 
> > 
> > Otherwise, LGTM, but need r+ from someone else.

Sorry, thx.
Comment 6 Peter Wang 2012-10-25 18:36:02 PDT
Created attachment 170781 [details]
Patch
Comment 7 Peter Wang 2012-10-25 18:39:04 PDT
Comment on attachment 170781 [details]
Patch

correct a mistake in ChangeLog according comment#5 and land this patch.
Comment 8 WebKit Review Bot 2012-10-25 18:40:23 PDT
Comment on attachment 170781 [details]
Patch

Rejecting attachment 170781 [details] from review queue.

peter.wang@torchmobile.com.cn does not have reviewer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py.

- If you do not have reviewer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.

- If you have reviewer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed).  The commit-queue restarts itself every 2 hours.  After restart the commit-queue will correctly respect your reviewer rights.
Comment 9 WebKit Review Bot 2012-10-25 18:41:05 PDT
Comment on attachment 170781 [details]
Patch

Rejecting attachment 170781 [details] from commit-queue.

peter.wang@torchmobile.com.cn does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py.

- If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.

- If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed).  The commit-queue restarts itself every 2 hours.  After restart the commit-queue will correctly respect your committer rights.
Comment 10 WebKit Review Bot 2012-10-25 22:41:43 PDT
Comment on attachment 170781 [details]
Patch

Clearing flags on attachment: 170781

Committed r132573: <http://trac.webkit.org/changeset/132573>
Comment 11 WebKit Review Bot 2012-10-25 22:41:47 PDT
All reviewed patches have been landed.  Closing bug.
Comment 12 Darin Adler 2014-04-24 16:45:57 PDT
Moving all JavaScriptGlue bugs to JavaScriptCore. The JavaScriptGlue framework itself is long gone. And most of the more recent bugs put in this component were put there by people who thought this was for some other aspect of “JavaScript glue” and have nothing to do with the actual original reason for the existence of this component, which was an OS-X-only framework named JavaScriptGlue.