Stub for WebSession API
Created attachment 220604 [details] patch
Created attachment 220605 [details] rebase + style fixes
Created attachment 220606 [details] ChangeLog
Attachment 220606 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/Shared/API/c/WKBase.h', u'Source/WebKit2/Shared/API/c/WKSharedAPICast.h', u'Source/WebKit2/Shared/APIObject.h', u'Source/WebKit2/UIProcess/API/C/WKSessionRef.cpp', u'Source/WebKit2/UIProcess/API/C/WKSessionRef.h', u'Source/WebKit2/UIProcess/API/Cocoa/WKSession.h', u'Source/WebKit2/UIProcess/API/Cocoa/WKSession.mm', u'Source/WebKit2/UIProcess/API/Cocoa/WKSessionInternal.h', u'Source/WebKit2/UIProcess/WebSession.cpp', u'Source/WebKit2/UIProcess/WebSession.h', u'Source/WebKit2/WebKit2.xcodeproj/project.pbxproj', '--commit-queue']" exit_code: 1 ERROR: Source/WebKit2/UIProcess/WebSession.h:27: WebSession_h is incorrect. #defined constants should use all uppercase names with words separated by underscores. [readability/naming/define/constants] [4] Total errors found: 1 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.
I don't understand this style error as it seems to follow the style guidelines for header guards. Am I missing something? (In reply to comment #4) > Attachment 220606 [details] did not pass style-queue: > > Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/Shared/API/c/WKBase.h', u'Source/WebKit2/Shared/API/c/WKSharedAPICast.h', u'Source/WebKit2/Shared/APIObject.h', u'Source/WebKit2/UIProcess/API/C/WKSessionRef.cpp', u'Source/WebKit2/UIProcess/API/C/WKSessionRef.h', u'Source/WebKit2/UIProcess/API/Cocoa/WKSession.h', u'Source/WebKit2/UIProcess/API/Cocoa/WKSession.mm', u'Source/WebKit2/UIProcess/API/Cocoa/WKSessionInternal.h', u'Source/WebKit2/UIProcess/WebSession.cpp', u'Source/WebKit2/UIProcess/WebSession.h', u'Source/WebKit2/WebKit2.xcodeproj/project.pbxproj', '--commit-queue']" exit_code: 1 > ERROR: Source/WebKit2/UIProcess/WebSession.h:27: WebSession_h is incorrect. #defined constants should use all uppercase names with words separated by underscores. [readability/naming/define/constants] [4] > Total errors found: 1 in 12 files > > > If any of these errors are false positives, please file a bug against check-webkit-style.
> I don't understand this style error as it seems to follow the style guidelines for header guards. Am I missing something? This is a newly introduced mistake in style checker, will roll out in bug 126645.
Comment on attachment 220606 [details] ChangeLog View in context: https://bugs.webkit.org/attachment.cgi?id=220606&action=review With those changes, r=me. > Source/WebKit2/UIProcess/API/Cocoa/WKSession.h:26 > +#include <WebKit2/WKSessionRef.h> This should not be included. It is in some of the objective-c headers as a temporary renaming mitigation. > Source/WebKit2/UIProcess/API/Cocoa/WKSession.h:28 > +#ifdef __OBJC__ This is also not needed. > Source/WebKit2/UIProcess/WebSession.h:37 > + bool getEphemeral(); This should be called either isEphemeral() or just ephemeral() (I prefer the isEphemeral()).
Created attachment 220642 [details] address comments
Comment on attachment 220642 [details] address comments Attachment 220642 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/5178054582206464
Created attachment 220644 [details] rebase
Created attachment 220645 [details] add reviewer
Comment on attachment 220645 [details] add reviewer Here are some high-level comments I though of yesterday: - WKSession is a very generic and non-descriptive name. I think it should be named to reflect what type of session it is. - I had to look up what ephemeral meant, and I suspect other developers that aren't fluent in english will have to do so too. Maybe we can come up with a better name? - I think the C++ object should be in the API namespace.
(In reply to comment #12) > (From update of attachment 220645 [details]) > Here are some high-level comments I though of yesterday: > > - WKSession is a very generic and non-descriptive name. I think it should be named to reflect what type of session it is. In the end, I think the WKSession should have all the session specific information: - Cookies - Caches - Local Storage - Databases - Etc. > - I had to look up what ephemeral meant, and I suspect other developers that aren't fluent in english will have to do so too. Maybe we can come up with a better name? - This is the name CFNetwork uses. I think it is quite good :(. > - I think the C++ object should be in the API namespace. Yeah, it should.
(In reply to comment #13) > (In reply to comment #12) > > - I think the C++ object should be in the API namespace. > Yeah, it should. So WebSession should be in namespace API instead of namespace WebKit? I'm not sure how this makes sense because API classes tend to be abstract concepts like Array, Number, Object, etc and the names all start with "API". Should I give WebSession the same treatment?
(In reply to comment #14) > (In reply to comment #13) > > (In reply to comment #12) > > > - I think the C++ object should be in the API namespace. > > Yeah, it should. > > So WebSession should be in namespace API instead of namespace WebKit? I'm not sure how this makes sense because API classes tend to be abstract concepts like Array, Number, Object, etc and the names all start with "API". We've started using the API namespaces for objects that are exposed as API. Right now that means most of the abstract objects but we're moving more of the Web* classes over to API*. > Should I give WebSession the same treatment? Yes.
Created attachment 220652 [details] move Session to API namespace
Created attachment 220653 [details] style
Comment on attachment 220653 [details] style Clearing flags on attachment: 220653 Committed r161542: <http://trac.webkit.org/changeset/161542>
All reviewed patches have been landed. Closing bug.