Bug 126180

Summary: [iOS] Upstream WebCore/page changes
Product: WebKit Reporter: Daniel Bates <dbates>
Component: WebCore Misc.Assignee: Daniel Bates <dbates>
Status: RESOLVED FIXED    
Severity: Normal CC: aestes, benjamin, cdumez, cmarcelo, commit-queue, darin, ddkilzer, dino, dstockwell, eflews.bot, esprehn+autocc, gyuyoung.kim, kangil.han, kondapallykalyan, simon.fraser
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch darin: review+, eflews.bot: commit-queue-

Description Daniel Bates 2013-12-23 14:06:01 PST
Upstream the iOS related changes to WebCore/page.
Comment 1 Daniel Bates 2013-12-23 14:09:14 PST
Created attachment 219931 [details]
Patch
Comment 2 WebKit Commit Bot 2013-12-23 14:12:08 PST
Attachment 219931 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WTF/ChangeLog', u'Source/WTF/wtf/FeatureDefines.h', u'Source/WebCore/ChangeLog', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', u'Source/WebCore/dom/EventNames.h', u'Source/WebCore/page/AlternativeTextClient.h', u'Source/WebCore/page/Chrome.cpp', u'Source/WebCore/page/Chrome.h', u'Source/WebCore/page/ChromeClient.h', u'Source/WebCore/page/DOMTimer.cpp', u'Source/WebCore/page/DOMWindow.cpp', u'Source/WebCore/page/DOMWindow.h', u'Source/WebCore/page/DOMWindow.idl', u'Source/WebCore/page/EditorClient.h', u'Source/WebCore/page/EventHandler.cpp', u'Source/WebCore/page/EventHandler.h', u'Source/WebCore/page/FocusController.h', u'Source/WebCore/page/Frame.cpp', u'Source/WebCore/page/Frame.h', u'Source/WebCore/page/FrameView.cpp', u'Source/WebCore/page/FrameView.h', u'Source/WebCore/page/Navigator.cpp', u'Source/WebCore/page/Navigator.h', u'Source/WebCore/page/Navigator.idl', u'Source/WebCore/page/NavigatorBase.cpp', u'Source/WebCore/page/Page.h', u'Source/WebCore/page/PageGroup.cpp', u'Source/WebCore/page/PageGroup.h', u'Source/WebCore/page/Settings.cpp', u'Source/WebCore/page/Settings.h', u'Source/WebCore/page/Settings.in', u'Source/WebCore/page/animation/CSSPropertyAnimation.cpp', u'Source/WebCore/page/ios/EventHandlerIOS.mm', u'Source/WebCore/page/ios/FrameIOS.mm', u'Source/WebCore/page/mac/ChromeMac.mm', u'Source/WebCore/page/mac/PageMac.cpp', u'Source/WebCore/page/mac/SettingsMac.mm', u'Source/WebCore/page/mac/WebCoreFrameView.h', u'Source/WebKit/ios/ChangeLog', u'Source/WebKit/ios/WebCoreSupport/WebChromeClientIOS.mm', u'Source/WebKit/ios/WebView/WebPDFViewIOS.mm', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.h', u'Source/WebKit2/WebProcess/WebCoreSupport/ios/WebChromeClientIOS.mm', u'Source/WebKit2/WebProcess/WebPage/WebPage.cpp', '--commit-queue']" exit_code: 1
ERROR: Source/WebCore/page/Frame.cpp:175:  Wrong number of spaces before statement. (expected: 8)  [whitespace/indent] [4]
ERROR: Source/WebCore/page/Frame.cpp:176:  Wrong number of spaces before statement. (expected: 8)  [whitespace/indent] [4]
ERROR: Source/WebCore/page/Frame.cpp:177:  Wrong number of spaces before statement. (expected: 8)  [whitespace/indent] [4]
Total errors found: 3 in 45 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Darin Adler 2013-12-23 17:00:49 PST
Comment on attachment 219931 [details]
Patch

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

OK to merge all of this. A lot of it needs refinement longer term.

> Source/WTF/wtf/FeatureDefines.h:85
> +#if !defined(ENABLE_LETTERPRESS)
> +#define ENABLE_LETTERPRESS 1
> +#endif

This doesn’t seem to be in alphabetical order. Can we keep the alphabetical sorting?

> Source/WTF/wtf/FeatureDefines.h:145
> +#if !defined(ENABLE_IOS_TOUCH_EVENTS) && ENABLE(TOUCH_EVENTS)
> +#define ENABLE_IOS_TOUCH_EVENTS 1
> +#endif

This doesn’t seem to be in alphabetical order. Can we keep the alphabetical sorting?

> Source/WebCore/page/Chrome.cpp:518
> +#if !PLATFORM(IOS)
>      m_client.setCursor(cursor);
> +#else
> +    UNUSED_PARAM(cursor);
> +#endif

Seems like this should be a platform-has-no-cursor-ifdef, not an iOS-ifdef. Maybe ENABLE(CURSOR_SUPPORT) or ENABLE(CURSOR)? Seems there is quite a bit of code in this patch that would go under that.

> Source/WebCore/page/Chrome.cpp:527
> +#if !PLATFORM(IOS)
>      m_client.setCursorHiddenUntilMouseMoves(hiddenUntilMouseMoves);
> +#else
> +    UNUSED_PARAM(hiddenUntilMouseMoves);
> +#endif

Ditto.

> Source/WebCore/page/ChromeClient.h:46
> +#include "Console.h"
> +#include "GraphicsLayer.h"
> +#include <wtf/HashMap.h>

Surprising that we need these includes in the header.
Comment 4 EFL EWS Bot 2013-12-23 19:16:14 PST
Comment on attachment 219931 [details]
Patch

Attachment 219931 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/51898023
Comment 5 Daniel Bates 2013-12-27 10:43:25 PST
Comment on attachment 219931 [details]
Patch

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

>> Source/WTF/wtf/FeatureDefines.h:85
>> +#endif
> 
> This doesn’t seem to be in alphabetical order. Can we keep the alphabetical sorting?

Will fix.

>> Source/WTF/wtf/FeatureDefines.h:145
>> +#endif
> 
> This doesn’t seem to be in alphabetical order. Can we keep the alphabetical sorting?

Will fix.

>> Source/WebCore/page/Chrome.cpp:518
>> +#endif
> 
> Seems like this should be a platform-has-no-cursor-ifdef, not an iOS-ifdef. Maybe ENABLE(CURSOR_SUPPORT) or ENABLE(CURSOR)? Seems there is quite a bit of code in this patch that would go under that.

Will define feature ENABLE(CURSOR_SUPPORT), enabled by default on all ports except iOS, and update this patch to use it.

>> Source/WebCore/page/Chrome.cpp:527
>> +#endif
> 
> Ditto.

See above remark.

>> Source/WebCore/page/ChromeClient.h:46
>> +#include <wtf/HashMap.h>
> 
> Surprising that we need these includes in the header.

We don't. Will remove. However we need to include header PlatformLayer.h.
Comment 6 Daniel Bates 2013-12-27 12:37:51 PST
Committed r161106: <http://trac.webkit.org/changeset/161106>
Comment 7 Daniel Bates 2013-12-27 14:15:59 PST
Committed Windows build fixes in:

<http://trac.webkit.org/changeset/161107>
<http://trac.webkit.org/changeset/161108>