Bug 184820 – Use WindowProxy in DOMWindow.idl

• Status: RESOLVED
• Resolution: FIXED
• Component: Bindings

[Chris Dumez]

Use WindowProxy in DOMWindow.idl to match the specification more closely.

[Chris Dumez]

Created attachment 338430 [details]
Patch

[Sam Weinig]

Comment on attachment 338430 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=338430&action=review

> Source/WebCore/page/DOMWindow.cpp:2305
> +WindowProxy* DOMWindow::open(DOMWindow& activeWindow, DOMWindow& firstWindow, const String& urlString, const AtomicString& frameName, const String& windowFeaturesString)

I'm a little worried about this losing it's RefPtr'ness.  I think giving WindowProxy ref() and deref() methods, that just ref/deref the underlying AbstractFrame and making this return a RefPtr<WindowProxy> would be safer. What do you think?

[Chris Dumez]

(In reply to Sam Weinig from comment #2)
> Comment on attachment 338430 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=338430&action=review
> 
> > Source/WebCore/page/DOMWindow.cpp:2305
> > +WindowProxy* DOMWindow::open(DOMWindow& activeWindow, DOMWindow& firstWindow, const String& urlString, const AtomicString& frameName, const String& windowFeaturesString)
> 
> I'm a little worried about this losing it's RefPtr'ness.  I think giving
> WindowProxy ref() and deref() methods, that just ref/deref the underlying
> AbstractFrame and making this return a RefPtr<WindowProxy> would be safer.
> What do you think?

Oh, I had similar concerns but did not want to have WindowProxy subclass RefCounted. Your idea sounds good.

[Chris Dumez]

Created attachment 338463 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 338463 [details]
Patch

Clearing flags on attachment: 338463

Committed r230867: <https://trac.webkit.org/changeset/230867>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/39613880>