Bug 126180 – [iOS] Upstream WebCore/page changes

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Daniel Bates]

Upstream the iOS related changes to WebCore/page.

[Daniel Bates]

Created attachment 219931 [details]
Patch

[WebKit Commit Bot]

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.

[Darin Adler]

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.

[EFL EWS Bot]

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

[Daniel Bates]

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.

[Daniel Bates]

Committed r161106: <http://trac.webkit.org/changeset/161106>

[Daniel Bates]

Committed Windows build fixes in:

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