Bug 173903

Summary: Web Replay: remove some unused code
Product: WebKit Reporter: BJ Burg <bburg>
Component: Web InspectorAssignee: BJ Burg <bburg>
Status: RESOLVED FIXED    
Severity: Normal CC: bburg, buildbot, dbates, inspector-bugzilla-changes, joepeck, rniwa
Priority: P2    
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews106 for mac-elcapitan-wk2
none
Patch
none
Archive of layout-test-results from ews106 for mac-elcapitan-wk2
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Archive of layout-test-results from ews104 for mac-elcapitan-wk2
none
For Landing
none
For Landing joepeck: review+

Description BJ Burg 2017-06-27 20:05:30 PDT
.
Comment 1 BJ Burg 2017-06-27 20:08:56 PDT
Created attachment 313978 [details]
Patch
Comment 2 Build Bot 2017-06-27 20:13:31 PDT
Attachment 313978 [details] did not pass style-queue:


ERROR: Source/WebCore/Configurations/FeatureDefines.xcconfig:0:  Any changes made to FeatureDefines should be made to all of them (changed file does not match Tools/TestWebKitAPI/Configurations/FeatureDefines.xcconfig). Use sync-feature-defines if possible.  [featuredefines/equality] [5]
ERROR: Source/WebCore/PAL/Configurations/FeatureDefines.xcconfig:0:  Any changes made to FeatureDefines should be made to all of them (changed file does not match Tools/TestWebKitAPI/Configurations/FeatureDefines.xcconfig). Use sync-feature-defines if possible.  [featuredefines/equality] [5]
ERROR: Source/WebKit/mac/Configurations/FeatureDefines.xcconfig:0:  Any changes made to FeatureDefines should be made to all of them (changed file does not match Tools/TestWebKitAPI/Configurations/FeatureDefines.xcconfig). Use sync-feature-defines if possible.  [featuredefines/equality] [5]
ERROR: Source/JavaScriptCore/Configurations/FeatureDefines.xcconfig:0:  Any changes made to FeatureDefines should be made to all of them (changed file does not match Tools/TestWebKitAPI/Configurations/FeatureDefines.xcconfig). Use sync-feature-defines if possible.  [featuredefines/equality] [5]
ERROR: Source/WebKit2/Configurations/FeatureDefines.xcconfig:0:  Any changes made to FeatureDefines should be made to all of them (changed file does not match Tools/TestWebKitAPI/Configurations/FeatureDefines.xcconfig). Use sync-feature-defines if possible.  [featuredefines/equality] [5]
Total errors found: 5 in 21 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Joseph Pecoraro 2017-06-27 20:23:22 PDT
Comment on attachment 313978 [details]
Patch

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

rs=me

> Source/JavaScriptCore/ChangeLog:9
> +        * Configurations/FeatureDefines.xcconfig:

This misses:
TestWebKitAPI/Configurations/FeatureDefines.xcconfig

> Source/WebCore/PlatformMac.cmake:-91
> -    "${WEBCORE_DIR}/ForwardingHeaders/replay"

You should be removing this directory as well. it contains:

./Source/WebCore/ForwardingHeaders/replay
./Source/WebCore/ForwardingHeaders/replay/EncodedValue.h
./Source/WebCore/ForwardingHeaders/replay/NondeterministicInput.h
./Source/WebCore/ForwardingHeaders/replay/InputCursor.h
./Source/WebCore/ForwardingHeaders/replay/EmptyInputCursor.h

> LayoutTests/ChangeLog:13
> +        * inspector/replay/window-navigator-plugins-memoized.html: Removed.

TestExpectations:

$ find-expectations 'inspector/replay'
LayoutTests/TestExpectations:184:webkit.org/b/148036 http/tests/inspector/replay/ [ Skip ]
LayoutTests/TestExpectations:201:webkit.org/b/137130 inspector/replay [ Skip ]
LayoutTests/TestExpectations:210:webkit.org/b/131318 http/tests/inspector/replay/document-last-modified-fallback-value.html [ Skip ]

Another directory of tests:
http/tests/inspector/replay
Comment 4 Build Bot 2017-06-27 21:39:44 PDT
Comment on attachment 313978 [details]
Patch

Attachment 313978 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/4011187

New failing tests:
fast/frames/iframe-window-focus.html
fast/scrolling/scroll-animator-select-list-events.html
fast/events/keyevent-iframe-removed-crash.html
fast/scrolling/scroll-animator-basic-events.html
fast/scrolling/scroll-animator-overlay-scrollbars-clicked.html
fast/events/keydown-1.html
fast/events/keypress-focus-change.html
editing/input/option-page-up-down.html
fast/events/recorded-keydown-event.html
pageoverlay/overlay-small-frame-mouse-events.html
editing/input/scroll-viewport-page-up-down.html
fast/scrolling/scroll-animator-overlay-scrollbars-hovered.html
fast/dom/access-key-iframe.html
Comment 5 Build Bot 2017-06-27 21:39:46 PDT
Created attachment 313986 [details]
Archive of layout-test-results from ews106 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6
Comment 6 BJ Burg 2017-06-29 11:30:53 PDT
Created attachment 314146 [details]
Patch
Comment 7 Build Bot 2017-06-29 12:57:39 PDT
Comment on attachment 314146 [details]
Patch

