Bug 174192

Summary: [PAL] Move SessionID into PAL
Product: WebKit Reporter: Don Olmstead <don.olmstead>
Component: PlatformAssignee: Don Olmstead <don.olmstead>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, ap, beidson, buildbot, commit-queue, koivisto, mcatanzaro, rniwa, sam, webkit-bug-importer, yoshiaki.jitsukawa
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
WIP Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Don Olmstead
Reported 2017-07-05 20:42:34 PDT
WebCore::SessionID is required in multiple places within WebCore/platform but is currently at WebCore/page/SessionID.h. For example platform/network/NetworkStorageSession and WebCoreCrossThreadCopier. It seems like it should reside within pal/network.
Attachments
WIP Patch (322.15 KB, patch)
2017-08-14 18:56 PDT, Don Olmstead 		no flags
DetailsFormatted DiffDiff
Patch (380.69 KB, patch)
2017-08-15 15:40 PDT, Don Olmstead 		no flags
DetailsFormatted DiffDiff
Patch (381.22 KB, patch)
2017-08-15 16:04 PDT, Don Olmstead 		no flags
DetailsFormatted DiffDiff
Patch (375.72 KB, patch)
2017-08-16 16:07 PDT, Don Olmstead 		no flags
DetailsFormatted DiffDiff
Patch (375.81 KB, patch)
2017-08-16 16:42 PDT, Don Olmstead 		no flags
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Don Olmstead
Comment 1 2017-07-05 20:43:15 PDT
Alex does this work for you? There's quite a lot of places that need to be touched to make this happen.
Alex Christensen
Comment 2 2017-07-06 09:36:16 PDT
Hmmm. PAL will probably need to use it, and it will probably need to go into PAL, but a SessionID corresponds with a WebsiteDataStore and all forms of persistent storage. Cookies and HTTP cache are two parts of that, but because it isn't a concept that has only to do with the network, I wouldn't put it in the network directory.
Don Olmstead
Comment 3 2017-08-14 18:56:36 PDT
Created attachment 318098 [details] WIP Patch WinCairo builds checking GTK/WPE. No XCode changes yet.
Build Bot
Comment 4 2017-08-14 18:58:29 PDT
Attachment 318098 [details] did not pass style-queue: ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.h:67: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.h:68: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.h:71: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.h:73: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.h:74: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.h:76: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.h:77: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.h:79: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.h:80: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.h:87: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.h:136: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/Storage/StorageProcessProxy.cpp:86: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/Storage/StorageProcessProxy.cpp:96: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.cpp:110: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.cpp:137: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.cpp:143: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.cpp:149: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.cpp:155: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.cpp:161: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.cpp:167: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.cpp:199: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.cpp:245: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.cpp:264: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/API/C/WKCookieManager.cpp:65: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/UIProcess/API/C/WKCookieManager.cpp:65: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/WebKit/UIProcess/API/C/WKCookieManager.cpp:70: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/UIProcess/API/C/WKCookieManager.cpp:70: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkProcess.cpp:328: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp:125: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp:140: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/loader/cache/MemoryCache.cpp:275: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm:203: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm:203: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/WebKit/UIProcess/API/glib/WebKitCookieManager.cpp:183: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/UIProcess/API/glib/WebKitCookieManager.cpp:183: Missing space inside { }. [whitespace/braces] [5] Total errors found: 35 in 194 files If any of these errors are false positives, please file a bug against check-webkit-style.
Don Olmstead
Comment 5 2017-08-15 15:40:31 PDT
Created attachment 318187 [details] Patch
Build Bot
Comment 6 2017-08-15 15:43:05 PDT
Attachment 318187 [details] did not pass style-queue: ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.h:67: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.h:68: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.h:71: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.h:73: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.h:74: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.h:76: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.h:77: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.h:79: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.h:80: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.h:87: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.h:136: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/Storage/StorageProcessProxy.cpp:86: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/Storage/StorageProcessProxy.cpp:96: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.cpp:110: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.cpp:137: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.cpp:143: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.cpp:149: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.cpp:155: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.cpp:161: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.cpp:167: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.cpp:199: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.cpp:245: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.cpp:264: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/API/C/WKCookieManager.cpp:65: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/UIProcess/API/C/WKCookieManager.cpp:65: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/WebKit/UIProcess/API/C/WKCookieManager.cpp:70: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/UIProcess/API/C/WKCookieManager.cpp:70: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkProcess.cpp:328: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp:125: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp:140: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/loader/cache/MemoryCache.cpp:275: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm:203: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm:203: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/WebKit/UIProcess/API/glib/WebKitCookieManager.cpp:183: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/UIProcess/API/glib/WebKitCookieManager.cpp:183: Missing space inside { }. [whitespace/braces] [5] Total errors found: 35 in 199 files If any of these errors are false positives, please file a bug against check-webkit-style.
Don Olmstead
Comment 7 2017-08-15 16:04:54 PDT
Created attachment 318190 [details] Patch
Build Bot
Comment 8 2017-08-15 16:06:30 PDT
Attachment 318190 [details] did not pass style-queue: ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.h:67: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.h:68: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.h:71: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.h:73: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.h:74: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.h:76: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.h:77: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.h:79: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.h:80: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.h:87: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.h:136: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/Storage/StorageProcessProxy.cpp:86: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/Storage/StorageProcessProxy.cpp:96: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.cpp:110: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.cpp:137: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.cpp:143: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.cpp:149: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.cpp:155: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.cpp:161: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.cpp:167: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.cpp:199: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.cpp:245: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.cpp:264: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/API/C/WKCookieManager.cpp:65: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/UIProcess/API/C/WKCookieManager.cpp:65: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/WebKit/UIProcess/API/C/WKCookieManager.cpp:70: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/UIProcess/API/C/WKCookieManager.cpp:70: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkProcess.cpp:328: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp:125: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp:140: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/loader/cache/MemoryCache.cpp:275: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm:203: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm:203: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/WebKit/UIProcess/API/glib/WebKitCookieManager.cpp:183: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/UIProcess/API/glib/WebKitCookieManager.cpp:183: Missing space inside { }. [whitespace/braces] [5] Total errors found: 35 in 199 files If any of these errors are false positives, please file a bug against check-webkit-style.
Don Olmstead
Comment 9 2017-08-15 16:31:35 PDT
Comment on attachment 318190 [details] Patch This moves SessionID into PAL completely fixing layering violations present in WebCore/platform. Mostly this was just a find/replace operation. Only notable thing is teaching the derived sources script in WebKit about the SessionID's location. Believe the style errors are just from the style checker not understanding the syntax usage.
Don Olmstead
Comment 10 2017-08-16 16:07:19 PDT
Created attachment 318300 [details] Patch Rebased
Don Olmstead
Comment 11 2017-08-16 16:42:42 PDT
Created attachment 318305 [details] Patch Rebased again
Build Bot
Comment 12 2017-08-16 16:45:00 PDT
Attachment 318305 [details] did not pass style-queue: ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.h:67: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.h:68: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.h:71: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.h:73: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.h:74: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.h:76: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.h:77: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.h:79: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.h:80: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.h:87: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.h:136: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/Storage/StorageProcessProxy.cpp:86: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/Storage/StorageProcessProxy.cpp:96: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.cpp:110: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.cpp:137: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.cpp:143: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.cpp:149: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.cpp:155: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.cpp:161: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.cpp:167: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.cpp:199: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.cpp:245: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/WebCookieManagerProxy.cpp:264: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/API/C/WKCookieManager.cpp:65: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/UIProcess/API/C/WKCookieManager.cpp:65: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/WebKit/UIProcess/API/C/WKCookieManager.cpp:70: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/UIProcess/API/C/WKCookieManager.cpp:70: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/WebKit/NetworkProcess/NetworkProcess.cpp:328: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp:125: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp:140: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/loader/cache/MemoryCache.cpp:275: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm:203: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm:203: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/WebKit/UIProcess/API/glib/WebKitCookieManager.cpp:183: Missing space before { [whitespace/braces] [5] ERROR: Source/WebKit/UIProcess/API/glib/WebKitCookieManager.cpp:183: Missing space inside { }. [whitespace/braces] [5] Total errors found: 35 in 198 files If any of these errors are false positives, please file a bug against check-webkit-style.
Antti Koivisto
Comment 13 2017-08-17 07:46:17 PDT
Comment on attachment 318305 [details] Patch rs=me
WebKit Commit Bot
Comment 14 2017-08-17 08:17:09 PDT
Comment on attachment 318305 [details] Patch Clearing flags on attachment: 318305 Committed r220857: <http://trac.webkit.org/changeset/220857>
WebKit Commit Bot
Comment 15 2017-08-17 08:17:11 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 16 2017-08-17 08:18:13 PDT
<rdar://problem/33940723>
Brady Eidson
Comment 17 2017-08-17 09:57:35 PDT
Why did SessionID.h move to <pal/identifier/.>? Why not just <pal/.>? I am strongly against segregating the paths within the pal exports. We don't do that in WebCore or WTF (with the exception of WTF/text/ which is a special case where there's a particular reason to do so) Could you either fix this in a followup or roll it out until you have time to revisit?
Don Olmstead
Comment 18 2017-08-17 10:33:43 PDT
(In reply to Brady Eidson from comment #17) > Why did SessionID.h move to <pal/identifier/.>? > > Why not just <pal/.>? > > I am strongly against segregating the paths within the pal exports. We don't > do that in WebCore or WTF (with the exception of WTF/text/ which is a > special case where there's a particular reason to do so) > > Could you either fix this in a followup or roll it out until you have time > to revisit? Is the ask to just move the file into the root? or to do something like WebCore where the includes look like <WebCore/*.h>? for WebKit(Legacy)? I'm fine with doing a followup with whatever the consensus is. I have been trying to organize things when moving them to PAL as a lot of WebCore/platform is a bit of a dumping ground.
Brady Eidson
Comment 19 2017-08-17 10:40:21 PDT
(In reply to Don Olmstead from comment #18) > (In reply to Brady Eidson from comment #17) > > Why did SessionID.h move to <pal/identifier/.>? > > > > Why not just <pal/.>? > > > > I am strongly against segregating the paths within the pal exports. We don't > > do that in WebCore or WTF (with the exception of WTF/text/ which is a > > special case where there's a particular reason to do so) > > > > Could you either fix this in a followup or roll it out until you have time > > to revisit? > > Is the ask to just move the file into the root? or to do something like > WebCore where the includes look like <WebCore/*.h>? for WebKit(Legacy)? > Move to root. I see now that we're segregating by path in all the <pal> headers. <pal/spi>, <pal/system>, <pal/crypto>, etc. This is silly. If this was a decision made by consensus anywhere, I missed the conversation. I would've participated as a strong "nay" > I have been trying to organize things when moving them to PAL as a lot of > WebCore/platform is a bit of a dumping ground. WebCore itself is a vast dumping ground. But all of the headers still export to a root path. WTF is a dumping ground, but most of the headers still export to a root path. WebKit is a dumping ground, but the headers export to a root path. System headers are a dumping ground, but they mostly export to a root path. It's unclear to me what the identified value is for engineers in the WebKit project to have to learn longer subdirectories for commonly used headers. e.g. When I used to include SessionID.h in WebKit, I knew it was <WebCore/SessionID.h> It would take a day or two to adapt to <pal/SessionID.h> It would take me a *LONG* time to remember <pal/identifier/SessionID.h>, and I see no value in anybody having to do so.
Don Olmstead
Comment 20 2017-08-17 14:25:38 PDT
(In reply to Brady Eidson from comment #19) > (In reply to Don Olmstead from comment #18) > > (In reply to Brady Eidson from comment #17) > > > Why did SessionID.h move to <pal/identifier/.>? > > > > > > Why not just <pal/.>? > > > > > > I am strongly against segregating the paths within the pal exports. We don't > > > do that in WebCore or WTF (with the exception of WTF/text/ which is a > > > special case where there's a particular reason to do so) > > > > > > Could you either fix this in a followup or roll it out until you have time > > > to revisit? > > > > Is the ask to just move the file into the root? or to do something like > > WebCore where the includes look like <WebCore/*.h>? for WebKit(Legacy)? > > > > Move to root. > > I see now that we're segregating by path in all the <pal> headers. > <pal/spi>, <pal/system>, <pal/crypto>, etc. > > This is silly. If this was a decision made by consensus anywhere, I missed > the conversation. I would've participated as a strong "nay" > > > I have been trying to organize things when moving them to PAL as a lot of > > WebCore/platform is a bit of a dumping ground. > > WebCore itself is a vast dumping ground. But all of the headers still export > to a root path. > > WTF is a dumping ground, but most of the headers still export to a root path. > > WebKit is a dumping ground, but the headers export to a root path. > > System headers are a dumping ground, but they mostly export to a root path. > > It's unclear to me what the identified value is for engineers in the WebKit > project to have to learn longer subdirectories for commonly used headers. > > e.g. When I used to include SessionID.h in WebKit, I knew it was > <WebCore/SessionID.h> > It would take a day or two to adapt to <pal/SessionID.h> > It would take me a *LONG* time to remember <pal/identifier/SessionID.h>, and > I see no value in anybody having to do so. A patch is at https://bugs.webkit.org/show_bug.cgi?id=175684 for your review that moves SessionID to the root of the pal directory. I see your issue with how things are currently looking and since PAL is still being fleshed out I'd like to address your feedback earlier rather than later. Is this something you want addressed on the mailing list so others can chime in or should I just put together a bug that will create forwarding headers which behave in the same way as the <WebCore/*.h> includes and start implementing?
Michael Catanzaro
Comment 21 2017-08-17 17:14:16 PDT
I would just go ahead and work on forwarding headers. There's no reason for PAL to operate differently than other WebKit subprojects.
Brady Eidson
Comment 22 2017-08-17 20:02:49 PDT
(In reply to Michael Catanzaro from comment #21) > I would just go ahead and work on forwarding headers. There's no reason for > PAL to operate differently than other WebKit subprojects. ^^^ 👍👍👍
Don Olmstead
Comment 23 2017-08-17 20:16:33 PDT
(In reply to Brady Eidson from comment #22) > (In reply to Michael Catanzaro from comment #21) > > I would just go ahead and work on forwarding headers. There's no reason for > > PAL to operate differently than other WebKit subprojects. > > ^^^ > > 👍👍👍 Will work on this next https://bugs.webkit.org/show_bug.cgi?id=175705 to get things resolved.
Note You need to log in before you can comment on or make changes to this bug.