Move Location to DOMWindow

Source/core/frame/DOMWindow.cpp

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "config.h"
 #include "core/frame/DOMWindow.h"
 
 #include "core/dom/SecurityContext.h"
 #include "core/frame/Frame.h"
 #include "core/frame/FrameClient.h"
+#include "core/frame/Location.h"
 #include "platform/weborigin/KURL.h"
 #include "platform/weborigin/SecurityOrigin.h"
 
 namespace blink {
 
+DOMWindow::~DOMWindow()
+{
+    reset();

[sof, 2015/02/03 21:25:10]

This is problematic in an Oilpan setting, as destructors are not allowed to
touch other heap objects.

For LocalDOMWindow, you could trigger the rest via FrameDestructionObserver's
willDetachFrameHost(). Perhaps RemoteDOMWindow needs to become a
FrameDestructionObserver (of its RemoteFrame)?

[sof, 2015/02/03 21:28:06]

s/rest/reset/g

[Nate Chapin, 2015/02/06 19:39:58]

Resetting the location in willDetachFrameHost() has observable side effects for
Location objects that are being referenced by JS. On trunk, we report the
location's url as about:blank if the location object is detached from its frame.
With reset in willDetachFrameHost(), we report undefined.

FWIW, Firefox returns an empty string in the same situation.

[sof, 2015/02/07 17:09:32]

I see, somewhat similar to changes seen for
https://codereview.chromium.org/881683003/

I'm surprised the change here isn't exercised by existing layout tests; or is it
& outputs haven't been updated?

[Nate Chapin, 2015/02/09 19:05:57]

There was a single layout test that had output changed. I didn't bother to
update the results because I was still debating internally as to whether this
was OK, and if it was, whether we should match Firefox's output rather than the
current default of undefined output.

+}
+
+Location* DOMWindow::location() const
+{
+    if (!m_location)
+        m_location = Location::create(frame());
+    return m_location.get();
+}
+
 bool DOMWindow::closed() const
 {
     return !frame() || !frame()->host();
 }
 
 unsigned DOMWindow::length() const
 {
     return frame() ? frame()->tree().scopedChildCount() : 0;
 }
 
@@ skipping 59 matching lines @@
             return false;
 
         // FIXME: The name canAccess seems to be a roundabout way to ask "can execute script".
         // Can we name the SecurityOrigin function better to make this more clear?
         if (callingWindow.frame()->securityContext()->securityOrigin()->canAccess(frame()->securityContext()->securityOrigin()))
             return false;
     }
     return true;
 }
 
+void DOMWindow::reset()
+{
+    // Location needs to be reset manually because it doesn't inherit from DOMWindowProperty.
+    if (m_location) {
+        m_location->reset();
+        m_location = nullptr;
+    }
+}
+
 } // namespace blink