Bug 1248847 - Assert AccessibleCaretEventHub mRefCnt > 1 in all its entry points. r=mats draft
authorTing-Yu Lin <tlin@mozilla.com>
Fri, 19 Feb 2016 18:21:16 +0800
changeset 332081 5994787b7feccf409db1faf6359676b5170ea203
parent 331960 d5daf10d3b74b04e8fa63c4e5429de8a0adf79f8
child 514539 a0c819ce61e8fb9e75af9f4f986501294e5bcbc7
push id11157
push usertlin@mozilla.com
push dateFri, 19 Feb 2016 10:28:05 +0000
reviewersmats
bugs1248847
milestone47.0a1
Bug 1248847 - Assert AccessibleCaretEventHub mRefCnt > 1 in all its entry points. r=mats Also I removed the 'explicit' keywords from the constructor since they have no argument so nothing can be implicited converted to them. MozReview-Commit-ID: GrFcqO0Uf1o
layout/base/AccessibleCaretEventHub.cpp
layout/base/gtest/TestAccessibleCaretEventHub.cpp
--- a/layout/base/AccessibleCaretEventHub.cpp
+++ b/layout/base/AccessibleCaretEventHub.cpp
@@ -468,16 +468,18 @@ nsEventStatus
 AccessibleCaretEventHub::HandleEvent(WidgetEvent* aEvent)
 {
   nsEventStatus status = nsEventStatus_eIgnore;
 
   if (!mInitialized) {
     return status;
   }
 
+  MOZ_ASSERT(mRefCnt.get() > 1, "Expect caller holds us as well!");
+
   switch (aEvent->mClass) {
   case eMouseEventClass:
     status = HandleMouseEvent(aEvent->AsMouseEvent());
     break;
 
   case eTouchEventClass:
     status = HandleTouchEvent(aEvent->AsTouchEvent());
     break;
@@ -650,61 +652,71 @@ AccessibleCaretEventHub::FireLongTap(nsI
 NS_IMETHODIMP
 AccessibleCaretEventHub::Reflow(DOMHighResTimeStamp aStart,
                                 DOMHighResTimeStamp aEnd)
 {
   if (!mInitialized) {
     return NS_OK;
   }
 
+  MOZ_ASSERT(mRefCnt.get() > 1, "Expect caller holds us as well!");
+
   AC_LOG("%s, state: %s", __FUNCTION__, mState->Name());
   mState->OnReflow(this);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 AccessibleCaretEventHub::ReflowInterruptible(DOMHighResTimeStamp aStart,
                                              DOMHighResTimeStamp aEnd)
 {
   if (!mInitialized) {
     return NS_OK;
   }
 
+  MOZ_ASSERT(mRefCnt.get() > 1, "Expect caller holds us as well!");
+
   return Reflow(aStart, aEnd);
 }
 
 void
 AccessibleCaretEventHub::AsyncPanZoomStarted()
 {
   if (!mInitialized) {
     return;
   }
 
+  MOZ_ASSERT(mRefCnt.get() > 1, "Expect caller holds us as well!");
+
   AC_LOG("%s, state: %s", __FUNCTION__, mState->Name());
   mState->OnScrollStart(this);
 }
 
 void
 AccessibleCaretEventHub::AsyncPanZoomStopped()
 {
   if (!mInitialized) {
     return;
   }
 
+  MOZ_ASSERT(mRefCnt.get() > 1, "Expect caller holds us as well!");
+
   AC_LOG("%s, state: %s", __FUNCTION__, mState->Name());
   mState->OnScrollEnd(this);
 }
 
 void
 AccessibleCaretEventHub::ScrollPositionChanged()
 {
   if (!mInitialized) {
     return;
   }
 
+  MOZ_ASSERT(mRefCnt.get() > 1, "Expect caller holds us as well!");
+
   AC_LOG("%s, state: %s", __FUNCTION__, mState->Name());
   mState->OnScrollPositionChanged(this);
 }
 
 void
 AccessibleCaretEventHub::LaunchScrollEndInjector()
 {
   if (!mScrollEndInjectorTimer) {
@@ -737,28 +749,32 @@ nsresult
 AccessibleCaretEventHub::NotifySelectionChanged(nsIDOMDocument* aDoc,
                                                 nsISelection* aSel,
                                                 int16_t aReason)
 {
   if (!mInitialized) {
     return NS_OK;
   }
 
+  MOZ_ASSERT(mRefCnt.get() > 1, "Expect caller holds us as well!");
+
   AC_LOG("%s, state: %s, reason: %d", __FUNCTION__, mState->Name(), aReason);
   mState->OnSelectionChanged(this, aDoc, aSel, aReason);
   return NS_OK;
 }
 
 void
 AccessibleCaretEventHub::NotifyBlur(bool aIsLeavingDocument)
 {
   if (!mInitialized) {
     return;
   }
 
+  MOZ_ASSERT(mRefCnt.get() > 1, "Expect caller holds us as well!");
+
   AC_LOG("%s, state: %s", __FUNCTION__, mState->Name());
   mState->OnBlur(this, aIsLeavingDocument);
 }
 
 nsPoint
 AccessibleCaretEventHub::GetTouchEventPosition(WidgetTouchEvent* aEvent,
                                                int32_t aIdentifier) const
 {
--- a/layout/base/gtest/TestAccessibleCaretEventHub.cpp
+++ b/layout/base/gtest/TestAccessibleCaretEventHub.cpp
@@ -30,17 +30,17 @@ using ::testing::_;
 // various combination of events and callbacks.
 
 namespace mozilla
 {
 
 class MockAccessibleCaretManager : public AccessibleCaretManager
 {
 public:
-  explicit MockAccessibleCaretManager()
+  MockAccessibleCaretManager()
     : AccessibleCaretManager(nullptr)
   {
   }
 
   MOCK_METHOD1(PressCaret, nsresult(const nsPoint& aPoint));
   MOCK_METHOD1(DragCaret, nsresult(const nsPoint& aPoint));
   MOCK_METHOD0(ReleaseCaret, nsresult());
   MOCK_METHOD1(TapCaret, nsresult(const nsPoint& aPoint));
@@ -58,17 +58,17 @@ public:
   using AccessibleCaretEventHub::PressCaretState;
   using AccessibleCaretEventHub::DragCaretState;
   using AccessibleCaretEventHub::PressNoCaretState;
   using AccessibleCaretEventHub::ScrollState;
   using AccessibleCaretEventHub::PostScrollState;
   using AccessibleCaretEventHub::LongTapState;
   using AccessibleCaretEventHub::FireScrollEnd;
 
-  explicit MockAccessibleCaretEventHub()
+  MockAccessibleCaretEventHub()
     : AccessibleCaretEventHub(nullptr)
   {
     mManager = MakeUnique<MockAccessibleCaretManager>();
     mInitialized = true;
   }
 
   virtual nsPoint GetTouchEventPosition(WidgetTouchEvent* aEvent,
                                         int32_t aIdentifier) const override
@@ -96,20 +96,30 @@ public:
                            const MockAccessibleCaretEventHub::State* aState)
 {
   return aOstream << aState->Name();
 }
 
 class AccessibleCaretEventHubTester : public ::testing::Test
 {
 public:
-  explicit AccessibleCaretEventHubTester()
+  AccessibleCaretEventHubTester()
   {
     DefaultValue<nsresult>::Set(NS_OK);
     EXPECT_EQ(mHub->GetState(), MockAccessibleCaretEventHub::NoActionState());
+
+    // AccessibleCaretEventHub requires the caller to hold a ref to it. We just
+    // add ref here for the sake of convenience.
+    mHub.get()->AddRef();
+  }
+
+  ~AccessibleCaretEventHubTester()
+  {
+    // Release the ref added in the constructor.
+    mHub.get()->Release();
   }
 
   static UniquePtr<WidgetEvent> CreateMouseEvent(EventMessage aMessage,
                                                  nscoord aX,
                                                  nscoord aY)
   {
     auto event = MakeUnique<WidgetMouseEvent>(true, aMessage, nullptr,
                                               WidgetMouseEvent::eReal);