Remove PageVisibilityObserver contextDestroyed() notifications.
PageVisibilityObserver allows a Page's lifetime states to be
observed, but none of the observers need to be notified of Page
destruction.
Adjust LifecycleNotifier<>::notifyContextDestroyed(), having it
call its corresponding LifecycleObserver's contextDestroyed()
notification method if the observer implements it, but not require
that it does implement contextDestroyed(). With that in place,
only have the lifecycle observers that need a contextDestroyed()
notification (all but PageVisibilityObserver) declare it.
This allows the removal of all the empty
PageVisibilityObserver::contextDestroyed() overrides.
R=haraken
BUG=610176
Review-Url: https://codereview.chromium.org/2634713002
Cr-Commit-Position: refs/heads/master@{#443816}
diff --git a/third_party/WebKit/Source/core/dom/ContextLifecycleObserver.h b/third_party/WebKit/Source/core/dom/ContextLifecycleObserver.h
index e39cc400..3bf231d 100644
--- a/third_party/WebKit/Source/core/dom/ContextLifecycleObserver.h
+++ b/third_party/WebKit/Source/core/dom/ContextLifecycleObserver.h
@@ -63,6 +63,8 @@
class CORE_EXPORT ContextLifecycleObserver
: public LifecycleObserver<ExecutionContext, ContextLifecycleObserver> {
public:
+ virtual void contextDestroyed(ExecutionContext*) {}
+
ExecutionContext* getExecutionContext() const { return lifecycleContext(); }
LocalFrame* frame() const;
diff --git a/third_party/WebKit/Source/core/dom/SynchronousMutationObserver.h b/third_party/WebKit/Source/core/dom/SynchronousMutationObserver.h
index b52fb0b..7a4c300 100644
--- a/third_party/WebKit/Source/core/dom/SynchronousMutationObserver.h
+++ b/third_party/WebKit/Source/core/dom/SynchronousMutationObserver.h
@@ -65,6 +65,9 @@
// Called before removing node.
virtual void nodeWillBeRemoved(Node&);
+ // Called when detaching document.
+ virtual void contextDestroyed(Document*) {}
+
protected:
SynchronousMutationObserver();
diff --git a/third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp b/third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp
index 8197f0d..a07ec21 100644
--- a/third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp
+++ b/third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp
@@ -1202,8 +1202,6 @@
m_context->stop();
}
-void HTMLCanvasElement::contextDestroyed(Page*) {}
-
void HTMLCanvasElement::styleDidChange(const ComputedStyle* oldStyle,
const ComputedStyle& newStyle) {
if (m_context)
diff --git a/third_party/WebKit/Source/core/html/HTMLCanvasElement.h b/third_party/WebKit/Source/core/html/HTMLCanvasElement.h
index e9dfbaf8..f1443be0 100644
--- a/third_party/WebKit/Source/core/html/HTMLCanvasElement.h
+++ b/third_party/WebKit/Source/core/html/HTMLCanvasElement.h
@@ -172,7 +172,6 @@
// ContextLifecycleObserver and PageVisibilityObserver implementation
void contextDestroyed(ExecutionContext*) override;
- void contextDestroyed(Page*) override;
// PageVisibilityObserver implementation
void pageVisibilityChanged() override;
diff --git a/third_party/WebKit/Source/core/workers/WorkerThreadLifecycleObserver.h b/third_party/WebKit/Source/core/workers/WorkerThreadLifecycleObserver.h
index 2e0e710..a01c39d 100644
--- a/third_party/WebKit/Source/core/workers/WorkerThreadLifecycleObserver.h
+++ b/third_party/WebKit/Source/core/workers/WorkerThreadLifecycleObserver.h
@@ -29,6 +29,9 @@
class CORE_EXPORT WorkerThreadLifecycleObserver
: public LifecycleObserver<WorkerThreadLifecycleContext,
WorkerThreadLifecycleObserver> {
+ public:
+ virtual void contextDestroyed(WorkerThreadLifecycleContext*) {}
+
protected:
explicit WorkerThreadLifecycleObserver(WorkerThreadLifecycleContext*);
virtual ~WorkerThreadLifecycleObserver();
diff --git a/third_party/WebKit/Source/modules/battery/BatteryManager.cpp b/third_party/WebKit/Source/modules/battery/BatteryManager.cpp
index 694dac1..4541db0 100644
--- a/third_party/WebKit/Source/modules/battery/BatteryManager.cpp
+++ b/third_party/WebKit/Source/modules/battery/BatteryManager.cpp
@@ -111,8 +111,6 @@
stopUpdating();
}
-void BatteryManager::contextDestroyed(Page*) {}
-
bool BatteryManager::hasPendingActivity() const {
// Prevent V8 from garbage collecting the wrapper object if there are
// event listeners attached to it.
diff --git a/third_party/WebKit/Source/modules/battery/BatteryManager.h b/third_party/WebKit/Source/modules/battery/BatteryManager.h
index fa13c2db..a93c5e75 100644
--- a/third_party/WebKit/Source/modules/battery/BatteryManager.h
+++ b/third_party/WebKit/Source/modules/battery/BatteryManager.h
@@ -59,7 +59,6 @@
void suspend() override;
void resume() override;
void contextDestroyed(ExecutionContext*) override;
- void contextDestroyed(Page*) override;
// ScriptWrappable implementation.
bool hasPendingActivity() const final;
diff --git a/third_party/WebKit/Source/modules/gamepad/NavigatorGamepad.cpp b/third_party/WebKit/Source/modules/gamepad/NavigatorGamepad.cpp
index 9d249c28..e148ac8c 100644
--- a/third_party/WebKit/Source/modules/gamepad/NavigatorGamepad.cpp
+++ b/third_party/WebKit/Source/modules/gamepad/NavigatorGamepad.cpp
@@ -186,8 +186,6 @@
stopUpdating();
}
-void NavigatorGamepad::contextDestroyed(Page*) {}
-
void NavigatorGamepad::registerWithDispatcher() {
GamepadDispatcher::instance().addController(this);
m_dispatchOneEventRunner->resume();
diff --git a/third_party/WebKit/Source/modules/gamepad/NavigatorGamepad.h b/third_party/WebKit/Source/modules/gamepad/NavigatorGamepad.h
index 7e7c95d..b0c8144 100644
--- a/third_party/WebKit/Source/modules/gamepad/NavigatorGamepad.h
+++ b/third_party/WebKit/Source/modules/gamepad/NavigatorGamepad.h
@@ -76,7 +76,6 @@
// ContextLifecycleObserver and PageVisibilityObserver
void contextDestroyed(ExecutionContext*) override;
- void contextDestroyed(Page*) override;
void pageVisibilityChanged() override;
// PlatformEventController
diff --git a/third_party/WebKit/Source/modules/geolocation/Geolocation.cpp b/third_party/WebKit/Source/modules/geolocation/Geolocation.cpp
index 8268f24d..327322c 100644
--- a/third_party/WebKit/Source/modules/geolocation/Geolocation.cpp
+++ b/third_party/WebKit/Source/modules/geolocation/Geolocation.cpp
@@ -123,8 +123,6 @@
m_lastPosition = nullptr;
}
-void Geolocation::contextDestroyed(Page*) {}
-
void Geolocation::recordOriginTypeAccess() const {
DCHECK(frame());
diff --git a/third_party/WebKit/Source/modules/geolocation/Geolocation.h b/third_party/WebKit/Source/modules/geolocation/Geolocation.h
index 7eb7c86..1bb2c19 100644
--- a/third_party/WebKit/Source/modules/geolocation/Geolocation.h
+++ b/third_party/WebKit/Source/modules/geolocation/Geolocation.h
@@ -66,7 +66,6 @@
// Inherited from ContextLifecycleObserver and PageVisibilityObserver.
void contextDestroyed(ExecutionContext*) override;
- void contextDestroyed(Page*) override;
Document* document() const;
LocalFrame* frame() const;
diff --git a/third_party/WebKit/Source/modules/nfc/NFC.cpp b/third_party/WebKit/Source/modules/nfc/NFC.cpp
index 76b577cb..5a07ae7 100644
--- a/third_party/WebKit/Source/modules/nfc/NFC.cpp
+++ b/third_party/WebKit/Source/modules/nfc/NFC.cpp
@@ -604,8 +604,6 @@
m_callbacks.clear();
}
-void NFC::contextDestroyed(Page*) {}
-
// https://w3c.github.io/web-nfc/#writing-or-pushing-content
// https://w3c.github.io/web-nfc/#dom-nfc-push
ScriptPromise NFC::push(ScriptState* scriptState,
diff --git a/third_party/WebKit/Source/modules/nfc/NFC.h b/third_party/WebKit/Source/modules/nfc/NFC.h
index 0235fda..8d0b25cb8 100644
--- a/third_party/WebKit/Source/modules/nfc/NFC.h
+++ b/third_party/WebKit/Source/modules/nfc/NFC.h
@@ -40,7 +40,6 @@
// ContextLifecycleObserver overrides.
void contextDestroyed(ExecutionContext*) override;
- void contextDestroyed(Page*) override;
// Pushes NFCPushMessage asynchronously to NFC tag / peer.
ScriptPromise push(ScriptState*,
diff --git a/third_party/WebKit/Source/modules/presentation/PresentationAvailability.cpp b/third_party/WebKit/Source/modules/presentation/PresentationAvailability.cpp
index 0992a37..11815a3b 100644
--- a/third_party/WebKit/Source/modules/presentation/PresentationAvailability.cpp
+++ b/third_party/WebKit/Source/modules/presentation/PresentationAvailability.cpp
@@ -98,8 +98,6 @@
setState(State::Inactive);
}
-void PresentationAvailability::contextDestroyed(Page*) {}
-
void PresentationAvailability::pageVisibilityChanged() {
if (m_state == State::Inactive)
return;
diff --git a/third_party/WebKit/Source/modules/presentation/PresentationAvailability.h b/third_party/WebKit/Source/modules/presentation/PresentationAvailability.h
index bceee4d..8aac2a6 100644
--- a/third_party/WebKit/Source/modules/presentation/PresentationAvailability.h
+++ b/third_party/WebKit/Source/modules/presentation/PresentationAvailability.h
@@ -53,7 +53,6 @@
void suspend() override;
void resume() override;
void contextDestroyed(ExecutionContext*) override;
- void contextDestroyed(Page*) override;
// PageVisibilityObserver implementation.
void pageVisibilityChanged() override;
diff --git a/third_party/WebKit/Source/modules/screen_orientation/ScreenOrientationControllerImpl.cpp b/third_party/WebKit/Source/modules/screen_orientation/ScreenOrientationControllerImpl.cpp
index a9d2290..a8ec2c0 100644
--- a/third_party/WebKit/Source/modules/screen_orientation/ScreenOrientationControllerImpl.cpp
+++ b/third_party/WebKit/Source/modules/screen_orientation/ScreenOrientationControllerImpl.cpp
@@ -215,8 +215,6 @@
m_activeLock = false;
}
-void ScreenOrientationControllerImpl::contextDestroyed(Page*) {}
-
void ScreenOrientationControllerImpl::notifyDispatcher() {
if (m_orientation && page()->isPageVisible())
startUpdating();
diff --git a/third_party/WebKit/Source/modules/screen_orientation/ScreenOrientationControllerImpl.h b/third_party/WebKit/Source/modules/screen_orientation/ScreenOrientationControllerImpl.h
index bba564b..72ca677d 100644
--- a/third_party/WebKit/Source/modules/screen_orientation/ScreenOrientationControllerImpl.h
+++ b/third_party/WebKit/Source/modules/screen_orientation/ScreenOrientationControllerImpl.h
@@ -57,7 +57,6 @@
// Inherited from ContextLifecycleObserver and PageVisibilityObserver.
void contextDestroyed(ExecutionContext*) override;
- void contextDestroyed(Page*) override;
void pageVisibilityChanged() override;
void notifyDispatcher();
diff --git a/third_party/WebKit/Source/modules/vibration/VibrationController.cpp b/third_party/WebKit/Source/modules/vibration/VibrationController.cpp
index eaa3fb1..f95c61f 100644
--- a/third_party/WebKit/Source/modules/vibration/VibrationController.cpp
+++ b/third_party/WebKit/Source/modules/vibration/VibrationController.cpp
@@ -178,8 +178,6 @@
m_service.reset();
}
-void VibrationController::contextDestroyed(Page*) {}
-
void VibrationController::pageVisibilityChanged() {
if (!page()->isPageVisible())
cancel();
diff --git a/third_party/WebKit/Source/modules/vibration/VibrationController.h b/third_party/WebKit/Source/modules/vibration/VibrationController.h
index 64c5a42..176f04f 100644
--- a/third_party/WebKit/Source/modules/vibration/VibrationController.h
+++ b/third_party/WebKit/Source/modules/vibration/VibrationController.h
@@ -68,9 +68,8 @@
DECLARE_VIRTUAL_TRACE();
private:
- // Inherited from ContextLifecycleObserver and PageVisibilityObserver.
+ // Inherited from ContextLifecycleObserver.
void contextDestroyed(ExecutionContext*) override;
- void contextDestroyed(Page*) override;
// Inherited from PageVisibilityObserver.
void pageVisibilityChanged() override;
diff --git a/third_party/WebKit/Source/modules/wake_lock/ScreenWakeLock.cpp b/third_party/WebKit/Source/modules/wake_lock/ScreenWakeLock.cpp
index 5abb4d5e..943f2d7 100644
--- a/third_party/WebKit/Source/modules/wake_lock/ScreenWakeLock.cpp
+++ b/third_party/WebKit/Source/modules/wake_lock/ScreenWakeLock.cpp
@@ -55,8 +55,6 @@
setKeepAwake(false);
}
-void ScreenWakeLock::contextDestroyed(Page*) {}
-
DEFINE_TRACE(ScreenWakeLock) {
Supplement<LocalFrame>::trace(visitor);
PageVisibilityObserver::trace(visitor);
diff --git a/third_party/WebKit/Source/modules/wake_lock/ScreenWakeLock.h b/third_party/WebKit/Source/modules/wake_lock/ScreenWakeLock.h
index bae393f..2d6ac5f 100644
--- a/third_party/WebKit/Source/modules/wake_lock/ScreenWakeLock.h
+++ b/third_party/WebKit/Source/modules/wake_lock/ScreenWakeLock.h
@@ -41,7 +41,7 @@
// Inherited from PageVisibilityObserver.
void pageVisibilityChanged() override;
- void contextDestroyed(Page*) override;
+
// Inherited from ContextLifecycleObserver.
void contextDestroyed(ExecutionContext*) override;
diff --git a/third_party/WebKit/Source/platform/LifecycleContextTest.cpp b/third_party/WebKit/Source/platform/LifecycleContextTest.cpp
index 0aee4ae7..b53f6d13a 100644
--- a/third_party/WebKit/Source/platform/LifecycleContextTest.cpp
+++ b/third_party/WebKit/Source/platform/LifecycleContextTest.cpp
@@ -56,7 +56,7 @@
return new TestingObserver(context);
}
- void contextDestroyed(DummyContext* destroyedContext) override {
+ void contextDestroyed(DummyContext* destroyedContext) {
if (m_observerToRemoveOnDestruct) {
destroyedContext->removeObserver(m_observerToRemoveOnDestruct);
m_observerToRemoveOnDestruct.clear();
diff --git a/third_party/WebKit/Source/platform/LifecycleNotifier.h b/third_party/WebKit/Source/platform/LifecycleNotifier.h
index 4dde3ba..4589d8d 100644
--- a/third_party/WebKit/Source/platform/LifecycleNotifier.h
+++ b/third_party/WebKit/Source/platform/LifecycleNotifier.h
@@ -42,10 +42,12 @@
void removeObserver(Observer*);
// notifyContextDestroyed() should be explicitly dispatched from an
- // observed context to notify observers that contextDestroyed().
+ // observed context to detach its observers, and if the observer kind
+ // requires it, notify each observer by invoking contextDestroyed().
//
- // When contextDestroyed() is called, the observer's lifecycleContext()
- // is still valid and safe to use during the notification.
+ // When contextDestroyed() is called, it is supplied the context as
+ // an argument, but the observer's lifecycleContext() is still valid
+ // and safe to use while handling the notification.
virtual void notifyContextDestroyed();
DEFINE_INLINE_VIRTUAL_TRACE() { visitor->trace(m_observers); }
@@ -82,6 +84,51 @@
// ASSERT(!m_observers.size());
}
+// Determine if |contextDestroyed(Observer*) is a public method on
+// class type |Observer|, or any of the class types it derives from.
+template <typename Observer, typename T>
+class HasContextDestroyed {
+ using YesType = char;
+ using NoType = int;
+
+ template <typename V>
+ static YesType checkHasContextDestroyedMethod(
+ V* observer,
+ T* context = nullptr,
+ typename std::enable_if<
+ std::is_same<decltype(observer->contextDestroyed(context)),
+ void>::value>::type* g = nullptr);
+ template <typename V>
+ static NoType checkHasContextDestroyedMethod(...);
+
+ public:
+ static_assert(sizeof(Observer), "Observer's class declaration not in scope");
+ static const bool value =
+ sizeof(YesType) ==
+ sizeof(checkHasContextDestroyedMethod<Observer>(nullptr));
+};
+
+// If |Observer::contextDestroyed()| is present, invoke it.
+template <typename Observer,
+ typename T,
+ bool = HasContextDestroyed<Observer, T>::value>
+class NotifyContextDestroyed {
+ STATIC_ONLY(NotifyContextDestroyed);
+
+ public:
+ static void call(Observer* observer, T* context) {
+ observer->contextDestroyed(context);
+ }
+};
+
+template <typename Observer, typename T>
+class NotifyContextDestroyed<Observer, T, false> {
+ STATIC_ONLY(NotifyContextDestroyed);
+
+ public:
+ static void call(Observer*, T*) {}
+};
+
template <typename T, typename Observer>
inline void LifecycleNotifier<T, Observer>::notifyContextDestroyed() {
// Observer unregistration is allowed, but effectively a no-op.
@@ -90,7 +137,7 @@
m_observers.swap(observers);
for (Observer* observer : observers) {
DCHECK(observer->lifecycleContext() == context());
- observer->contextDestroyed(context());
+ NotifyContextDestroyed<Observer, T>::call(observer, context());
observer->clearContext();
}
}
diff --git a/third_party/WebKit/Source/platform/LifecycleObserver.h b/third_party/WebKit/Source/platform/LifecycleObserver.h
index e0d21b0..a1ac632d 100644
--- a/third_party/WebKit/Source/platform/LifecycleObserver.h
+++ b/third_party/WebKit/Source/platform/LifecycleObserver.h
@@ -39,8 +39,6 @@
public:
DEFINE_INLINE_VIRTUAL_TRACE() { visitor->trace(m_lifecycleContext); }
- virtual void contextDestroyed(Context*) {}
-
Context* lifecycleContext() const { return m_lifecycleContext; }
void clearContext() { setContext(nullptr); }
