Bug 168376 - WKWebView inside a UICollectionView flashes when items are inserted above it
Summary: WKWebView inside a UICollectionView flashes when items are inserted above it
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tim Horton
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-02-15 09:52 PST by Tim Horton
Modified: 2017-02-16 13:12 PST (History)
2 users (show)

See Also:


Attachments
Patch (18.44 KB, patch)
2017-02-15 09:53 PST, Tim Horton
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews125 for ios-simulator-wk2 (843.58 KB, application/zip)
2017-02-15 11:08 PST, Build Bot
no flags Details
Patch (18.44 KB, patch)
2017-02-15 12:24 PST, Tim Horton
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews124 for ios-simulator-wk2 (1.22 MB, application/zip)
2017-02-15 13:18 PST, Build Bot
no flags Details
Patch (28.31 KB, patch)
2017-02-15 15:50 PST, Tim Horton
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews126 for ios-simulator-wk2 (15.77 MB, application/zip)
2017-02-15 17:02 PST, Build Bot
no flags Details
Patch (30.46 KB, patch)
2017-02-15 18:50 PST, Tim Horton
no flags Details | Formatted Diff | Diff
Patch (30.51 KB, patch)
2017-02-15 19:36 PST, Tim Horton
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews124 for ios-simulator-wk2 (1.11 MB, application/zip)
2017-02-15 20:51 PST, Build Bot
no flags Details
Patch (30.64 KB, patch)
2017-02-16 12:29 PST, Tim Horton
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tim Horton 2017-02-15 09:52:55 PST
WKWebView inside a UICollectionView flashes when items are inserted above it
Comment 1 Tim Horton 2017-02-15 09:53:31 PST
Created attachment 301627 [details]
Patch
Comment 2 Build Bot 2017-02-15 11:07:57 PST
Comment on attachment 301627 [details]
Patch

Attachment 301627 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3122794

New failing tests:
scrollingcoordinator/ios/sync-layer-positions-after-scroll.html
scrollingcoordinator/ios/non-stable-viewport-scroll.html
Comment 3 Build Bot 2017-02-15 11:08:01 PST
Created attachment 301634 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6
Comment 4 Tim Horton 2017-02-15 11:12:45 PST
Gonna need Simon's help with that one.
Comment 5 Tim Horton 2017-02-15 12:24:38 PST
Created attachment 301643 [details]
Patch
Comment 6 Build Bot 2017-02-15 13:18:06 PST
Comment on attachment 301643 [details]
Patch

Attachment 301643 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3123568

New failing tests:
fast/text-autosizing/ios/autosize-width.html
Comment 7 Build Bot 2017-02-15 13:18:08 PST
Created attachment 301646 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6
Comment 8 Tim Horton 2017-02-15 15:50:45 PST
Created attachment 301668 [details]
Patch
Comment 9 WebKit Commit Bot 2017-02-15 15:53:47 PST
Attachment 301668 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:292:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 1 in 18 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 10 Simon Fraser (smfr) 2017-02-15 16:00:29 PST
Comment on attachment 301668 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=301668&action=review

> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:2216
> +        [webView _updateContentRectsWithState:!webView->_hasScheduledVisibleContentRectUpdateFromScrollViewInUnstableState];

It seems a little silly to pass state to _updateContentRectsWithState: which webView has internally.

> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:2322
> +        _visibleContentRectUpdateCallbacks.takeLast()();

Took me two reads to see ()()
Maybe auto callback = _visibleContentRectUpdateCallbacks.takeLast();
callback();
Comment 11 Tim Horton 2017-02-15 16:04:12 PST
(In reply to comment #10)
> Comment on attachment 301668 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=301668&action=review
> 
> > Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:2216
> > +        [webView _updateContentRectsWithState:!webView->_hasScheduledVisibleContentRectUpdateFromScrollViewInUnstableState];
> 
> It seems a little silly to pass state to _updateContentRectsWithState: which
> webView has internally.
> 
> > Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:2322
> > +        _visibleContentRectUpdateCallbacks.takeLast()();
> 
> Took me two reads to see ()()
> Maybe auto callback = _visibleContentRectUpdateCallbacks.takeLast();
> callback();

Both fair points, I will fix them (and wait for the bots).
Comment 12 Build Bot 2017-02-15 17:02:24 PST
Comment on attachment 301668 [details]
Patch

Attachment 301668 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3125027

New failing tests:
scrollingcoordinator/ios/sync-layer-positions-after-scroll.html
pageoverlay/overlay-remove-reinsert-view.html
Comment 13 Build Bot 2017-02-15 17:02:29 PST
Created attachment 301675 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6
Comment 14 Tim Horton 2017-02-15 18:50:12 PST
Created attachment 301686 [details]
Patch
Comment 15 Simon Fraser (smfr) 2017-02-15 18:52:05 PST
Comment on attachment 301686 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=301686&action=review

> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:2212
> +        return;
> +    _hasInstalledPreCommitHandlerForVisibleRectUpdate = YES;

Blank line after the return please.
Comment 16 Tim Horton 2017-02-15 19:36:40 PST
Created attachment 301690 [details]
Patch
Comment 17 WebKit Commit Bot 2017-02-15 19:39:05 PST
Attachment 301690 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:292:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 1 in 19 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 18 Build Bot 2017-02-15 20:51:19 PST
Comment on attachment 301690 [details]
Patch

Attachment 301690 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3126051

New failing tests:
fast/text-autosizing/ios/autosize-width.html
Comment 19 Build Bot 2017-02-15 20:51:26 PST
Created attachment 301698 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6
Comment 20 Tim Horton 2017-02-15 21:06:57 PST
This one makes even less sense.
Comment 21 Tim Horton 2017-02-16 12:29:49 PST
Created attachment 301798 [details]
Patch
Comment 22 WebKit Commit Bot 2017-02-16 12:33:13 PST
Attachment 301798 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:292:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 1 in 19 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 23 Tim Horton 2017-02-16 13:12:50 PST
https://trac.webkit.org/changeset/212459