Bug 124803 - Merge WebKit2 iOS specific code
Summary: Merge WebKit2 iOS specific code
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: All Unspecified
: P2 Normal
Assignee: Enrica Casucci
URL:
Keywords:
Depends on: 124806
Blocks:
  Show dependency treegraph
 
Reported: 2013-11-22 16:33 PST by Enrica Casucci
Modified: 2013-11-22 21:52 PST (History)
1 user (show)

See Also:


Attachments
Patch1 (18.49 KB, patch)
2013-11-22 16:44 PST, Enrica Casucci
thorton: review+
Details | Formatted Diff | Diff
Patch2 (231.21 KB, patch)
2013-11-22 16:45 PST, Enrica Casucci
thorton: review+
Details | Formatted Diff | Diff
Patch3 (152.02 KB, patch)
2013-11-22 16:46 PST, Enrica Casucci
thorton: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Enrica Casucci 2013-11-22 16:33:05 PST
This is to track the work to upstream the iOS code to OpenSource.
Comment 1 Enrica Casucci 2013-11-22 16:44:56 PST
Created attachment 217728 [details]
Patch1

This is part 1.
Comment 2 Enrica Casucci 2013-11-22 16:45:30 PST
Created attachment 217729 [details]
Patch2

This is part 2.
Comment 3 Enrica Casucci 2013-11-22 16:46:20 PST
Created attachment 217731 [details]
Patch3

This is part 3.
Comment 4 Tim Horton 2013-11-22 17:02:30 PST
Comment on attachment 217728 [details]
Patch1

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

> NetworkProcess/EntryPoint/mac/XPCService/NetworkServiceEntryPoint.mm:39
> +#if ENABLE(NETWORK_PROCESS)

should this whole file be #if ENABLE(NETWORK_PROCESS)'d?
Comment 5 Tim Horton 2013-11-22 17:45:20 PST
Comment on attachment 217729 [details]
Patch2

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

r+ from me and andersca with comments that don't necessarily need to all be addressed in this patch but should be addressed eventually.

> ChangeLog:8
> +        * UIProcess/API/C/WKNativeEvent.h:

Please remove this long list and just note that you did so.

> UIProcess/API/C/WKPage.cpp:894
>  // -- DEPRECATED --

WKPageSetInvalidMessageFunction shouldn't be in this NPAPI block.

