Description
mitz
2014-07-26 23:29:18 PDT
Created attachment 235587 [details]
I. Introduce CredentialBase and make Credential derive from it
Attachment 235587 [details] did not pass style-queue:
ERROR: Source/WebCore/platform/network/CredentialBase.h:89: The parameter name "a" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/WebCore/platform/network/CredentialBase.cpp:43: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
ERROR: Source/WebCore/platform/network/CredentialBase.cpp:157: Multi line control clauses should use braces. [whitespace/braces] [4]
Total errors found: 3 in 9 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 235587 [details] I. Introduce CredentialBase and make Credential derive from it View in context: https://bugs.webkit.org/attachment.cgi?id=235587&action=review r=me -- the comments I made are sort of “side notes”; not all necessarily relevant to this file refactoring > Source/WebCore/platform/network/CredentialBase.cpp:26 > #include "config.h" > +#include "CredentialBase.h" Should add a blank line before the first include. > Source/WebCore/platform/network/CredentialBase.cpp:36 > : m_user("") > , m_password("") Should use the more efficient emptyString() some day if not now. > Source/WebCore/platform/network/CredentialBase.cpp:48 > : m_user(user.length() ? user : "") > , m_password(password.length() ? password : "") Should use the more efficient emptyString() some day if not now. > Source/WebCore/platform/network/CredentialBase.h:25 > + */ > +#ifndef Credential_h Should add a blank line before the ifndef. > Source/WebCore/platform/network/CredentialBase.h:30 > +#define CERTIFICATE_CREDENTIALS_SUPPORTED (PLATFORM(COCOA)) The name of this conditional is pretty strange. The feature in question involves interface that is done with platform-specific types, but yet this has a name that sounds like an abstract concept that would not be CoreFoundation-specific. > Source/WebCore/platform/network/CredentialBase.h:25 > */ > -#ifndef Credential_h > -#define Credential_h > +#ifndef CredentialBase_h Should add a blank line before the #ifndef. Committed attachment 235587 [details] as <http://trac.webkit.org/r171722>. Created attachment 235664 [details]
II. Move the Cocoa-specific parts of CredentialBase into a Cocoa-specific Credential class
Attachment 235664 [details] did not pass style-queue:
ERROR: Source/WebCore/platform/network/CredentialBase.h:51: The parameter name "a" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/WebCore/platform/network/cocoa/CredentialCocoa.h:63: The parameter name "identity" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/WebCore/platform/network/cocoa/CredentialCocoa.h:63: The parameter name "persistence" adds no information, so it should be removed. [readability/parameter_name] [5]
Total errors found: 3 in 8 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 235665 [details]
II. Move the Cocoa-specific parts of CredentialBase into a Cocoa-specific Credential class
Created attachment 235670 [details]
II. Move the Cocoa-specific parts of CredentialBase into a Cocoa-specific Credential class
Created attachment 235672 [details]
II. Move the Cocoa-specific parts of CredentialBase into a Cocoa-specific Credential class
Committed attachment 235672 [details] as <http://trac.webkit.org/r171747>. Created attachment 235706 [details]
WIP for EWS
Attachment 235706 [details] did not pass style-queue:
ERROR: Source/WebCore/platform/network/cocoa/CredentialCocoa.mm:89: Missing spaces around : [whitespace/init] [4]
ERROR: Source/WebCore/platform/network/cf/AuthenticationCF.cpp:58: Comma should be at the beginning of the line in a member initialization list. [whitespace/init] [4]
ERROR: Source/WebCore/platform/network/cf/AuthenticationCF.cpp:58: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
ERROR: Source/WebCore/platform/network/mac/AuthenticationMac.mm:156: Comma should be at the beginning of the line in a member initialization list. [whitespace/init] [4]
ERROR: Source/WebCore/platform/network/mac/AuthenticationMac.mm:156: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
ERROR: Source/WebCore/platform/network/mac/AuthenticationMac.mm:202: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
ERROR: Source/WebKit2/Shared/mac/WebCoreArgumentCodersMac.mm:340: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/WebKit2/Shared/mac/WebCoreArgumentCodersMac.mm:340: Extra space before ( in function call [whitespace/parens] [4]
Total errors found: 8 in 22 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 235716 [details]
III. Make Credential hold onto an NSURLCredential when needed
Attachment 235716 [details] did not pass style-queue:
ERROR: Source/WebCore/platform/network/cocoa/CredentialCocoa.mm:91: Missing spaces around : [whitespace/init] [4]
ERROR: Source/WebCore/platform/network/cf/AuthenticationCF.cpp:58: Comma should be at the beginning of the line in a member initialization list. [whitespace/init] [4]
ERROR: Source/WebCore/platform/network/cf/AuthenticationCF.cpp:58: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
ERROR: Source/WebCore/platform/network/mac/AuthenticationMac.mm:156: Comma should be at the beginning of the line in a member initialization list. [whitespace/init] [4]
ERROR: Source/WebCore/platform/network/mac/AuthenticationMac.mm:156: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
ERROR: Source/WebCore/platform/network/mac/AuthenticationMac.mm:202: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
ERROR: Source/WebKit2/Shared/mac/WebCoreArgumentCodersMac.mm:340: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/WebKit2/Shared/mac/WebCoreArgumentCodersMac.mm:340: Extra space before ( in function call [whitespace/parens] [4]
Total errors found: 8 in 23 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Committed attachment 235716 [details] as <http://trac.webkit.org/r171801>. Created attachment 235788 [details]
IV. Allow the delegate to respond to server trust challenges
Oops! I used a script that obsoleted all other parts and removed their review info. But parts I-III are in. Part IV is what remains. (In reply to comment #15) > Committed attachment 235716 [details] as <http://trac.webkit.org/r171801>. Commit queue is stucked after this change with the following error: /Volumes/Data/EWS/WebKit/Source/WebCore/platform/network/cocoa/CredentialCocoa.mm:55:13: error: enumeration value 'NSURLCredentialPersistenceSynchronizable' not handled in switch [-Werror,-Wswitch] switch (persistence) { ^ 1 error generated. How is it possible if there isn't build failure on buildbots, but only the EWS. Do they have different configuration? Why? (In reply to comment #18) > (In reply to comment #15) > > Committed attachment 235716 [details] [details] as <http://trac.webkit.org/r171801>. > > Commit queue is stucked after this change with the following error: > > /Volumes/Data/EWS/WebKit/Source/WebCore/platform/network/cocoa/CredentialCocoa.mm:55:13: error: enumeration value 'NSURLCredentialPersistenceSynchronizable' not handled in switch [-Werror,-Wswitch] > switch (persistence) { > ^ > 1 error generated. > > > How is it possible if there isn't build failure on buildbots, > but only the EWS. Do they have different configuration? Why? This error occurs when using the OS X 10.9 SDK to build for OS X 10.8. (In reply to comment #19) > This error occurs when using the OS X 10.9 SDK to build for OS X 10.8. OK, and how can we fix the commit queue? (In reply to comment #20) > (In reply to comment #19) > > This error occurs when using the OS X 10.9 SDK to build for OS X 10.8. > > OK, and how can we fix the commit queue? I am hoping <http://trac.webkit.org/r171854> fixes this particular build configuration. (In reply to comment #21) > (In reply to comment #20) > > (In reply to comment #19) > > > This error occurs when using the OS X 10.9 SDK to build for OS X 10.8. > > > > OK, and how can we fix the commit queue? > > I am hoping <http://trac.webkit.org/r171854> fixes this particular build configuration. I had to revert that change, since it broke building with the OS X 10.8 SDK. I will try something else in a moment. (In reply to comment #22) > (In reply to comment #21) > > (In reply to comment #20) > > > (In reply to comment #19) > > > > This error occurs when using the OS X 10.9 SDK to build for OS X 10.8. > > > > > > OK, and how can we fix the commit queue? > > > > I am hoping <http://trac.webkit.org/r171854> fixes this particular build configuration. > > I had to revert that change, since it broke building with the OS X 10.8 SDK. I will try something else in a moment. Tried another fix in <http://trac.webkit.org/r171857>. Committed attachment 235788 [details] as <http://trac.webkit.org/r171858>. (In reply to comment #15) > Committed attachment 235716 [details] as <http://trac.webkit.org/r171801>. This broke client certificate authentication, because NSURLCredential doesn’t serialize identities properly. See bug 138144. *** Bug 140197 has been marked as a duplicate of this bug. *** *** Bug 140197 has been marked as a duplicate of this bug. *** |