[webkit-changes] [284997] trunk/Source/WebKit

Thu, 28 Oct 2021 13:26:37 -0700

Title: [284997] trunk/Source/WebKit
Revision
284997
Author
wenson_hs...@apple.com
Date
2021-10-28 13:26:28 -0700 (Thu, 28 Oct 2021)

Log Message

Add WebPageProxy identifiers to `ViewGestures` category logging
https://bugs.webkit.org/show_bug.cgi?id=232437

Reviewed by Tim Horton.

Make it easier to correlate release logging from "ViewGestures" and "ViewState" categories in WebKit2 by having
"ViewGestures" logging include "pageProxyID=...", instead of just the content view pointer. This makes it more
straightforward to (for instance) detect when the user is trying to interact with a page that has stopped
receiving layer tree commits in the UI process for an extended period of time.

Note that in all of the below codepaths, we either already call into `_page`, or they're called as a result of
gesture recognizer delegate methods that should never be invoked by WebKit clients prior to common
initialization (where we initialize `_page`), so null checks for `_page` are unnecessary here.

* UIProcess/API/ios/WKWebViewIOS.mm:
(-[WKWebView _didRelaunchProcess]):

Also log the web process pid here, to make it easier to correlate logging in WKWebView with logs from the
corresponding web process.

* UIProcess/ios/WKContentViewInteraction.h:

Drive-by fix: also rename `m_commitPotentialTapPointerId` to `_commitPotentialTapPointerId`, so that it is
consistent with the other Objective-C ivar names.

* UIProcess/ios/WKContentViewInteraction.mm:
(-[WKContentView _handleSmartMagnificationInformationForPotentialTap:renderRect:fitEntireRect:viewportMinimumScale:viewportMaximumScale:nodeIsRootLevel:]):
(-[WKContentView _endPotentialTapAndEnableDoubleTapGesturesIfNecessary]):
(-[WKContentView _singleTapIdentified:]):
(-[WKContentView _singleTapDidReset:]):
(-[WKContentView _doubleTapDidFail:]):
(-[WKContentView _commitPotentialTapFailed]):
(-[WKContentView _didCompleteSyntheticClick]):
(-[WKContentView _singleTapRecognized:]):
(-[WKContentView _doubleTapRecognized:]):

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (284996 => 284997)


--- trunk/Source/WebKit/ChangeLog	2021-10-28 20:22:21 UTC (rev 284996)
+++ trunk/Source/WebKit/ChangeLog	2021-10-28 20:26:28 UTC (rev 284997)
@@ -1,3 +1,41 @@
+2021-10-28  Wenson Hsieh  <wenson_hs...@apple.com>
+
+        Add WebPageProxy identifiers to `ViewGestures` category logging
+        https://bugs.webkit.org/show_bug.cgi?id=232437
+
+        Reviewed by Tim Horton.
+
+        Make it easier to correlate release logging from "ViewGestures" and "ViewState" categories in WebKit2 by having
+        "ViewGestures" logging include "pageProxyID=...", instead of just the content view pointer. This makes it more
+        straightforward to (for instance) detect when the user is trying to interact with a page that has stopped
+        receiving layer tree commits in the UI process for an extended period of time.
+
+        Note that in all of the below codepaths, we either already call into `_page`, or they're called as a result of
+        gesture recognizer delegate methods that should never be invoked by WebKit clients prior to common
+        initialization (where we initialize `_page`), so null checks for `_page` are unnecessary here.
+
+        * UIProcess/API/ios/WKWebViewIOS.mm:
+        (-[WKWebView _didRelaunchProcess]):
+
+        Also log the web process pid here, to make it easier to correlate logging in WKWebView with logs from the
+        corresponding web process.
+
+        * UIProcess/ios/WKContentViewInteraction.h:
+
+        Drive-by fix: also rename `m_commitPotentialTapPointerId` to `_commitPotentialTapPointerId`, so that it is
+        consistent with the other Objective-C ivar names.
+
+        * UIProcess/ios/WKContentViewInteraction.mm:
+        (-[WKContentView _handleSmartMagnificationInformationForPotentialTap:renderRect:fitEntireRect:viewportMinimumScale:viewportMaximumScale:nodeIsRootLevel:]):
+        (-[WKContentView _endPotentialTapAndEnableDoubleTapGesturesIfNecessary]):
+        (-[WKContentView _singleTapIdentified:]):
+        (-[WKContentView _singleTapDidReset:]):
+        (-[WKContentView _doubleTapDidFail:]):
+        (-[WKContentView _commitPotentialTapFailed]):
+        (-[WKContentView _didCompleteSyntheticClick]):
+        (-[WKContentView _singleTapRecognized:]):
+        (-[WKContentView _doubleTapRecognized:]):
+
 2021-10-28  Tim Horton  <timothy_hor...@apple.com>
 
         Unreviewed build fix.