> UIProcess/API/ios/PageClientImplIOS.h:36
> +    class PageClientImpl : public PageClient {

None of this should be indented.

> UIProcess/API/ios/UIWKRemoteView.mm:1
> +/*

We should get rid of this file, we won't need it.

> UIProcess/API/ios/WKContentView.h:50
> +NS_CLASS_AVAILABLE_IOS(8_0)

This should use WK_API_CLASS, not this.

> UIProcess/API/ios/WKContentView.mm:267
> +        [_delegate contentView:self
> +didChangeViewportArgumentsSize:CGSizeMake(arguments.width, arguments.height)
> +                  initialScale:arguments.zoom
> +                  minimumScale:arguments.minZoom
> +                  maximumScale:arguments.maxZoom
> +             allowsUserScaling:arguments.userZoom];

This should be on a single line

> UIProcess/API/ios/WKInteractionView.mm:71
> +@property (copy,nonatomic) NSArray *selectionRects;

space after the comma

> UIProcess/API/ios/WKInteractionView.mm:182
> +- (void)setScrollView:(UIWebScrollView*)scrollView

space before the star

> UIProcess/API/ios/WKInteractionView.mm:350
> +        case UIGestureRecognizerStateBegan:

unindent cases

> UIProcess/API/ios/WKInteractionView.mm:493
> +    if (action == @selector(_promptForReplace:))
> +        // FIXME: need to implement
> +        return NO;
> +
> +    if (action == @selector(select:))
> +        // Disable select in password fields so that you can't see word boundaries.
> +        return !_page->editorState().isInPasswordField && [self hasContent] && !_page->editorState().selectionIsNone && !_page->editorState().selectionIsRange;

Curly braces, please, even though they're not required.

> UIProcess/API/ios/WKInteractionView.mm:761
> +    WKInteractionView* view = static_cast<WKInteractionView*>(context);

stars are on the wrong side

> UIProcess/API/ios/WKInteractionView.mm:767
> +static void selectionChangedWithTouch(const WebCore::IntPoint& point, uint32_t touch, WKErrorRef error, void* context)

We usually 'using namespace WebCore' in implementation files to avoid all of these WebCore::s

> UIProcess/API/ios/WKInteractionView.mm:773
> +    WKInteractionView* view = static_cast<WKInteractionView*>(context);

stars on the wrong side

> UIProcess/API/ios/WKInteractionView.mm:1331
> +    return (id)[UIFont fontWithFamilyName:_autocorrectionData.fontName traits:(UIFontTrait)_autocorrectionData.fontTraits size:scaledSize];

Why is there an (id) here? This method returns a UIFont.

> UIProcess/API/ios/WKView.mm:118
> +- (void)contentViewdidCommitLoadForMainFrame:(WKContentView *)contentView

I thought the capitalization of this method was fixed recently?

> UIProcess/API/mac/WKBrowsingContextController.mm:85
> +    id<WKBrowsingContextLoadDelegateInternal> _loadDelegateInternal;

space after id, says andersca

> UIProcess/API/mac/WKBrowsingContextController.mm:86
> +#endif //PLATFORM(IOS)

space after //

> UIProcess/API/mac/WKBrowsingContextController.mm:506
> +#endif //PLATFORM(IOS)

space after //

> UIProcess/API/mac/WKProcessGroup.mm:238
> +- (WKGeolocationProviderIOS *) _geolocationProvider

no space between ) and _

> UIProcess/API/mac/WKProcessGroupPrivate.h:29
> +#if PLATFORM(IOS)

This can't be in API/SPI headers.

> UIProcess/API/mac/WKProcessGroupPrivate.h:36
> +#if PLATFORM(IOS)

Nor this.

> UIProcess/PageClient.h:48
> +OBJC_CLASS UIWKView;

I think we don't need this.

> UIProcess/WebPageProxy.h:1191
> +    HashMap<uint64_t, RefPtr<TouchesCallback> > m_touchesCallbacks;
> +    HashMap<uint64_t, RefPtr<AutocorrectionDataCallback> > m_autocorrectionCallbacks;

no space between >>

> UIProcess/ios/TiledCoreAnimationDrawingAreaProxyIOS.h:1
> +/*

We should get rid of TCADA*IOS

> UIProcess/ios/WebPageProxyIOS.mm:50
> +    // Just return the iOS 5.1 user agent for now.
> +    return "Mozilla/5.0 (iPad; CPU OS 5_1 like Mac OS X) AppleWebKit/534.46 (KHTML, like Gecko) Version/5.1 Mobile/9B176 Safari/7534.48.3";

We should fix this.

> UIProcess/mac/WebContextMac.mm:38
> +#if !PLATFORM(IOS)
>  #import <QuartzCore/CARemoteLayerServer.h>
> +#endif

This should be in its own block. Also we really don't need it but we'll clean it up later.
Comment 6 Tim Horton 2013-11-22 17:56:37 PST
Comment on attachment 217731 [details]
Patch3

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

r+ from andersca and I with minor comments

> ChangeLog:8
> +        * WebProcess/EntryPoint/mac/LegacyProcess/WebContentProcessMain.mm:

elide list please

> WebProcess/WebCoreSupport/WebEditorClient.h:162
> +    virtual NSArray* supportedPasteboardTypesForCurrentSelection() OVERRIDE;
> +    virtual NSArray* readDataFromPasteboard(NSString* type, int index) OVERRIDE;

stars on the wrong side
Comment 7 Enrica Casucci 2013-11-22 19:27:41 PST
Committed revision 159724.
Comment 8 Tim Horton 2013-11-22 19:56:26 PST
Small build fix in http://trac.webkit.org/changeset/159725