Commit 07a5a71d authored by zerny@chromium.org's avatar zerny@chromium.org

Oilpan: Notify LocalFrame supplements of destruction.

This fixes the flaky crash in screen_orientation/page-visibility.html

The willBeDestroyed callback of GeolocationController has not been called in either build since r175404.

In addition LocalFrame supplements receive an explicit callback when the LocalHost is torn down in order to perform coordinated cleanup. This callback can be removed again once LocalFrame is moved to the heap and it again dies together with its supplements.

R=ager@chromium.org, tkent@chromium.org
BUG=

Review URL: https://codereview.chromium.org/335573004

git-svn-id: svn://svn.chromium.org/blink/trunk@176102 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent a2147d7b
......@@ -90,12 +90,19 @@ GeolocationController::~GeolocationController()
m_client->controllerForTestRemoved(this);
}
// FIXME: Oilpan: Once GeolocationClient is on-heap m_client should be a strong
// pointer and |willBeDestroyed| can potentially be removed from Supplement.
void GeolocationController::willBeDestroyed()
{
if (m_client)
m_client->geolocationDestroyed();
}
void GeolocationController::persistentHostHasBeenDestroyed()
{
observeContext(0);
}
PassOwnPtrWillBeRawPtr<GeolocationController> GeolocationController::create(LocalFrame& frame, GeolocationClient* client)
{
return adoptPtrWillBeNoop(new GeolocationController(frame, client));
......
......@@ -69,9 +69,10 @@ public:
static const char* supplementName();
static GeolocationController* from(LocalFrame* frame) { return static_cast<GeolocationController*>(WillBeHeapSupplement<LocalFrame>::from(frame, supplementName())); }
// Inherited from Supplement.
virtual void trace(Visitor*) OVERRIDE;
virtual void willBeDestroyed() OVERRIDE;
virtual void persistentHostHasBeenDestroyed() OVERRIDE;
private:
GeolocationController(LocalFrame&, GeolocationClient*);
......
......@@ -22,6 +22,12 @@ ScreenOrientationController::~ScreenOrientationController()
{
}
void ScreenOrientationController::persistentHostHasBeenDestroyed()
{
// Unregister lifecycle observation once page is being torn down.
observeContext(0);
}
void ScreenOrientationController::provideTo(LocalFrame& frame, blink::WebScreenOrientationClient* client)
{
ScreenOrientationController* controller = new ScreenOrientationController(frame, client);
......
......@@ -25,6 +25,8 @@ class ScreenOrientationController FINAL : public NoBaseWillBeGarbageCollectedFin
public:
virtual ~ScreenOrientationController();
virtual void persistentHostHasBeenDestroyed() OVERRIDE;
blink::WebScreenOrientationType orientation() const;
static void provideTo(LocalFrame&, blink::WebScreenOrientationClient*);
......
......@@ -145,6 +145,9 @@ public:
virtual void trace(Visitor*) { }
virtual void willBeDestroyed() { }
// FIXME: Oilpan: Remove this callback once PersistentHeapSupplementable is removed again.
virtual void persistentHostHasBeenDestroyed() { }
};
template<typename T, bool>
......@@ -194,8 +197,8 @@ public:
it->value->willBeDestroyed();
}
private:
// FIXME: Oilpan: Remove this ignore once PersistentHeapSupplementable is removed again.
// FIXME: Oilpan: Make private and remove this ignore once PersistentHeapSupplementable is removed again.
protected:
GC_PLUGIN_IGNORE("")
typename SupplementableTraits<T, isGarbageCollected>::SupplementMap m_supplements;
......@@ -219,6 +222,12 @@ template<typename T>
class PersistentHeapSupplementable : public SupplementableBase<T, true> {
public:
PersistentHeapSupplementable() : m_root(this) { }
virtual ~PersistentHeapSupplementable()
{
typedef typename SupplementableTraits<T, true>::SupplementMap::iterator SupplementIterator;
for (SupplementIterator it = this->m_supplements.begin(); it != this->m_supplements.end(); ++it)
it->value->persistentHostHasBeenDestroyed();
}
private:
class TraceDelegate : PersistentBase<ThreadLocalPersistents<AnyThread>, TraceDelegate> {
public:
......
......@@ -509,6 +509,15 @@ WebRemoteFrame* WebLocalFrameImpl::toWebRemoteFrame()
void WebLocalFrameImpl::close()
{
m_client = 0;
// FIXME: Oilpan: Signal to LocalFrame and its supplements that the frame is
// being torn down so it can do prompt clean-up. For example, this will
// clear the raw back pointer to m_geolocationClientProxy. Once
// GeolocationClientProxy is on-heap it looks like we can completely remove
// |willBeDestroyed| from supplements since tracing will ensure safety.
if (m_frame)
m_frame->willBeDestroyed();
deref(); // Balances ref() acquired in WebFrame::create
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment