Add IDL definitions and verify PasswordCredential object can be instantiated. A. Add IDL definition for Credential: <https://w3c.github.io/webappsec-credential-management/#interfaces-credential-types-credential>. B. Add IDL definition for SiteBoundCredential: <https://w3c.github.io/webappsec-credential-management/#interfaces-credential-types-siteboundcredential>. C. Add IDL definition for PasswordCredential: <https://w3c.github.io/webappsec-credential-management/#interfaces-credential-types-passwordcredential>. D. Write tests for the interfaces Credential, SiteBoundCredential, and PasswordCredential.
<rdar://problem/30167149>
Created attachment 302189 [details] Patch
Comment on attachment 302189 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=302189&action=review > Source/WebCore/Modules/credentials/PasswordCredential.cpp:34 > + // This is a dummy constructor for now. This comment doesn't add much. > Source/WebCore/Modules/credentials/PasswordCredential.h:28 > +#include "DomFormData.h" This should bee DOMFormData.h > Source/WebCore/Modules/credentials/PasswordCredential.h:30 > +#include "URLSearchParams.h" If you added an out of line destructor, you could probably #include less. > Source/WebCore/Modules/credentials/PasswordCredential.h:31 > +#include <wtf/RefCounted.h> I don't think you need this. > Source/WebCore/Modules/credentials/PasswordCredential.idl:29 > + Constructor(PasswordCredentialData data), The spec has an additional constructor that takes an HTMLFormElement. Why leave that out? > Source/WebCore/Modules/credentials/SiteBoundCredential.cpp:35 > + , m_origin(URL()) This is unnecessary. > Source/WebCore/Modules/credentials/SiteBoundCredential.h:50 > + URL m_origin; Hard to tell, since it is unused, but should this be a SecurityOrigin/SecurityOriginData instead of a URL if it just needed for the origin?
Comment on attachment 302189 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=302189&action=review Please take care to fix the Windows and GTK builds before landing. We will need to limit these interfaces to a secure context. Towards this we will have to introduce such a concept. We can do this is in a subsequent patch. Please address Sam Weinig's remarks. In particular, we should look to store a SecurityOrigin in the [[origin]] slot in a SiteBoundCredential object instead of using a URL. > Source/WebCore/Modules/credentials/BasicCredential.cpp:45 > + return "password"; We should use ASCIILiteral() here to avoid making a copying of the string literal. > Source/WebCore/Modules/credentials/BasicCredential.cpp:47 > + return "federated"; Ditto.
Comment on attachment 302189 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=302189&action=review Thanks Sam and Dan for reviewing my patch. >> Source/WebCore/Modules/credentials/BasicCredential.cpp:45 >> + return "password"; > > We should use ASCIILiteral() here to avoid making a copying of the string literal. Fixed. >> Source/WebCore/Modules/credentials/BasicCredential.cpp:47 >> + return "federated"; > > Ditto. Fixed. >> Source/WebCore/Modules/credentials/PasswordCredential.cpp:34 >> + // This is a dummy constructor for now. > > This comment doesn't add much. Removed. >> Source/WebCore/Modules/credentials/PasswordCredential.h:28 >> +#include "DomFormData.h" > > This should bee DOMFormData.h Fixed. >> Source/WebCore/Modules/credentials/PasswordCredential.h:30 >> +#include "URLSearchParams.h" > > If you added an out of line destructor, you could probably #include less. Doesn't seems working. >> Source/WebCore/Modules/credentials/PasswordCredential.h:31 >> +#include <wtf/RefCounted.h> > > I don't think you need this. Removed. >> Source/WebCore/Modules/credentials/PasswordCredential.idl:29 >> + Constructor(PasswordCredentialData data), > > The spec has an additional constructor that takes an HTMLFormElement. Why leave that out? Added a dummy one as well. >> Source/WebCore/Modules/credentials/SiteBoundCredential.cpp:35 >> + , m_origin(URL()) > > This is unnecessary. Removed. >> Source/WebCore/Modules/credentials/SiteBoundCredential.h:50 >> + URL m_origin; > > Hard to tell, since it is unused, but should this be a SecurityOrigin/SecurityOriginData instead of a URL if it just needed for the origin? Changed to SecurityOrigin.
Created attachment 302580 [details] Patch for landing
Created attachment 302585 [details] Patch for landing
Comment on attachment 302585 [details] Patch for landing Attachment 302585 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/3181564 New failing tests: media/modern-media-controls/volume-down-support/volume-down-support.html
Created attachment 302600 [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
Comment on attachment 302585 [details] Patch for landing Clearing flags on attachment: 302585 Committed r213081: <http://trac.webkit.org/changeset/213081>