Bug 75063

Summary: Start extracting platform specific bits out of PlatformEvents
Product: WebKit Reporter: Sam Weinig <sam>
Component: New BugsAssignee: Sam Weinig <sam>
Status: RESOLVED FIXED    
Severity: Normal CC: dglazkov, gustavo.noronha, gustavo, rniwa, webkit.review.bot, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch andersca: review+

Description Sam Weinig 2011-12-21 21:34:33 PST
Start extracting platform specific bits out of PlatformEvents
Comment 1 Sam Weinig 2011-12-21 21:35:03 PST
Created attachment 120268 [details]
Patch
Comment 2 Sam Weinig 2011-12-21 21:39:02 PST
This initial patch moves all the NSEvent -> PlatformEvent construction into a new factory style construction model.  The long term goal is to merge the WebKit2 WebEvents with these cleaned up PlatformEvents.
Comment 3 Sam Weinig 2011-12-22 13:16:23 PST
Created attachment 120369 [details]
Patch
Comment 4 WebKit Review Bot 2011-12-22 13:46:48 PST
Comment on attachment 120369 [details]
Patch

Attachment 120369 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11019012
Comment 5 Sam Weinig 2011-12-22 13:48:01 PST
Created attachment 120375 [details]
Patch
Comment 6 Gustavo Noronha (kov) 2011-12-22 13:58:40 PST
Comment on attachment 120375 [details]
Patch

Attachment 120375 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/10955033
Comment 7 Early Warning System Bot 2011-12-22 14:09:42 PST
Comment on attachment 120375 [details]
Patch

Attachment 120375 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/11016030
Comment 8 WebKit Review Bot 2011-12-22 14:11:51 PST
Comment on attachment 120375 [details]
Patch

Attachment 120375 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10939028
Comment 9 Sam Weinig 2011-12-22 14:37:36 PST
Created attachment 120383 [details]
Patch
Comment 10 WebKit Review Bot 2011-12-22 14:40:12 PST
Attachment 120369 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebCore/platform/mac/PlatformEventFactory.h:31:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 24 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 11 Sam Weinig 2011-12-22 14:47:08 PST
Created attachment 120385 [details]
Patch
Comment 12 WebKit Review Bot 2011-12-22 14:53:53 PST
Attachment 120385 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebCore/platform/mac/PlatformEventFactory.h:31:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 31 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 13 Early Warning System Bot 2011-12-22 14:58:07 PST
Comment on attachment 120385 [details]
Patch

Attachment 120385 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/11019054
Comment 14 WebKit Review Bot 2011-12-22 15:10:22 PST
Comment on attachment 120385 [details]
Patch

Attachment 120385 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10939059
Comment 15 Collabora GTK+ EWS bot 2011-12-22 15:20:48 PST
Comment on attachment 120385 [details]
Patch

Attachment 120385 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/10955074
Comment 16 Sam Weinig 2011-12-22 20:12:51 PST
Created attachment 120428 [details]
Patch
Comment 17 WebKit Review Bot 2011-12-22 20:15:34 PST
Attachment 120428 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebCore/platform/mac/PlatformEventFactory.h:31:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 32 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 18 Gustavo Noronha (kov) 2011-12-22 20:29:06 PST
Comment on attachment 120428 [details]
Patch

Attachment 120428 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/10962149
Comment 19 WebKit Review Bot 2011-12-22 20:34:08 PST
Comment on attachment 120428 [details]
Patch

Attachment 120428 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11019152
Comment 20 Sam Weinig 2011-12-22 20:35:45 PST
Created attachment 120431 [details]
Patch
Comment 21 Sam Weinig 2011-12-22 20:47:08 PST
I am not sure if I like the 4 factory functions.  I might be happier exposing the interfaces for the builder classes so that the header had classes like:

    class NativePlatformMouseEvent : public PlatformMouseEvent {
    public:
        NativePlatformMouseEvent(NSEvent *event, NSView *windowView);
    };

instead of:

    static PlatformMouseEvent createPlatformMouseEvent(NSEvent *, NSView *windowView);

and call sites were:

    foo->handleMouseEvent(NativePlatformMouseEvent(cocoaEvent, view));

instead of:

    foo->handleMouseEvent(PlatformEventFactory::createPlatformMouseEvent(cocoaEvent, view));

Hrm.
Comment 22 Sam Weinig 2011-12-23 19:46:00 PST
Committed r103643: <http://trac.webkit.org/changeset/103643>
Comment 23 Ryosuke Niwa 2011-12-23 21:27:33 PST
This patch appears to have broken fast/events/continuous-platform-wheelevent-in-scrolling-div.html and fast/events/platform-wheelevent-in-scrolling-div.html on Mac:

Started failing here:
http://build.webkit.org/builders/SnowLeopard%20Intel%20Release%20(Tests)/builds/35834
http://build.webkit.org/results/SnowLeopard%20Intel%20Release%20(Tests)/r103643%20(35834)/results.html

Didn't fail on the previous build:
http://build.webkit.org/results/SnowLeopard%20Intel%20Release%20(Tests)/r103641%20(35833)/results.html
Comment 24 Ryosuke Niwa 2011-12-23 21:48:34 PST
This patch also broke Chromium Mac builds:
http://build.webkit.org/builders/Chromium%20Mac%20Release/builds/28684/steps/compile-webkit/logs/stdio
Comment 25 Ryosuke Niwa 2011-12-23 22:35:22 PST
Fixed Chromium Mac build in http://trac.webkit.org/changeset/103652.