[Webkit-unassigned] [Bug 230735] Add a module map file for PrivateFrameworks/WebKitLegacy

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=230735

David Kilzer (:ddkilzer) <ddkilzer at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #442115|review?                     |review-
              Flags|                            |

--- Comment #22 from David Kilzer (:ddkilzer) <ddkilzer at webkit.org> ---
Comment on attachment 442115
  --> https://bugs.webkit.org/attachment.cgi?id=442115
Patch

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

r- for the following:
- Import <TargetConditions.h> instead of CoreGraphics.h or Foundation.h if that's all that's needed.
- Figure out if WebDownload.h is Public or Private header.  (Xcode says it's Private, which means it could import NSURLDownloadSPI.h.)

> Source/WebCore/platform/ios/WebItemProviderPasteboard.h:-32
> +#import <CoreGraphics/CoreGraphics.h>
> +
>  #if TARGET_OS_IPHONE
>  #import <WebCore/AbstractPasteboard.h>
>  #endif
>  
>  #if TARGET_OS_IOS
>  
> -struct CGSize;

Let's `#import <TargetConditionals.h>` instead of including the whole framework header, and add the `strict CGSize;` statement back.

> Source/WebCore/platform/ios/wak/WAKAppKitStubs.h:-32
> +#import <Foundation/Foundation.h>
> +
>  #if TARGET_OS_IPHONE
>  
>  #import <CoreGraphics/CoreGraphics.h>
> -#import <Foundation/Foundation.h>

To be explicit about why it's outside the `#if TARGET_OS_IPHONE`, I'd rather `#import <TargetConditionals.h>` here instead of moving the Foundation header.

> Source/WebCore/platform/ios/wak/WAKResponder.h:33
> +#import <Foundation/Foundation.h>
> +
>  #if TARGET_OS_IPHONE
>  
> -#import "WKTypes.h"
> -#import <Foundation/Foundation.h>
> +#import <WebCore/WKTypes.h>

Ditto.

> Source/WebCore/platform/ios/wak/WAKView.h:35
> +#import <Foundation/Foundation.h>
> +
>  #if TARGET_OS_IPHONE
>  
> -#import "WAKAppKitStubs.h"
> -#import "WAKResponder.h"
> -#import <Foundation/Foundation.h>
>  #import <CoreGraphics/CoreGraphics.h>
> +#import <WebCore/WAKAppKitStubs.h>
> +#import <WebCore/WAKResponder.h>

Ditto.

> Source/WebCore/platform/ios/wak/WAKWindow.h:35
> +#import <Foundation/Foundation.h>
> +
>  #if TARGET_OS_IPHONE
>  
> -#import "WAKAppKitStubs.h"
> -#import "WAKView.h"
> -#import "WKContentObservation.h"
>  #import <CoreGraphics/CoreGraphics.h>
> -#import <Foundation/Foundation.h>
> +#import <WebCore/WAKAppKitStubs.h>
> +#import <WebCore/WAKView.h>
> +#import <WebCore/WKContentObservation.h>

Ditto.

> Source/WebKitLegacy/ios/Misc/WebGeolocationCoreLocationProvider.h:28
> - at class CLLocationManager;
> - at class NSString;
> +#import <Foundation/Foundation.h>

Why is Foundation.h needed here?  (Don't necessarily need to change it—just curious.)

Oh, if it's to inherit from NSObject, then that's fine.  (No reply needed.)

> Source/WebKitLegacy/mac/DOM/WebDOMOperationsPrivate.h:29
> +#import <Foundation/Foundation.h>

If this is for TargetConditionals.h, let's import TargetConditions.h directly instead.

> Source/WebKitLegacy/mac/History/WebHistoryItemPrivate.h:29
> +#import <CoreGraphics/CoreGraphics.h>

If this is for TargetConditionals.h, let's import TargetConditions.h directly instead.

> Source/WebKitLegacy/mac/Misc/NSURLDownloadSPI.h:26
> +#import <Foundation/Foundation.h>

If this is for TargetConditionals.h, let's import TargetConditions.h directly instead.

> Source/WebKitLegacy/mac/Misc/WebCache.h:27
> +#import <CoreGraphics/CoreGraphics.h>
> +#import <Foundation/Foundation.h>

If CoreGraphics.h is just for TargetConditionals.h, lets import TargetConditions.h directly instead.

> Source/WebKitLegacy/mac/Misc/WebDownload.h:40
> -#if (defined TARGET_OS_MACCATALYST && TARGET_OS_MACCATALYST)
> +#import <Foundation/Foundation.h>
> +#import <WebKitLegacy/WebKitAvailability.h>
> +
> +#if defined(TARGET_OS_MACCATALYST) && TARGET_OS_MACCATALYST
>  #import <CFNetwork/CFNSURLConnection.h>
> -#elif !TARGET_OS_IPHONE || (defined USE_APPLE_INTERNAL_SDK && USE_APPLE_INTERNAL_SDK)
> +#elif !TARGET_OS_IPHONE || (defined(USE_APPLE_INTERNAL_SDK) && USE_APPLE_INTERNAL_SDK)
>  #import <Foundation/NSURLDownload.h>
>  #else
>  #import <WebKitLegacy/NSURLDownloadSPI.h>

I commented about this on the older patch, but are we _sure_ that WebDownload.h is a PUBLIC header?  The Xcode project says it's PRIVATE:

$ grep "WebDownload.h in Headers" Source/WebKitLegacy/WebKitLegacy.xcodeproj/project.pbxproj 
                939810770824BF01008DF038 /* WebDownload.h in Headers */ = {isa = PBXBuildFile; fileRef = 6578F5DE045F817400000128 /* WebDownload.h */; settings = {ATTRIBUTES = (Private, ); }; };
                                939810770824BF01008DF038 /* WebDownload.h in Headers */,

If it is really public, it should probably be fixed in the Xcode project, too.

> Source/WebKitLegacy/mac/Plugins/WebPlugin.h:29
> +#import <CoreGraphics/CoreGraphics.h>

If this is just for <TargetConditionals.h>, let's import that directly instead.

> Source/WebKitLegacy/mac/Storage/WebDatabaseManagerPrivate.h:29
> +#import <Foundation/Foundation.h>

If this is for TargetConditionals.h, let's import TargetConditions.h directly instead.

> Source/WebKitLegacy/mac/WebCoreSupport/WebCreateFragmentInternal.h:30
> +#import <Foundation/Foundation.h>

Could probably be replaced with:

@class NSAttributedString;

> Source/WebKitLegacy/mac/WebCoreSupport/WebSecurityOriginPrivate.h:29
> +#import <Foundation/Foundation.h>

If this is for TargetConditionals.h, let's import TargetConditions.h directly instead.

> Source/WebKitLegacy/mac/WebView/WebResourceLoadDelegatePrivate.h:-35
> +#import <Foundation/Foundation.h>
>  #import <TargetConditionals.h>
>  
>  @class WebView;
>  @class WebDataSource;
> - at class NSURLAuthenticationChallenge;
> - at class NSURLResponse;
> - at class NSURLRequest;

If this is for TargetConditionals.h, let's import TargetConditions.h directly instead.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20211022/8bb1277b/attachment-0001.htm>