Move Location to DOMWindow

Source/core/frame/Location.h

 /*
  * Copyright (C) 2008, 2010 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
  * 1.  Redistributions of source code must retain the above copyright
  *     notice, this list of conditions and the following disclaimer.
  * 2.  Redistributions in binary form must reproduce the above copyright
@@ skipping 22 matching lines @@
 #include "core/dom/DOMStringList.h"
 #include "core/frame/DOMWindowProperty.h"
 #include "wtf/PassRefPtr.h"
 #include "wtf/RefCounted.h"
 #include "wtf/text/WTFString.h"
 
 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;
 
     void assign(LocalDOMWindow* callingWindow, LocalDOMWindow* enteredWindow, const String&);
     void replace(LocalDOMWindow* callingWindow, LocalDOMWindow* enteredWindow, const String&);
     void reload(LocalDOMWindow* callingWindow);
 
     void setProtocol(LocalDOMWindow* callingWindow, LocalDOMWindow* enteredWindow, const String&, ExceptionState&);
     String protocol() const;
     void setHost(LocalDOMWindow* callingWindow, LocalDOMWindow* enteredWindow, const String&);
     String host() const;
     void setHostname(LocalDOMWindow* callingWindow, LocalDOMWindow* enteredWindow, const String&);
     String hostname() const;
     void setPort(LocalDOMWindow* callingWindow, LocalDOMWindow* enteredWindow, const String&);
     String port() const;
     void setPathname(LocalDOMWindow* callingWindow, LocalDOMWindow* enteredWindow, const String&);
     String pathname() const;
     void setSearch(LocalDOMWindow* callingWindow, LocalDOMWindow* enteredWindow, const String&);
     String search() const;
     void setHash(LocalDOMWindow* callingWindow, LocalDOMWindow* enteredWindow, const String&);
     String hash() const;
     String origin() const;
 
     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
 
 #endif // Location_h