Attachment 314146 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/4020897

New failing tests:
pageoverlay/overlay-small-frame-mouse-events.html
fast/scrolling/scroll-animator-overlay-scrollbars-clicked.html
fast/scrolling/scroll-animator-basic-events.html
fast/scrolling/scroll-animator-select-list-events.html
fast/scrolling/scroll-animator-overlay-scrollbars-hovered.html
Comment 8 Build Bot 2017-06-29 12:57:40 PDT
Created attachment 314155 [details]
Archive of layout-test-results from ews106 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6
Comment 9 BJ Burg 2017-06-29 15:44:33 PDT
Created attachment 314178 [details]
Patch
Comment 10 Joseph Pecoraro 2017-06-29 16:47:53 PDT
Bots are failing to build:

> /usr/lib/ccache/c++ Source/WebCore/page/Page.cpp
> /home/ews/ltilve-gtk-wk2-ews/WebKit/Source/WebCore/page/Page.cpp:105:29: fatal error: UserInputBridge.h: No such file or directory
>  #include "UserInputBridge.h"
>                              ^
> compilation terminated.
Comment 11 BJ Burg 2017-06-30 09:55:52 PDT
Created attachment 314272 [details]
Patch
Comment 12 Joseph Pecoraro 2017-06-30 10:41:41 PDT
> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:2399:72: error: 'class WebCore::EventHandler' has no member named 'handleKeyEvent'; did you mean 'handleWheelEvent'?
>      return page->focusController().focusedOrMainFrame().eventHandler().handleKeyEvent(platform(keyboardEvent));
>                                                                        ^~~~~~~~~~~~~~
Comment 13 BJ Burg 2017-06-30 15:33:35 PDT
Created attachment 314306 [details]
Patch
Comment 14 BJ Burg 2017-07-03 16:17:46 PDT
Created attachment 314533 [details]
Patch
Comment 15 BJ Burg 2017-07-03 16:50:32 PDT
Created attachment 314538 [details]
Patch
Comment 16 Build Bot 2017-07-03 16:54:53 PDT
Attachment 314538 [details] did not pass style-queue:


ERROR: Source/WebCore/Configurations/FeatureDefines.xcconfig:0:  Any changes made to FeatureDefines should be made to all of them (changed file does not match Tools/TestWebKitAPI/Configurations/FeatureDefines.xcconfig). Use sync-feature-defines if possible.  [featuredefines/equality] [5]
ERROR: Source/WebCore/PAL/Configurations/FeatureDefines.xcconfig:0:  Any changes made to FeatureDefines should be made to all of them (changed file does not match Tools/TestWebKitAPI/Configurations/FeatureDefines.xcconfig). Use sync-feature-defines if possible.  [featuredefines/equality] [5]
ERROR: Source/WebKit/mac/Configurations/FeatureDefines.xcconfig:0:  Any changes made to FeatureDefines should be made to all of them (changed file does not match Tools/TestWebKitAPI/Configurations/FeatureDefines.xcconfig). Use sync-feature-defines if possible.  [featuredefines/equality] [5]
ERROR: Source/JavaScriptCore/Configurations/FeatureDefines.xcconfig:0:  Any changes made to FeatureDefines should be made to all of them (changed file does not match Tools/TestWebKitAPI/Configurations/FeatureDefines.xcconfig). Use sync-feature-defines if possible.  [featuredefines/equality] [5]
ERROR: Tools/TestWebKitAPI/Configurations/FeatureDefines.xcconfig:0:  Any changes made to FeatureDefines should be made to all of them (changed file does not match Source/WebKit2/Configurations/FeatureDefines.xcconfig). Use sync-feature-defines if possible.  [featuredefines/equality] [5]
Total errors found: 5 in 22 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 17 Build Bot 2017-07-03 18:17:53 PDT
Comment on attachment 314538 [details]
Patch

Attachment 314538 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/4047777

New failing tests:
pageoverlay/overlay-small-frame-mouse-events.html
fast/scrolling/scroll-animator-overlay-scrollbars-clicked.html
fast/scrolling/scroll-animator-basic-events.html
fast/scrolling/scroll-animator-select-list-events.html
fast/scrolling/scroll-animator-overlay-scrollbars-hovered.html
Comment 18 Build Bot 2017-07-03 18:17:55 PDT
Created attachment 314543 [details]
Archive of layout-test-results from ews104 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6
Comment 19 Joseph Pecoraro 2017-07-03 19:12:25 PDT
Comment on attachment 314538 [details]
Patch

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

> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:2311
> -                return page->corePage()->userInputBridge().handleMouseMoveOnScrollbarEvent(platformMouseEvent);
> -            return page->corePage()->userInputBridge().handleMouseMoveEvent(platformMouseEvent);
> +                return frame.eventHandler().passMouseMovedEventToScrollbars(platformMouseEvent);
> +            return frame.eventHandler().handleMouseMoveEvent(platformMouseEvent);

Interesting that these are using `frame`. It looks like UserInputBridge would have used the equivalent of `page->mainFrame()` for some of these. Its possible that using the specific frame is correct, in which case maybe these few failing tests need to be updated.
Comment 20 BJ Burg 2017-07-07 10:33:53 PDT
Created attachment 314853 [details]
For Landing
Comment 21 Build Bot 2017-07-07 10:38:43 PDT
Attachment 314853 [details] did not pass style-queue:


ERROR: Source/WebCore/Configurations/FeatureDefines.xcconfig:0:  Any changes made to FeatureDefines should be made to all of them (changed file does not match Tools/TestWebKitAPI/Configurations/FeatureDefines.xcconfig). Use sync-feature-defines if possible.  [featuredefines/equality] [5]
ERROR: Source/WebCore/PAL/Configurations/FeatureDefines.xcconfig:0:  Any changes made to FeatureDefines should be made to all of them (changed file does not match Tools/TestWebKitAPI/Configurations/FeatureDefines.xcconfig). Use sync-feature-defines if possible.  [featuredefines/equality] [5]
ERROR: Source/WebKit/mac/Configurations/FeatureDefines.xcconfig:0:  Any changes made to FeatureDefines should be made to all of them (changed file does not match Tools/TestWebKitAPI/Configurations/FeatureDefines.xcconfig). Use sync-feature-defines if possible.  [featuredefines/equality] [5]
ERROR: Source/JavaScriptCore/Configurations/FeatureDefines.xcconfig:0:  Any changes made to FeatureDefines should be made to all of them (changed file does not match Tools/TestWebKitAPI/Configurations/FeatureDefines.xcconfig). Use sync-feature-defines if possible.  [featuredefines/equality] [5]
ERROR: Tools/TestWebKitAPI/Configurations/FeatureDefines.xcconfig:0:  Any changes made to FeatureDefines should be made to all of them (changed file does not match Source/WebKit2/Configurations/FeatureDefines.xcconfig). Use sync-feature-defines if possible.  [featuredefines/equality] [5]
Total errors found: 5 in 21 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 22 BJ Burg 2017-07-07 13:10:52 PDT
Created attachment 314866 [details]
For Landing
Comment 23 Build Bot 2017-07-07 13:14:02 PDT
Attachment 314866 [details] did not pass style-queue:


ERROR: Source/WebCore/Configurations/FeatureDefines.xcconfig:0:  Any changes made to FeatureDefines should be made to all of them (changed file does not match Tools/TestWebKitAPI/Configurations/FeatureDefines.xcconfig). Use sync-feature-defines if possible.  [featuredefines/equality] [5]
ERROR: Source/WebCore/PAL/Configurations/FeatureDefines.xcconfig:0:  Any changes made to FeatureDefines should be made to all of them (changed file does not match Tools/TestWebKitAPI/Configurations/FeatureDefines.xcconfig). Use sync-feature-defines if possible.  [featuredefines/equality] [5]
ERROR: Source/WebKit/mac/Configurations/FeatureDefines.xcconfig:0:  Any changes made to FeatureDefines should be made to all of them (changed file does not match Tools/TestWebKitAPI/Configurations/FeatureDefines.xcconfig). Use sync-feature-defines if possible.  [featuredefines/equality] [5]
ERROR: Source/JavaScriptCore/Configurations/FeatureDefines.xcconfig:0:  Any changes made to FeatureDefines should be made to all of them (changed file does not match Tools/TestWebKitAPI/Configurations/FeatureDefines.xcconfig). Use sync-feature-defines if possible.  [featuredefines/equality] [5]
ERROR: Tools/TestWebKitAPI/Configurations/FeatureDefines.xcconfig:0:  Any changes made to FeatureDefines should be made to all of them (changed file does not match Source/WebKit2/Configurations/FeatureDefines.xcconfig). Use sync-feature-defines if possible.  [featuredefines/equality] [5]
Total errors found: 5 in 21 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 24 Joseph Pecoraro 2017-07-07 14:15:47 PDT
Comment on attachment 314866 [details]
For Landing

rs=me.

The style bot is complaining about mismatched xcconfigs, so we should resolve that.
Comment 25 BJ Burg 2017-07-07 14:27:04 PDT
(In reply to Joseph Pecoraro from comment #24)
> Comment on attachment 314866 [details]
> For Landing
> 
> rs=me.
> 
> The style bot is complaining about mismatched xcconfigs, so we should
> resolve that.

ENABLE_WEB_REPLAY never got into that feature defines file, so it's confused.
Comment 26 BJ Burg 2017-07-10 11:24:33 PDT
Committed r219301: <http://trac.webkit.org/changeset/219301>