| Summary: | A mistake in WebCore::JavaScriptCallFrame::evaluate which will cause assert failed | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Peter Wang <PeterHWang> | ||||||
| Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | charles.wei, fpizlo, ggaren, mark.lam, mhahnenberg, oliver, timothy, webkit.review.bot, yurys | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Bug Depends on: | 99801 | ||||||||
| Bug Blocks: | |||||||||
| Attachments: |
|
||||||||
Created attachment 170581 [details]
Patch
it comes from <http://trac.webkit.org/changeset/121381>, a small neglect. 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. (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. (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. Created attachment 170781 [details]
Patch
Comment on attachment 170781 [details] Patch correct a mistake in ChangeLog according comment#5 and land this patch. 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 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 on attachment 170781 [details] Patch Clearing flags on attachment: 170781 Committed r132573: <http://trac.webkit.org/changeset/132573> All reviewed patches have been landed. Closing bug. 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. |
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.