Bug 723190: Fix/mitigate crash at nsGfxScrollFrameInner::ScrollToImpl. r=smichaud
authorJosh Aas <joshmoz@gmail.com>
Fri, 17 Feb 2012 23:32:16 -0500
changeset 87151 19d7edbf60bc9816ec1c094298d8dfa81872824e
parent 87150 1dcee8f1559a0dac6b9f49c5c052417f03e754df
child 87152 20478b6732123ec2ace4d3568c4ed8221472c0b2
child 87160 121daf1e632b5dfc072b28bf432723ab8d8f5b2e
push id22083
push userbmo@edmorley.co.uk
push dateSat, 18 Feb 2012 11:19:19 +0000
treeherdermozilla-central@20478b673212 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmichaud
bugs723190
milestone13.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 723190: Fix/mitigate crash at nsGfxScrollFrameInner::ScrollToImpl. r=smichaud
dom/plugins/base/nsPluginInstanceOwner.cpp
--- a/dom/plugins/base/nsPluginInstanceOwner.cpp
+++ b/dom/plugins/base/nsPluginInstanceOwner.cpp
@@ -3678,42 +3678,52 @@ void nsPluginInstanceOwner::SetFrame(nsO
         // called, so OnDestroyImage() can't yet have been called.  So we need
         // to do ourselves what OnDestroyImage() would have done.
         NS_RELEASE_THIS();
       }
 #endif
       container->SetCurrentImage(nsnull);
     }
 
-    // If we had an old frame and we're not going to have a new one then
-    // we should unregister for some things.
+#if defined(XP_MACOSX) && !defined(NP_NO_QUICKDRAW)
     if (!aFrame) {
-      // Unregister scroll position listeners
+      // At this point we had a frame but it is going away and we're not getting a new one.
+      // Unregister for a scroll position listening, which is only required for Carbon
+      // event model plugins on Mac OS X. It's OK to unregister when we didn't register,
+      // so don't be strict about unregistering. Better to unregister when we didn't have to
+      // than to not unregister when we should.
       for (nsIFrame* f = mObjectFrame; f; f = nsLayoutUtils::GetCrossDocParentFrame(f)) {
         nsIScrollableFrame* sf = do_QueryFrame(f);
         if (sf) {
           sf->RemoveScrollPositionListener(this);
         }
       }
     }
+#endif
 
     // Make sure the old frame isn't holding a reference to us.
     mObjectFrame->SetInstanceOwner(nsnull);
   } else {
+    // Scroll position listening is only required for Carbon event model plugins on Mac OS X.
+    // Note that we probably have a crash bug in the way we register/unregister, bug 723190.
+    // Bug 723190 is mitigated by limiting registration to Carbon event model plugins.
+#if defined(XP_MACOSX) && !defined(NP_NO_QUICKDRAW)
     if (aFrame) {
-      // We didn't have an object frame before but we do now!
-      // We need to register a scroll position listener on every scrollable
-      // frame up to the top
-      for (nsIFrame* f = aFrame; f; f = nsLayoutUtils::GetCrossDocParentFrame(f)) {
-        nsIScrollableFrame* sf = do_QueryFrame(f);
-        if (sf) {
-          sf->AddScrollPositionListener(this);
+      // We didn't have an object frame before but we do now. We need to register a scroll
+      // position listener on every scrollable frame up to the top.
+      if (GetEventModel() == NPEventModelCarbon) {
+        for (nsIFrame* f = aFrame; f; f = nsLayoutUtils::GetCrossDocParentFrame(f)) {
+          nsIScrollableFrame* sf = do_QueryFrame(f);
+          if (sf) {
+            sf->AddScrollPositionListener(this);
+          }
         }
       }
     }
+#endif
   }
 
   // Swap in the new frame (or no frame)
   mObjectFrame = aFrame;
 
   // Set up a new frame
   if (mObjectFrame) {
     mObjectFrame->SetInstanceOwner(this);