577979 - Factor out WindowOwnerStillExists bits into a subclass

Status: NEW

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

Description

[Doug Turner (:dougt)]

Attachment: patch v.1

The geolocation code holds onto a content window so that it can determine if the window has been closed before invoking various js callbacks.

When I started working on the desktop notification api, I found that I was writing the same code.  I think each time we extend in a similar fashion, we will want to hold onto the same bits.

This patch basically pulls the required bits to support WindowOwnerStillExists into a base class, and it converts the geolocation call site.

Happy to name it something else given a suggestion.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 456790 [details] [diff] [review]
patch v.1

>+PRBool
>+nsDOMDeviceCenterBase::WindowOwnerStillExists()
>+{
>+  // an owner was never set when was created, which means
>+  // that this object is being used without a window.
>+  if (mOwner == nsnull)
>+    return PR_TRUE;
>+
>+  nsCOMPtr<nsPIDOMWindow> window = do_QueryReferent(mOwner);
>+
>+  if (window)
This method should return PR_FALSE if !window
Seems like it is wrong in the original code.


>+  {
>+    PRBool closed = PR_FALSE;
>+    window->GetClosed(&closed);
>+    if (closed)
>+      return PR_FALSE;
>+
>+    nsPIDOMWindow* outer = window->GetOuterWindow();
>+    if (!outer || outer->GetCurrentInnerWindow() != window)
>+      return PR_FALSE;

You could just do 
return outer && outer->GetCurrentInnerWindow() == window

Comment 2

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1472046

Move all DOM bugs that haven't been updated in more than 3 years and has no one currently assigned to P5.

If you have questions, please contact :mdaly.