Closed Bug 1024170 Opened 10 years ago Closed 10 years ago

JSCompartment wrap fails to wrap string on worker

Categories

(Core :: JavaScript Engine, defect)

Product:
Core
Component:
JavaScript Engine
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla33
Tracking Flags:
Tracking Status
firefox30 --- wontfix
firefox31 + fixed
firefox32 + fixed
firefox33 --- fixed
b2g-v2.0 --- fixed
b2g-v2.1 --- fixed

People

(Reporter: seward.zheng, Assigned: seward.zheng)

References

Details

(Whiteboard: [qa-])

Attachments

(1 file)

Patch
1.32 KB, patch
bhackett1024
: review+
Sylvestre
: approval-mozilla-aurora+
Sylvestre
: approval-mozilla-beta+
Details | Diff | Splinter Review 
js/src/jscompartment.cpp:276

JSCompartment calls str->zone(), which requires the zone to have exclusive access. This is in fact unnecessary.
Attached patch Patch — — Splinter Review 
Assignee: nobody → szheng
Status: NEW → ASSIGNED

        Attachment #8438796 -
        Flags: review?(bhackett1024)

        Attachment #8438796 -
        Flags: review?(bhackett1024) → review+

    
    

    
  
Shihua, this will need a full try run also.
Flags: needinfo?(szheng)

    
    

    
  
(In reply to ben turner [:bent] (use the needinfo? flag!) from comment #2)
> Shihua, this will need a full try run also.

Try: https://tbpl.mozilla.org/?tree=Try&rev=5165684548a3
Flags: needinfo?(szheng)

    
  
Blocks: 901097

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/8ed54f8585b2
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Blocks: 964059

    
    

    
  
This needs backporting to branches, no?  Please request approvals.

          status-firefox30:
          --- → wontfix

          status-firefox31:
          --- → affected

          status-firefox32:
          --- → affected

          tracking-firefox31:
          --- → ?

          tracking-firefox32:
          --- → ?
Flags: needinfo?(szheng)

    
    

    
  
Comment on attachment 8438796 [details] [diff] [review]
Patch

Approval Request Comment
[Feature/regressing bug #]: 600402
[User impact if declined]: Have unexpected failures when dealing with strings on web workers
[Describe test coverage new/current, TBPL]: https://tbpl.mozilla.org/?tree=Try&rev=5165684548a3 , passed
[Risks and why]: Low
[String/UUID change made/needed]: None

        Attachment #8438796 -
        Flags: approval-mozilla-beta?

        Attachment #8438796 -
        Flags: approval-mozilla-aurora?
Flags: needinfo?(szheng)

    
    

    
  
Looking at bug 600402, I can see that the bug existed for the last 3 years. We are late in the beta cycle, why do you think we should take it?
Flags: needinfo?(szheng)

    
    

    
  
Note that this blocks bug 1027221 from being uplifted as well.
Blocks: 1027221

    
    

    
  
(In reply to Sylvestre Ledru [:sylvestre] from comment #8)
> Looking at bug 600402, I can see that the bug existed for the last 3 years.
> We are late in the beta cycle, why do you think we should take it?

This bug, as well as bug 1027221, blocks worker features from shipping. It will be very helpful to get them landed in beta channel early.

In addition, the stability is not expected to be impaired by these patches.
Flags: needinfo?(szheng)

        Attachment #8438796 -
        Flags: approval-mozilla-beta?

        Attachment #8438796 -
        Flags: approval-mozilla-beta+

        Attachment #8438796 -
        Flags: approval-mozilla-aurora?

        Attachment #8438796 -
        Flags: approval-mozilla-aurora+

    
    

    
  
Is this something that impacts b2g v1.4 (b2g30) as well? If so, should this be blocking?
Flags: needinfo?(szheng)

    
    

    
  
Tagging [qa-]. Please advise if this needs QA attention before we release.
Whiteboard: [qa-]

    
    

    
  
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #12)
> Is this something that impacts b2g v1.4 (b2g30) as well? If so, should this
> be blocking?

Impact, but nonblocking.
Flags: needinfo?(szheng)

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: