[iOS][WK2] When the secondary UIScrollView delegates respond to callbacks, delay...
authorbenjamin@webkit.org <benjamin@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 13 May 2014 22:47:55 +0000 (22:47 +0000)
committerbenjamin@webkit.org <benjamin@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 13 May 2014 22:47:55 +0000 (22:47 +0000)
https://bugs.webkit.org/show_bug.cgi?id=132849
<rdar://problem/16863716>

Patch by Benjamin Poulain <bpoulain@apple.com> on 2014-05-13
Reviewed by Anders Carlsson.

When there are two delegates responding to UIScrollView changes, there was often an intermediate invalid
state forwarded to the UIProcess.

For example, on scroll, WKWebView would update the state based on the current obscured insets in response to
delegate callbacks.
After that update, Safari would modify the insets, thus updating the state again.
The first state changed by WKWebView is incomplete, and should never have been set.

This patch works around the issue by delaying visible update rect in those cases.

When the two delegates of WKScrollView respond to the same selector, WKScrollView invokes
[UIWebView _willInvokeUIScrollViewDelegateCallback] on entry, and
[UIWebView _didInvokeUIScrollViewDelegateCallback] on exit.

Between those two calls, WKWebView does not forward the new UI state to the WebProcess.

* UIProcess/API/Cocoa/WKWebView.mm:
(-[WKWebView _willInvokeUIScrollViewDelegateCallback]):
(-[WKWebView _didInvokeUIScrollViewDelegateCallback]):
(-[WKWebView _updateVisibleContentRects]):
* UIProcess/API/Cocoa/WKWebViewInternal.h:
* UIProcess/API/ios/WKViewIOS.mm:
(-[WKView _commonInitializationWithContextRef:pageGroupRef:relatedToPage:]):
* UIProcess/ios/WKScrollView.h:
* UIProcess/ios/WKScrollView.mm:
(-[WKScrollViewDelegateForwarder initWithInternalDelegate:externalDelegate:]):
(-[WKScrollViewDelegateForwarder forwardInvocation:]):
(-[WKScrollView setInternalDelegate:]):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@168747 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit2/ChangeLog
Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm
Source/WebKit2/UIProcess/API/Cocoa/WKWebViewInternal.h
Source/WebKit2/UIProcess/API/ios/WKViewIOS.mm
Source/WebKit2/UIProcess/ios/WKScrollView.h
Source/WebKit2/UIProcess/ios/WKScrollView.mm

index 6dc90c7..c23d18e 100644 (file)
@@ -1,5 +1,42 @@
 2014-05-13  Benjamin Poulain  <bpoulain@apple.com>
 
+        [iOS][WK2] When the secondary UIScrollView delegates respond to callbacks, delay the state change until both delegates have finished
+        https://bugs.webkit.org/show_bug.cgi?id=132849
+        <rdar://problem/16863716>
+
+        Reviewed by Anders Carlsson.
+
+        When there are two delegates responding to UIScrollView changes, there was often an intermediate invalid
+        state forwarded to the UIProcess.
+
+        For example, on scroll, WKWebView would update the state based on the current obscured insets in response to
+        delegate callbacks.
+        After that update, Safari would modify the insets, thus updating the state again.
+        The first state changed by WKWebView is incomplete, and should never have been set.
+
+        This patch works around the issue by delaying visible update rect in those cases.
+
+        When the two delegates of WKScrollView respond to the same selector, WKScrollView invokes 
+        [UIWebView _willInvokeUIScrollViewDelegateCallback] on entry, and
+        [UIWebView _didInvokeUIScrollViewDelegateCallback] on exit.
+
+        Between those two calls, WKWebView does not forward the new UI state to the WebProcess.
+
+        * UIProcess/API/Cocoa/WKWebView.mm:
+        (-[WKWebView _willInvokeUIScrollViewDelegateCallback]):
+        (-[WKWebView _didInvokeUIScrollViewDelegateCallback]):
+        (-[WKWebView _updateVisibleContentRects]):
+        * UIProcess/API/Cocoa/WKWebViewInternal.h:
+        * UIProcess/API/ios/WKViewIOS.mm:
+        (-[WKView _commonInitializationWithContextRef:pageGroupRef:relatedToPage:]):
+        * UIProcess/ios/WKScrollView.h:
+        * UIProcess/ios/WKScrollView.mm:
+        (-[WKScrollViewDelegateForwarder initWithInternalDelegate:externalDelegate:]):
+        (-[WKScrollViewDelegateForwarder forwardInvocation:]):
+        (-[WKScrollView setInternalDelegate:]):
+
+2014-05-13  Benjamin Poulain  <bpoulain@apple.com>
+
         [iOS][WK2] Remove the _extendedBackgroundExclusionInsets SPI
         https://bugs.webkit.org/show_bug.cgi?id=132848
         <rdar://problem/16875093>
index 62e4420..f596417 100644 (file)
     RetainPtr<UIView <WKWebViewContentProvider>> _customContentView;
 
     WebCore::Color _scrollViewBackgroundColor;
+
+    BOOL _delayUpdateVisibleContentRects;
+    BOOL _hadDelayedUpdateVisibleContentRects;
 #endif
 #if PLATFORM(MAC)
     RetainPtr<WKView> _wkView;
     [_customContentView web_setContentProviderData:data suggestedFilename:suggestedFilename];
 }
 