Modified: trunk/Source/WebKit/UIProcess/API/ios/WKWebViewIOS.mm (284996 => 284997)


--- trunk/Source/WebKit/UIProcess/API/ios/WKWebViewIOS.mm	2021-10-28 20:22:21 UTC (rev 284996)
+++ trunk/Source/WebKit/UIProcess/API/ios/WKWebViewIOS.mm	2021-10-28 20:26:28 UTC (rev 284997)
@@ -732,7 +732,7 @@
 
 - (void)_didRelaunchProcess
 {
-    WKWEBVIEW_RELEASE_LOG("%p -[WKWebView _didRelaunchProcess]", self);
+    WKWEBVIEW_RELEASE_LOG("%p -[WKWebView _didRelaunchProcess] (pid=%d)", self, _page->processIdentifier());
     _hasScheduledVisibleRectUpdate = NO;
     _viewStabilityWhenVisibleContentRectUpdateScheduled = { };
     if (_gestureController)

Modified: trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h (284996 => 284997)


--- trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h	2021-10-28 20:22:21 UTC (rev 284996)
+++ trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h	2021-10-28 20:26:28 UTC (rev 284997)
@@ -429,7 +429,7 @@
     BOOL _treatAsContentEditableUntilNextEditorStateUpdate;
     bool _isWaitingOnPositionInformation;
 
-    WebCore::PointerID m_commitPotentialTapPointerId;
+    WebCore::PointerID _commitPotentialTapPointerId;
 
     BOOL _keyboardDidRequestDismissal;

Modified: trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm (284996 => 284997)


--- trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm	2021-10-28 20:22:21 UTC (rev 284996)
+++ trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm	2021-10-28 20:26:28 UTC (rev 284997)
@@ -2164,18 +2164,18 @@
     // We check both the system preference and the page preference, because we only want this
     // to apply in "desktop" mode.
     if (preferences.preferFasterClickOverDoubleTap() && _page->preferFasterClickOverDoubleTap()) {
-        RELEASE_LOG(ViewGestures, "Potential tap found an element and fast taps are preferred. Trigger click. (%p)", self);
+        RELEASE_LOG(ViewGestures, "Potential tap found an element and fast taps are preferred. Trigger click. (%p, pageProxyID=%llu)", self, _page->identifier().toUInt64());
         if (preferences.zoomOnDoubleTapWhenRoot() && nodeIsRootLevel) {
-            RELEASE_LOG(ViewGestures, "The click handler was on a root-level element, so don't disable double-tap. (%p)", self);
+            RELEASE_LOG(ViewGestures, "The click handler was on a root-level element, so don't disable double-tap. (%p, pageProxyID=%llu)", self, _page->identifier().toUInt64());
             return;
         }
 
         if (preferences.alwaysZoomOnDoubleTap()) {
-            RELEASE_LOG(ViewGestures, "DTTZ is forced on, so don't disable double-tap. (%p)", self);
+            RELEASE_LOG(ViewGestures, "DTTZ is forced on, so don't disable double-tap. (%p, pageProxyID=%llu)", self, _page->identifier().toUInt64());
             return;
         }
 
-        RELEASE_LOG(ViewGestures, "Give preference to click by disabling double-tap. (%p)", self);
+        RELEASE_LOG(ViewGestures, "Give preference to click by disabling double-tap. (%p, pageProxyID=%llu)", self, _page->identifier().toUInt64());
         [self _setDoubleTapGesturesEnabled:NO];
         return;
     }
@@ -2183,11 +2183,11 @@
     auto currentScale = self._contentZoomScale;
     auto targetScale = _smartMagnificationController->zoomFactorForTargetRect(renderRect, fitEntireRect, viewportMinimumScale, viewportMaximumScale);
     if (std::min(targetScale, currentScale) / std::max(targetScale, currentScale) > fasterTapSignificantZoomThreshold) {
-        RELEASE_LOG(ViewGestures, "Potential tap would not cause a significant zoom. Trigger click. (%p)", self);
+        RELEASE_LOG(ViewGestures, "Potential tap would not cause a significant zoom. Trigger click. (%p, pageProxyID=%llu)", self, _page->identifier().toUInt64());
         [self _setDoubleTapGesturesEnabled:NO];
         return;
     }
-    RELEASE_LOG(ViewGestures, "Potential tap may cause significant zoom. Wait. (%p)", self);
+    RELEASE_LOG(ViewGestures, "Potential tap may cause significant zoom. Wait. (%p, pageProxyID=%llu)", self, _page->identifier().toUInt64());
 }
 
 - (void)_cancelLongPressGestureRecognizer
