Bug 1435788 - Allow handling focus-changing events prior to the first update on GPU process restart. r?rhunt draft
authorKartikaya Gupta <kgupta@mozilla.com>
Wed, 14 Mar 2018 08:43:56 -0400
changeset 767359 42450a7b70a2853ed2b2d62598fc8d7a1e4bb7e5
parent 767358 7c4a8603cda48d07756a4718cd8d6be804542271
push id102570
push userkgupta@mozilla.com
push dateWed, 14 Mar 2018 12:44:25 +0000
reviewersrhunt
bugs1435788
milestone61.0a1
Bug 1435788 - Allow handling focus-changing events prior to the first update on GPU process restart. r?rhunt If the GPU process restarts, the mLastAPZProcessedEvent gets reset to 1. The code in Update() checks for this special case and resets the value to the incoming content process sequence number. However, we before that first call to Update() on the sampler thread, the FocusState might get calls to ReceiveFocusChangingEvent(), which would be triggered by input events on the controller thread. These input events would advance mLastAPZProcessedEvent which would bypass the special case handling in Update(). This can leave mLastAPZProcessedEvent at a lower value than mLastContentProcessedEvent which triggers assertion failures. This patch ensures that calls to ReceiveFocusChangingEvent() during this initial state doesn't increment mLastAPZProcessedEvent, and so allows the special case handling in Update() to work as expected. It also adds the special case handling to the branch where the first Update() call results in no focus target being selected. MozReview-Commit-ID: 7P2O2qg0mXj
gfx/layers/apz/src/FocusState.cpp
gfx/layers/apz/src/FocusState.h
--- a/gfx/layers/apz/src/FocusState.cpp
+++ b/gfx/layers/apz/src/FocusState.cpp
@@ -14,16 +14,17 @@
 namespace mozilla {
 namespace layers {
 
 FocusState::FocusState()
   : mMutex("FocusStateMutex")
   , mLastAPZProcessedEvent(1)
   , mLastContentProcessedEvent(0)
   , mFocusHasKeyEventListeners(false)
+  , mReceivedUpdate(false)
   , mFocusLayersId(0)
   , mFocusHorizontalTarget(FrameMetrics::NULL_SCROLL_ID)
   , mFocusVerticalTarget(FrameMetrics::NULL_SCROLL_ID)
 {
 }
 
 uint64_t
 FocusState::LastAPZProcessedEvent() const
@@ -46,32 +47,43 @@ FocusState::IsCurrent(const MutexAutoLoc
 }
 
 void
 FocusState::ReceiveFocusChangingEvent()
 {
   APZThreadUtils::AssertOnControllerThread();
   MutexAutoLock lock(mMutex);
 
+  if (!mReceivedUpdate) {
+    // In the initial state don't advance mLastAPZProcessedEvent because we
+    // might blow away the information that we're in a freshly-restarted GPU
+    // process. This information (i.e. that mLastAPZProcessedEvent == 1) needs
+    // to be preserved until the first call to Update() which will then advance
+    // mLastAPZProcessedEvent to match the content-side sequence number.
+    return;
+  }
   mLastAPZProcessedEvent += 1;
+  FS_LOG("Focus changing event incremented aseq to %" PRIu64 "\n",
+         mLastAPZProcessedEvent);
 }
 
 void
 FocusState::Update(uint64_t aRootLayerTreeId,
                    uint64_t aOriginatingLayersId,
                    const FocusTarget& aState)
 {
   APZThreadUtils::AssertOnSamplerThread();
   MutexAutoLock lock(mMutex);
 
   FS_LOG("Update with rlt=%" PRIu64 ", olt=%" PRIu64 ", ft=(%s, %" PRIu64 ")\n",
          aRootLayerTreeId,
          aOriginatingLayersId,
          aState.Type(),
          aState.mSequenceNumber);
+  mReceivedUpdate = true;
 
   // Update the focus tree with the latest target
   mFocusTree[aOriginatingLayersId] = aState;
 
   // Reset our internal state so we can recalculate it
   mFocusHasKeyEventListeners = false;
   mFocusLayersId = aRootLayerTreeId;
   mFocusHorizontalTarget = FrameMetrics::NULL_SCROLL_ID;
@@ -98,21 +110,30 @@ FocusState::Update(uint64_t aRootLayerTr
     // enclosing method, FocusState::Update, should return or continue to the
     // next iteration of the while loop, respectively.
     struct FocusTargetDataMatcher {
 
       FocusState& mFocusState;
       const uint64_t mSequenceNumber;
 
       bool match(const FocusTarget::NoFocusTarget& aNoFocusTarget) {
-        FS_LOG("Setting target to nil (reached a nil target)\n");
+        FS_LOG("Setting target to nil (reached a nil target) with seq=%" PRIu64 "\n",
+               mSequenceNumber);
 
         // Mark what sequence number this target has for debugging purposes so
         // we can always accurately report on whether we are stale or not
         mFocusState.mLastContentProcessedEvent = mSequenceNumber;
+
+        // If this focus state was just created and content has experienced more
+        // events then us, then assume we were recreated and sync focus sequence
+        // numbers.
+        if (mFocusState.mLastAPZProcessedEvent == 1 &&
+            mFocusState.mLastContentProcessedEvent > mFocusState.mLastAPZProcessedEvent) {
+          mFocusState.mLastAPZProcessedEvent = mFocusState.mLastContentProcessedEvent;
+        }
         return true;
       }
 
       bool match(const FocusTarget::RefLayerId aRefLayerId) {
         // Guard against infinite loops
         MOZ_ASSERT(mFocusState.mFocusLayersId != aRefLayerId);
         if (mFocusState.mFocusLayersId == aRefLayerId) {
           FS_LOG("Setting target to nil (bailing out of infinite loop, lt=%" PRIu64 ")\n",
--- a/gfx/layers/apz/src/FocusState.h
+++ b/gfx/layers/apz/src/FocusState.h
@@ -152,16 +152,18 @@ private:
   // on input events.
   uint64_t mLastAPZProcessedEvent;
   // The focus sequence number last received in a focus update.
   uint64_t mLastContentProcessedEvent;
 
   // A flag whether there is a key listener on the event target chain for the
   // focused element
   bool mFocusHasKeyEventListeners;
+  // A flag that is false until the first call to Update().
+  bool mReceivedUpdate;
 
   // The layer tree ID which contains the scrollable frame of the focused element
   uint64_t mFocusLayersId;
   // The scrollable layer corresponding to the scrollable frame that is used to
   // scroll the focused element. This depends on the direction the user is
   // scrolling.
   FrameMetrics::ViewID mFocusHorizontalTarget;
   FrameMetrics::ViewID mFocusVerticalTarget;