+- (void)_willInvokeUIScrollViewDelegateCallback
+{
+    _delayUpdateVisibleContentRects = YES;
+}
+
+- (void)_didInvokeUIScrollViewDelegateCallback
+{
+    _delayUpdateVisibleContentRects = NO;
+    if (_hadDelayedUpdateVisibleContentRects) {
+        _hadDelayedUpdateVisibleContentRects = NO;
+        [self _updateVisibleContentRects];
+    }
+}
+
 static CGFloat contentZoomScale(WKWebView* webView)
 {
     UIView *zoomView;
@@ -883,6 +900,11 @@ static inline void setViewportConfigurationMinimumLayoutSize(WebKit::WebPageProx
     if (![self usesStandardContentView])
         return;
 
+    if (_delayUpdateVisibleContentRects) {
+        _hadDelayedUpdateVisibleContentRects = YES;
+        return;
+    }
+
     if (_isAnimatingResize)
         return;
 
index 377cf83..79386d7 100644 (file)
@@ -73,6 +73,9 @@ struct ViewSnapshot;
 
 - (void)_setHasCustomContentView:(BOOL)hasCustomContentView loadedMIMEType:(const WTF::String&)mimeType;
 - (void)_didFinishLoadingDataForCustomContentProviderWithSuggestedFilename:(const WTF::String&)suggestedFilename data:(NSData *)data;
+
+- (void)_willInvokeUIScrollViewDelegateCallback;
+- (void)_didInvokeUIScrollViewDelegateCallback;
 #endif
 @end
 
index fd986bf..670572c 100644 (file)
@@ -221,7 +221,6 @@ using namespace WebKit;
     CGRect bounds = self.bounds;
 
     _scrollView = adoptNS([[WKScrollView alloc] initWithFrame:bounds]);
-    [_scrollView setInternalDelegate:self];
     [_scrollView setBouncesZoom:YES];
 
     [self addSubview:_scrollView.get()];
index 93ca9be..a87f475 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2013 Apple Inc. All rights reserved.
+ * Copyright (C) 2013, 2014 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
 
 #import <UIKit/UIWebScrollView.h>
 
+@class WKWebView;
+
 @interface WKScrollView : UIWebScrollView
 
-@property (nonatomic, assign) id <UIScrollViewDelegate> internalDelegate;
+@property (nonatomic, assign) WKWebView <UIScrollViewDelegate> *internalDelegate;
 
 @end
 
index f41210c..6b90bc3 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2013 Apple Inc. All rights reserved.
+ * Copyright (C) 2013, 2014 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -28,6 +28,7 @@
 
 #if PLATFORM(IOS)
 
+#import "WKWebViewInternal.h"
 #import <CoreGraphics/CGFloat.h>
 
 @interface UIScrollView (UIScrollViewInternalHack)
 
 @interface WKScrollViewDelegateForwarder : NSObject <UIScrollViewDelegate>
 
-- (instancetype)initWithInternalDelegate:(id <UIScrollViewDelegate>)internalDelegate externalDelegate:(id <UIScrollViewDelegate>)externalDelegate;
+- (instancetype)initWithInternalDelegate:(WKWebView *)internalDelegate externalDelegate:(id <UIScrollViewDelegate>)externalDelegate;
 
 @end
 
 @implementation WKScrollViewDelegateForwarder {
-    id <UIScrollViewDelegate> _internalDelegate;
+    WKWebView *_internalDelegate;
     id <UIScrollViewDelegate> _externalDelegate;
 }
 
-- (instancetype)initWithInternalDelegate:(id <UIScrollViewDelegate>)internalDelegate externalDelegate:(id <UIScrollViewDelegate>)externalDelegate
+- (instancetype)initWithInternalDelegate:(WKWebView <UIScrollViewDelegate> *)internalDelegate externalDelegate:(id <UIScrollViewDelegate>)externalDelegate
 {
     self = [super init];
     if (!self)
 
 - (void)forwardInvocation:(NSInvocation *)anInvocation
 {
-    BOOL messageHandled = NO;
-    if ([_internalDelegate respondsToSelector:[anInvocation selector]]) {
+    SEL aSelector = [anInvocation selector];
+    BOOL internalDelegateWillRespond = [_internalDelegate respondsToSelector:aSelector];
+    BOOL externalDelegateWillRespond = [_externalDelegate respondsToSelector:aSelector];
+
+    if (internalDelegateWillRespond && externalDelegateWillRespond)
+        [_internalDelegate _willInvokeUIScrollViewDelegateCallback];
+
+    if (internalDelegateWillRespond)
         [anInvocation invokeWithTarget:_internalDelegate];
-        messageHandled = YES;
-    }
-    if ([_externalDelegate respondsToSelector:[anInvocation selector]]) {
+    if (externalDelegateWillRespond)
         [anInvocation invokeWithTarget:_externalDelegate];
-        messageHandled = YES;
-    }
-    if (!messageHandled)
+
+    if (internalDelegateWillRespond && externalDelegateWillRespond)
+        [_internalDelegate _didInvokeUIScrollViewDelegateCallback];
+
+    if (!internalDelegateWillRespond && !externalDelegateWillRespond)
         [super forwardInvocation:anInvocation];
 }
 
     WKScrollViewDelegateForwarder *_delegateForwarder;
 }
 
-- (void)setInternalDelegate:(id <UIScrollViewDelegate>)internalDelegate
+- (void)setInternalDelegate:(WKWebView <UIScrollViewDelegate> *)internalDelegate
 {
     if (internalDelegate == _internalDelegate)
         return;