Move Location to DOMWindow

Source/core/frame/Location.h

Index: Source/core/frame/Location.h
diff --git a/Source/core/frame/Location.h b/Source/core/frame/Location.h
index 8b6f0a37be2716593c370d99d93ca53099bb4247..d9dda748cca84b28a3bbcf4cec9d620d295edc12 100644
--- a/Source/core/frame/Location.h
+++ b/Source/core/frame/Location.h
@@ -40,18 +40,25 @@ namespace blink {
 
 class LocalDOMWindow;
 class ExceptionState;
-class LocalFrame;
+class Frame;
 class KURL;
 
-class Location final : public RefCountedWillBeGarbageCollected<Location>, public ScriptWrappable, public DOMWindowProperty {
+// This class corresponds to the JS Location API, which is the only DOM API besides Window that is operable
+// in a RemoteFrame. Rather than making DOMWindowProperty support RemoteFrames and generating a lot
+// code churn, Location is implemented as a one-off with some custom lifetime management code. Namely,
+// it needs a manual call to reset() from DOMWindow::reset() to ensure it doesn't retain a stale Frame pointer.

[dcheng, 2015/02/02 21:50:36]

Would we still need reset() with Oilpan? I guess we wouldn't want a cached JS
reference to location to keep Frame alive. But we should get that for free with
WeakMember in an Oilpan build, right?

haraken@, sigbjornf@: is the usual convention to wrap these sort of things in
#if !ENABLE(OILPAN)?

[sof, 2015/02/02 22:53:04]

We normally keep such Oilpan-specific dispose/reset/... smaller methods without
wrapping #if-ery around them.

WeakMembers would take care of it wrt safety and not prolonging lifetime, but
you don't want to expose GC behavior&timing to the outside world, if possible.
https://codereview.chromium.org/881683003/ is an example of the difference in
behavior that might arise if you do rely on WeakMembers in this area.

Without having looked at details here, but for Oilpan, I think you want to keep
a WeakMember and try to find an effective place for the reset() call
(DOMWindow::reset() isn't called as part of frame destruction/finalization with
Oilpan.)

[haraken, 2015/02/03 00:56:09]

Right, in order not to make GC's behavior observable, it is important to clear
it explicitly.

[Nate Chapin, 2015/02/03 19:17:21]

I added an unconditional reset() to ~DOMWindow. I think this does what is
required, but I'm sufficiently uncertain of oilpan's GC behavior to be confident
of it.

+class Location final : public RefCountedWillBeGarbageCollected<Location>, public ScriptWrappable {
     DEFINE_WRAPPERTYPEINFO();
     WILL_BE_USING_GARBAGE_COLLECTED_MIXIN(Location);
 public:
-    static PassRefPtrWillBeRawPtr<Location> create(LocalFrame* frame)
+    static PassRefPtrWillBeRawPtr<Location> create(Frame* frame)
     {
         return adoptRefWillBeNoop(new Location(frame));
     }
 
+    Frame* frame() const { return m_frame.get(); }
+    void reset() { m_frame = nullptr; }
+
     void setHref(LocalDOMWindow* callingWindow, LocalDOMWindow* enteredWindow, const String&);
     String href() const;
 
@@ -77,15 +84,17 @@ public:
 
     PassRefPtrWillBeRawPtr<DOMStringList> ancestorOrigins() const;
 
-    virtual void trace(Visitor*) override;
+    virtual void trace(Visitor*);
 
 private:
-    explicit Location(LocalFrame*);
+    explicit Location(Frame*);
 
     enum class SetLocation { Normal, ReplaceThisFrame };
     void setLocation(const String&, LocalDOMWindow* callingWindow, LocalDOMWindow* enteredWindow, SetLocation = SetLocation::Normal);
 
     const KURL& url() const;
+
+    RawPtrWillBeWeakMember<Frame> m_frame;
 };
 
 } // namespace blink