Bug 115587

Summary: No need to declare JSC::LLInt::Data as friend class of WTF::StringImpl
Product: WebKit Reporter: Xan Lopez <xan.lopez>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: REOPENED    
Severity: Normal CC: benjamin, cmarcelo, commit-queue, ggaren, kling, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
stringimplllintdata.diff none

Xan Lopez
Reported 2013-05-04 08:14:47 PDT
I suppose LLInt::Data used some private method/data from StringImpl in the past, but that's not the case anymore. So remove the (bogus) layering violation.
Attachments
stringimplllintdata.diff (2.13 KB, patch)
2013-05-04 08:18 PDT, Xan Lopez 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Xan Lopez
Comment 1 2013-05-04 08:18:16 PDT
Created attachment 200528 [details] stringimplllintdata.diff
WebKit Commit Bot
Comment 2 2013-05-04 12:34:24 PDT
The commit-queue encountered the following flaky tests while processing attachment 200528 [details]: fast/frames/crash-remove-iframe-during-object-beforeload.html bug 115322 (author: zalan@apple.com) The commit-queue is continuing to process your patch.
WebKit Commit Bot
Comment 3 2013-05-04 12:35:45 PDT
Comment on attachment 200528 [details] stringimplllintdata.diff Clearing flags on attachment: 200528 Committed r149563: <http://trac.webkit.org/changeset/149563>
WebKit Commit Bot
Comment 4 2013-05-04 12:35:47 PDT
All reviewed patches have been landed. Closing bug.
Andreas Kling
Comment 5 2013-05-04 13:28:39 PDT
Reverted r149563 for reason: Broke LLInt build. Committed r149568: <http://trac.webkit.org/changeset/149568>
Xan Lopez
Comment 6 2013-05-04 13:48:57 PDT
(In reply to comment #5) > Reverted r149563 for reason: > > Broke LLInt build. > > Committed r149568: <http://trac.webkit.org/changeset/149568> Right, there was an ASSERT accessing a private flag, missed that (like EWS). We can change it to use the public getter, will upload a new patch later when I can verify with a debug build.
Geoffrey Garen
Comment 7 2013-05-04 13:52:10 PDT
Can someone svn blame to find out when this interface was added, and when its use was removed? If the use was truly removed, we can remove the ASSERT. Otherwise, we probably can't remove the interface.
Note You need to log in before you can comment on or make changes to this bug.