Closed Bug 1058043 Opened 10 years ago Closed 10 years ago

ServiceWorkerRegistration should not keep a reference to the window

Categories

(Core :: DOM: Core & HTML, defect)

Product:
Core
Component:
DOM: Core & HTML
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla34

People

(Reporter: baku, Assigned: nsm)

References

Details

Attachments

(3 files, 2 obsolete files)

deth.patch
7.49 KB, patch
Details | Diff | Splinter Review
Patch 2 - StopListeningForEvents in destructor
4.27 KB, patch
baku
: review+
Details | Diff | Splinter Review
patch 3 - ServiceWorkerContainer
1.95 KB, patch
nsm
: review+
Details | Diff | Splinter Review 
      No description provided.
Attached patch deth.patch (obsolete) — — Splinter Review 

        Attachment #8478276 -
        Flags: review?(nsm.nikhil)

        Attachment #8478276 -
        Flags: review?(nsm.nikhil) → review+

    
    

    
  

      
      
      
      

        Attached patch
          
          
        deth.patch (obsolete)
        — 
          — Splinter Review
      

    


  
https://tbpl.mozilla.org/?tree=Try&rev=42b606e75e62

        Attachment #8478276 -
        Attachment is obsolete: true

    
  
Keywords: checkin-needed

    
    

    
  

      
      
      
      

        Attached patch
          
          
        deth.patch
        — 
          — Splinter Review
      

    


  
rebased

        Attachment #8478501 -
        Attachment is obsolete: true

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/06168844e276
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34

    
    

    
  
StopListeningForEvents() should be called in the dtor too (with appropriate checks), in case the registration is GCed before the window goes away.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee: amarchesini → nsm.nikhil
Status: REOPENED → ASSIGNED

    
    

    
  
Comment on attachment 8480111 [details] [diff] [review]
Patch 2 - StopListeningForEvents in destructor

Review of attachment 8480111 [details] [diff] [review]:
-----------------------------------------------------------------

We wrote the same patch :)
We should call RemoveReadyPromise() in the dtor of ServiceWorkerContainer as well.

        Attachment #8480111 -
        Flags: review?(amarchesini) → review+

    
    

    
  


  

        Attachment #8480465 -
        Flags: review?(nsm.nikhil)

    
    

    
  
Comment on attachment 8480465 [details] [diff] [review]
patch 3 - ServiceWorkerContainer

Review of attachment 8480465 [details] [diff] [review]:
-----------------------------------------------------------------

Shouldn't this patch be attached to the .ready bug? Please update the commit message when landing.

        Attachment #8480465 -
        Flags: review?(nsm.nikhil) → review+

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/b7d78e4bddde
Status: ASSIGNED → RESOLVED
Closed: 10 years ago → 10 years ago
Resolution: --- → FIXED
Component: DOM → DOM: Core & HTML

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: