[iOS] Potential crash under WebCore::notifyLowPowerModeChanged(WebCore::LowPowerModeN...
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 23 Jun 2017 17:33:13 +0000 (17:33 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 23 Jun 2017 17:33:13 +0000 (17:33 +0000)
https://bugs.webkit.org/show_bug.cgi?id=173755
<rdar://problem/32940942>

Reviewed by Mark Lam.

The crash was happening because the WebLowPowerModeObserver would dispatch
a lambda to the main thread but the LowPowerModeNotifier object could be
dead by the time we get to the main thread.

To address the issue, keep a strong ref to the WebLowPowerModeObserver in
the lambda we dispatch to the main thread to make sure it stays alive until
we execute the lambda. In the LowPowerModeNotifier destructor, we now reset
the WebLowPowerModeObserver's notifier pointer to nil and I added a null
check for this notifier in the lambda.

* platform/LowPowerModeNotifier.cpp:
(WebCore::LowPowerModeNotifier::~LowPowerModeNotifier):
* platform/LowPowerModeNotifier.h:
* platform/ios/LowPowerModeNotifierIOS.mm:
(-[WebLowPowerModeObserver initWithNotifier:]):
(-[WebLowPowerModeObserver _didReceiveLowPowerModeChange]):
(WebCore::LowPowerModeNotifier::LowPowerModeNotifier):
(WebCore::LowPowerModeNotifier::~LowPowerModeNotifier):
(WebCore::notifyLowPowerModeChanged):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@218751 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/platform/LowPowerModeNotifier.cpp
Source/WebCore/platform/LowPowerModeNotifier.h
Source/WebCore/platform/ios/LowPowerModeNotifierIOS.mm

index e16adbb..03e73e1 100644 (file)
@@ -1,3 +1,31 @@
+2017-06-23  Chris Dumez  <cdumez@apple.com>
+
+        [iOS] Potential crash under WebCore::notifyLowPowerModeChanged(WebCore::LowPowerModeNotifier*, bool)
+        https://bugs.webkit.org/show_bug.cgi?id=173755
+        <rdar://problem/32940942>
+
+        Reviewed by Mark Lam.
+
+        The crash was happening because the WebLowPowerModeObserver would dispatch
+        a lambda to the main thread but the LowPowerModeNotifier object could be
+        dead by the time we get to the main thread.
+
+        To address the issue, keep a strong ref to the WebLowPowerModeObserver in
+        the lambda we dispatch to the main thread to make sure it stays alive until
+        we execute the lambda. In the LowPowerModeNotifier destructor, we now reset
+        the WebLowPowerModeObserver's notifier pointer to nil and I added a null
+        check for this notifier in the lambda.
+
+        * platform/LowPowerModeNotifier.cpp:
+        (WebCore::LowPowerModeNotifier::~LowPowerModeNotifier):
+        * platform/LowPowerModeNotifier.h:
+        * platform/ios/LowPowerModeNotifierIOS.mm:
+        (-[WebLowPowerModeObserver initWithNotifier:]):
+        (-[WebLowPowerModeObserver _didReceiveLowPowerModeChange]):
+        (WebCore::LowPowerModeNotifier::LowPowerModeNotifier):
+        (WebCore::LowPowerModeNotifier::~LowPowerModeNotifier):
+        (WebCore::notifyLowPowerModeChanged):
+
 2017-06-23  Alex Christensen  <achristensen@webkit.org>
 
         Add SPI to WKURLSchemeTask for redirection
index a46dd13..6de8f2a 100644 (file)
@@ -34,6 +34,10 @@ LowPowerModeNotifier::LowPowerModeNotifier(LowPowerModeChangeCallback&&)
 {
 }
 
+LowPowerModeNotifier::~LowPowerModeNotifier()
+{
+}
+
 bool LowPowerModeNotifier::isLowPowerModeEnabled() const
 {
     return false;
index ea5beff..2cddaab 100644 (file)
@@ -40,13 +40,14 @@ class LowPowerModeNotifier {
 public:
     using LowPowerModeChangeCallback = WTF::Function<void(bool isLowPowerModeEnabled)>;
     WEBCORE_EXPORT explicit LowPowerModeNotifier(LowPowerModeChangeCallback&&);
+    WEBCORE_EXPORT ~LowPowerModeNotifier();
 
     WEBCORE_EXPORT bool isLowPowerModeEnabled() const;
 
 private:
 #if PLATFORM(IOS)
     void notifyLowPowerModeChanged(bool);
-    friend void notifyLowPowerModeChanged(LowPowerModeNotifier*, bool);
+    friend void notifyLowPowerModeChanged(LowPowerModeNotifier&, bool);
 
     RetainPtr<WebLowPowerModeObserver> m_observer;
     LowPowerModeChangeCallback m_callback;
index cd64ca8..29a3aa8 100644 (file)
 @implementation WebLowPowerModeObserver {
 }
 
-- (WebLowPowerModeObserver *)initWithNotifier:(WebCore::LowPowerModeNotifier *)notifier
+- (WebLowPowerModeObserver *)initWithNotifier:(WebCore::LowPowerModeNotifier&)notifier
 {
     self = [super init];
     if (!self)
         return nil;
 
-    _notifier = notifier;
+    _notifier = &notifier;
     _isLowPowerModeEnabled = [NSProcessInfo processInfo].lowPowerModeEnabled;
     [[NSNotificationCenter defaultCenter] addObserver:self selector:@selector(_didReceiveLowPowerModeChange) name:NSProcessInfoPowerStateDidChangeNotification object:nil];
     return self;
@@ -62,8 +62,9 @@
 {
     _isLowPowerModeEnabled = [NSProcessInfo processInfo].lowPowerModeEnabled;
     // We need to make sure we notify the client on the main thread.
-    callOnMainThread([notifier = _notifier, isLowPowerModeEnabled = _isLowPowerModeEnabled] {
-        notifyLowPowerModeChanged(notifier, isLowPowerModeEnabled);
+    callOnMainThread([self, protectedSelf = RetainPtr<WebLowPowerModeObserver>(self)] {
+        if (_notifier)
+            notifyLowPowerModeChanged(*_notifier, _isLowPowerModeEnabled);
     });
 }
 
 namespace WebCore {
 
 LowPowerModeNotifier::LowPowerModeNotifier(LowPowerModeChangeCallback&& callback)
-    : m_observer(adoptNS([[WebLowPowerModeObserver alloc] initWithNotifier: this]))
+    : m_observer(adoptNS([[WebLowPowerModeObserver alloc] initWithNotifier:*this]))
     , m_callback(WTFMove(callback))
 {
 }
 
+LowPowerModeNotifier::~LowPowerModeNotifier()
+{
+    m_observer.get().notifier = nil;
+}
+
 bool LowPowerModeNotifier::isLowPowerModeEnabled() const
 {
     return m_observer.get().isLowPowerModeEnabled;
@@ -87,10 +93,10 @@ void LowPowerModeNotifier::notifyLowPowerModeChanged(bool enabled)
     m_callback(enabled);
 }
 
-void notifyLowPowerModeChanged(LowPowerModeNotifier* notifier, bool enabled)
+void notifyLowPowerModeChanged(LowPowerModeNotifier& notifier, bool enabled)
 {
     RELEASE_LOG(PerformanceLogging, "Low power mode state has changed to %d", enabled);
-    notifier->notifyLowPowerModeChanged(enabled);
+    notifier.notifyLowPowerModeChanged(enabled);
 }
 
 } // namespace WebCore