Bug 175057 – [PAL] Move spi/cf directory into PAL

• Component: Platform
• Status: RESOLVED FIXED

[Yoshiaki Jitsukawa]

[PAL] Move spi/cf directory into PAL

[Yoshiaki Jitsukawa]

Moving all the SPI headers at once tends to cause merge conflict so I'm trying one directory by one.

[Yoshiaki Jitsukawa]

Created attachment 316910 [details]
Patch

[Build Bot]

Attachment 316910 [details] did not pass style-queue:


ERROR: Source/WebCore/PAL/pal/spi/cf/CFNetworkConnectionCacheSPI.h:26:  Use #pragma once instead of #ifndef for header guard.  [build/header_guard] [5]
ERROR: Source/WebCore/PAL/pal/spi/cf/CFNetworkConnectionCacheSPI.h:42:  _CFNetworkHTTPConnectionCacheGetLimit is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/PAL/pal/spi/cf/CFNetworkConnectionCacheSPI.h:43:  _CFNetworkHTTPConnectionCacheSetLimit is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/PAL/pal/spi/cf/CFLocaleSPI.h:26:  Use #pragma once instead of #ifndef for header guard.  [build/header_guard] [5]
ERROR: Source/WebCore/PAL/pal/spi/cf/CoreMediaSPI.h:26:  Use #pragma once instead of #ifndef for header guard.  [build/header_guard] [5]
Total errors found: 5 in 72 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Yoshiaki Jitsukawa]

Do we need to fix style of SPI headers?

[Yoshiaki Jitsukawa]

Created attachment 316917 [details]
Patch

I forgot to fix sources under the "Tools" directory...

[Build Bot]

Attachment 316917 [details] did not pass style-queue:


ERROR: Source/WebCore/PAL/pal/spi/cf/CFNetworkConnectionCacheSPI.h:26:  Use #pragma once instead of #ifndef for header guard.  [build/header_guard] [5]
ERROR: Source/WebCore/PAL/pal/spi/cf/CFNetworkConnectionCacheSPI.h:42:  _CFNetworkHTTPConnectionCacheGetLimit is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/PAL/pal/spi/cf/CFNetworkConnectionCacheSPI.h:43:  _CFNetworkHTTPConnectionCacheSetLimit is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/PAL/pal/spi/cf/CFLocaleSPI.h:26:  Use #pragma once instead of #ifndef for header guard.  [build/header_guard] [5]
ERROR: Source/WebCore/PAL/pal/spi/cf/CoreMediaSPI.h:26:  Use #pragma once instead of #ifndef for header guard.  [build/header_guard] [5]
Total errors found: 5 in 74 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Myles C. Maxfield]

Looks good once you fix the style check.

[Don Olmstead]

Just the pragma once correct?

[Yoshiaki Jitsukawa]

Created attachment 316942 [details]
Patch

Fixed the pragma once. I'm not sure whether the function names should be fixed or not...

[Build Bot]

Attachment 316942 [details] did not pass style-queue:


ERROR: Source/WebCore/PAL/pal/spi/cf/CFNetworkConnectionCacheSPI.h:41:  _CFNetworkHTTPConnectionCacheGetLimit is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/PAL/pal/spi/cf/CFNetworkConnectionCacheSPI.h:42:  _CFNetworkHTTPConnectionCacheSetLimit is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 2 in 74 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Yoshiaki Jitsukawa]

Created attachment 316948 [details]
Patch

Add a path-specific(WebCore/PAL/pal/spi) style rules to ignore underscores in identifier names.

[Build Bot]

Comment on attachment 316948 [details]
Patch

Attachment 316948 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/4239366

New failing tests:
fast/dom/HTMLTemplateElement/ownerDocumentXHTML.xhtml

[Build Bot]

Created attachment 316955 [details]
Archive of layout-test-results from ews113 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Yoshiaki Jitsukawa]

Created attachment 316990 [details]
Patch

[Yoshiaki Jitsukawa]

(In reply to Myles C. Maxfield from comment #7)
> Looks good once you fix the style check.

I've fixed the style check
 - Fix the include guards
 - Add a path-specific style rule for WebCore/PAL/pal/spi to ignore [readability/naming/underscores]

Would you review my patch?

The test failure on mac-debug EWS seems to be a false positive.

[Myles C. Maxfield]

Comment on attachment 316990 [details]
Patch

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

It looks like there are a lot of unrelated changes in the Xcode project files. Are these intentional?

Also, because this patch touches the WebKit sub-project, you’ll need an OWNER to review those pieces.

Other than that, this looks good.

> Source/WebCore/WebCore.xcodeproj/project.pbxproj:-21137
> -				CDA29A2E1CBF73FC00901CCF /* PlaybackSessionInterfaceAVKit.h */,

Can you make sure you run the sort-Xcode script in Tools/Scripts on this project?

[Yoshiaki Jitsukawa]

(In reply to Myles C. Maxfield from comment #16)
> Comment on attachment 316990 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=316990&action=review
> 
> It looks like there are a lot of unrelated changes in the Xcode project
> files. Are these intentional?
> 
> Also, because this patch touches the WebKit sub-project, you’ll need an
> OWNER to review those pieces.
> 
> Other than that, this looks good.
> 
> > Source/WebCore/WebCore.xcodeproj/project.pbxproj:-21137
> > -				CDA29A2E1CBF73FC00901CCF /* PlaybackSessionInterfaceAVKit.h */,
> 
> Can you make sure you run the sort-Xcode script in Tools/Scripts on this
> project?

Thank you. I CC Alex from the Owners list. 

I ran the sort script and unfortunately those unrelated diff were made by it.

[Myles C. Maxfield]

(In reply to Yoshiaki Jitsukawa from comment #17)
> (In reply to Myles C. Maxfield from comment #16)
> > Comment on attachment 316990 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=316990&action=review
> > 
> > It looks like there are a lot of unrelated changes in the Xcode project
> > files. Are these intentional?
> > 
> > Also, because this patch touches the WebKit sub-project, you’ll need an
> > OWNER to review those pieces.
> > 
> > Other than that, this looks good.
> > 
> > > Source/WebCore/WebCore.xcodeproj/project.pbxproj:-21137
> > > -				CDA29A2E1CBF73FC00901CCF /* PlaybackSessionInterfaceAVKit.h */,
> > 
> > Can you make sure you run the sort-Xcode script in Tools/Scripts on this
> > project?
> 
> Thank you. I CC Alex from the Owners list. 
> 
> I ran the sort script and unfortunately those unrelated diff were made by it.

Can we sort the file in a preliminary separate patch?

[Yoshiaki Jitsukawa]

(In reply to Myles C. Maxfield from comment #18)
> Can we sort the file in a preliminary separate patch?

Sure. Let me try.

[Yoshiaki Jitsukawa]

(In reply to Yoshiaki Jitsukawa from comment #19)
> (In reply to Myles C. Maxfield from comment #18)
> > Can we sort the file in a preliminary separate patch?
> 
> Sure. Let me try.

I've filed these bugs and sent patches.
Bug 175121 - [WebCore] Sort Xcode project files
Bug 175122 - [WebKit] Sort Xcode project file (just in case)

[Yoshiaki Jitsukawa]

Created attachment 317160 [details]
Patch

[Don Olmstead]

Comment on attachment 317160 [details]
Patch

Not sure whats up with ios-sim but it seems like its flaky in other patches we've submitted today.

[WebKit Commit Bot]

Comment on attachment 317160 [details]
Patch

Clearing flags on attachment: 317160

Committed r220243: <http://trac.webkit.org/changeset/220243>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/33713217>