[webkit-changes] [218751] trunk/Source/WebCore

Fri, 23 Jun 2017 10:33:53 -0700

Title: [218751] trunk/Source/WebCore
Revision
218751
Author
[email protected]
Date
2017-06-23 10:33:13 -0700 (Fri, 23 Jun 2017)

Log Message

[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):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (218750 => 218751)


--- trunk/Source/WebCore/ChangeLog	2017-06-23 17:25:43 UTC (rev 218750)
+++ trunk/Source/WebCore/ChangeLog	2017-06-23 17:33:13 UTC (rev 218751)
@@ -1,3 +1,31 @@
+2017-06-23  Chris Dumez  <[email protected]>
+
+        [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  <[email protected]>
 
         Add SPI to WKURLSchemeTask for redirection

Modified: trunk/Source/WebCore/platform/LowPowerModeNotifier.cpp (218750 => 218751)


--- trunk/Source/WebCore/platform/LowPowerModeNotifier.cpp	2017-06-23 17:25:43 UTC (rev 218750)
+++ trunk/Source/WebCore/platform/LowPowerModeNotifier.cpp	2017-06-23 17:33:13 UTC (rev 218751)
@@ -34,6 +34,10 @@
 {
 }
 
+LowPowerModeNotifier::~LowPowerModeNotifier()
+{
+}
+
 bool LowPowerModeNotifier::isLowPowerModeEnabled() const
 {
     return false;

Modified: trunk/Source/WebCore/platform/LowPowerModeNotifier.h (218750 => 218751)


--- trunk/Source/WebCore/platform/LowPowerModeNotifier.h	2017-06-23 17:25:43 UTC (rev 218750)
+++ trunk/Source/WebCore/platform/LowPowerModeNotifier.h	2017-06-23 17:33:13 UTC (rev 218751)
@@ -40,6 +40,7 @@
 public:
     using LowPowerModeChangeCallback = WTF::Function<void(bool isLowPowerModeEnabled)>;
     WEBCORE_EXPORT explicit LowPowerModeNotifier(LowPowerModeChangeCallback&&);
+    WEBCORE_EXPORT ~LowPowerModeNotifier();
 
     WEBCORE_EXPORT bool isLowPowerModeEnabled() const;
 
@@ -46,7 +47,7 @@
 private:
 #if PLATFORM(IOS)
     void notifyLowPowerModeChanged(bool);
-    friend void notifyLowPowerModeChanged(LowPowerModeNotifier*, bool);
+    friend void notifyLowPowerModeChanged(LowPowerModeNotifier&, bool);
 
     RetainPtr<WebLowPowerModeObserver> m_observer;
     LowPowerModeChangeCallback m_callback;

Modified: trunk/Source/WebCore/platform/ios/LowPowerModeNotifierIOS.mm (218750 => 218751)


--- trunk/Source/WebCore/platform/ios/LowPowerModeNotifierIOS.mm	2017-06-23 17:25:43 UTC (rev 218750)
+++ trunk/Source/WebCore/platform/ios/LowPowerModeNotifierIOS.mm	2017-06-23 17:33:13 UTC (rev 218751)
@@ -40,13 +40,13 @@
 @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);
     });
 }
 
@@ -72,11 +73,16 @@
 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 @@
     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

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to