@@ -3084,12 +3084,12 @@
 - (void)_endPotentialTapAndEnableDoubleTapGesturesIfNecessary
 {
     if (self.webView._allowsDoubleTapGestures) {
-        RELEASE_LOG(ViewGestures, "ending potential tap - double taps are back. (%p)", self);
+        RELEASE_LOG(ViewGestures, "ending potential tap - double taps are back. (%p, pageProxyID=%llu)", self, _page->identifier().toUInt64());
 
         [self _setDoubleTapGesturesEnabled:YES];
     }
 
-    RELEASE_LOG(ViewGestures, "Ending potential tap. (%p)", self);
+    RELEASE_LOG(ViewGestures, "Ending potential tap. (%p, pageProxyID=%llu)", self, _page->identifier().toUInt64());
 
     _potentialTapInProgress = NO;
 }
@@ -3105,7 +3105,7 @@
 
     bool shouldRequestMagnificationInformation = _page->preferences().fasterClicksEnabled();
     if (shouldRequestMagnificationInformation)
-        RELEASE_LOG(ViewGestures, "Single tap identified. Request details on potential zoom. (%p)", self);
+        RELEASE_LOG(ViewGestures, "Single tap identified. Request details on potential zoom. (%p, pageProxyID=%llu)", self, _page->identifier().toUInt64());
 
     _page->potentialTapAtPosition(gestureRecognizer.location, shouldRequestMagnificationInformation, [self nextTapIdentifier]);
     _potentialTapInProgress = YES;
@@ -3128,7 +3128,7 @@
     cancelPotentialTapIfNecessary(self);
     if (auto* singleTapTouchIdentifier = [_singleTapGestureRecognizer lastActiveTouchIdentifier]) {
         WebCore::PointerID pointerId = [singleTapTouchIdentifier unsignedIntValue];
-        if (m_commitPotentialTapPointerId != pointerId)
+        if (_commitPotentialTapPointerId != pointerId)
             _page->touchWithIdentifierWasRemoved(pointerId);
     }
 
@@ -3138,14 +3138,14 @@
 
 - (void)_doubleTapDidFail:(UITapGestureRecognizer *)gestureRecognizer
 {
-    RELEASE_LOG(ViewGestures, "Double tap was not recognized. (%p)", self);
+    RELEASE_LOG(ViewGestures, "Double tap was not recognized. (%p, pageProxyID=%llu)", self, _page->identifier().toUInt64());
     ASSERT(gestureRecognizer == _doubleTapGestureRecognizer);
 }
 
 - (void)_commitPotentialTapFailed
 {
-    _page->touchWithIdentifierWasRemoved(m_commitPotentialTapPointerId);
-    m_commitPotentialTapPointerId = 0;
+    _page->touchWithIdentifierWasRemoved(_commitPotentialTapPointerId);
+    _commitPotentialTapPointerId = 0;
 
     [self _cancelInteraction];
     
@@ -3165,10 +3165,10 @@
 
 - (void)_didCompleteSyntheticClick
 {
-    _page->touchWithIdentifierWasRemoved(m_commitPotentialTapPointerId);
-    m_commitPotentialTapPointerId = 0;
+    _page->touchWithIdentifierWasRemoved(_commitPotentialTapPointerId);
+    _commitPotentialTapPointerId = 0;
 
-    RELEASE_LOG(ViewGestures, "Synthetic click completed. (%p)", self);
+    RELEASE_LOG(ViewGestures, "Synthetic click completed. (%p, pageProxyID=%llu)", self, _page->identifier().toUInt64());
     [self _resetInputViewDeferral];
 }
 
@@ -3193,12 +3193,12 @@
     if ([_inputPeripheral singleTapShouldEndEditing])
         [_inputPeripheral endEditing];
 
-    RELEASE_LOG(ViewGestures, "Single tap recognized - commit potential tap (%p)", self);
+    RELEASE_LOG(ViewGestures, "Single tap recognized - commit potential tap (%p, pageProxyID=%llu)", self, _page->identifier().toUInt64());
 
     WebCore::PointerID pointerId = WebCore::mousePointerID;
     if (auto* singleTapTouchIdentifier = [_singleTapGestureRecognizer lastActiveTouchIdentifier]) {
         pointerId = [singleTapTouchIdentifier unsignedIntValue];
-        m_commitPotentialTapPointerId = pointerId;
+        _commitPotentialTapPointerId = pointerId;
     }
     _page->commitPotentialTap(WebKit::webEventModifierFlags(gestureRecognizer.modifierFlags), _layerTreeTransactionIdAtLastInteractionStart, pointerId);
 
@@ -3208,7 +3208,7 @@
 
 - (void)_doubleTapRecognized:(UITapGestureRecognizer *)gestureRecognizer
 {
-    RELEASE_LOG(ViewGestures, "Identified a double tap (%p)", self);
+    RELEASE_LOG(ViewGestures, "Identified a double tap (%p, pageProxyID=%llu)", self, _page->identifier().toUInt64());
 
     [self _resetIsDoubleTapPending];
     _lastInteractionLocation = gestureRecognizer.location;

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to