Bug 31825 – [Android] Android is missing implementation of SSL Key generator functions.

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Andrei Popescu]

WebCore requires the varioius platform ports to implement 

void getSupportedKeySizes(Vector<String>& keys)

and

String signedPublicKeyAndChallengeString(unsigned index, const String& challenge, const KURL& url)

Android is missing the implementation for these functions.

[Andrei Popescu]

Created attachment 43760 [details]
Add implementation of SSL Key Generator functions.

[Andrei Popescu]

Created attachment 43762 [details]
Add implementation of SSL Key Generator functions.

Fixed a style problem (indent inside a namespace).

[Eric Seidel (no email)]

Comment on attachment 43762 [details]
Add implementation of SSL Key Generator functions.

Bah!  ChromiumBridge was bad enough.  Sad that this model has infected android too.  Why wouldn't these be a Client interface hung off of Page?  Or why do these need to be implemented in WebKit at all instead of a library which WebCore could directly link to?

[Darin Fisher (:fishd, Google)]

> Bah!  ChromiumBridge was bad enough.  Sad that this model has infected android
> too.  Why wouldn't these be a Client interface hung off of Page?  Or why do
> these need to be implemented in WebKit at all instead of a library which
> WebCore could directly link to?

Andrei is trying to move the Android port to match the same model as Chromium so that we can share the WebKit API.  In that world, the methods contained by ChromiumBridge correspond to WebKitClient method.  It seems to me of little value to introduce an intermediate Client interface hung off of Page for this.  The value of ChromiumBridge (or PlatformBridge) is that it provides a grouping for these methods.  I don't understand you want more layers here.

Andrei and I discussed renaming ChromiumBridge to PlatformBridge.  I would personally prefer to see that change happen first.

[Eric Seidel (no email)]

(In reply to comment #4)
> Andrei is trying to move the Android port to match the same model as Chromium
> so that we can share the WebKit API.  In that world, the methods contained by
> ChromiumBridge correspond to WebKitClient method.  It seems to me of little
> value to introduce an intermediate Client interface hung off of Page for this. 
> The value of ChromiumBridge (or PlatformBridge) is that it provides a grouping
> for these methods.  I don't understand you want more layers here.

Yeah, I think you and I have fallen out of step wrt to ChromiumBridge design.  I'll make a point of syncing up with you about such when I'm in MTV next week.

WebCore in the Apple ports is self-contained.  It only has down-dependencies.  It feels strange to me that in the Chromium port it has hard up-dependencies (no default implementation), but maybe that makes sense given the sandboxing model.

[Eric Seidel (no email)]

More aptly said: I've fallen behind in my understanding of the grand vision of ChromiumBridge.  I thought it was meant as a temporary step before better Clients could be designed.  It no longer seems that way, if we're encouraging other ports to use it.  My understanding of WebCore/WebKit design is that Clients were the way to talk up, and platform was the way to talk down.  Chromium bridge is a linking shim which allows Platform to talk up to WebKit.  Which feels strange to me.

[Andrei Popescu]

(In reply to comment #6)
> More aptly said: I've fallen behind in my understanding of the grand vision of
> ChromiumBridge.  I thought it was meant as a temporary step before better
> Clients could be designed.  It no longer seems that way, if we're encouraging
> other ports to use it.  My understanding of WebCore/WebKit design is that
> Clients were the way to talk up, and platform was the way to talk down. 
> Chromium bridge is a linking shim which allows Platform to talk up to WebKit. 
> Which feels strange to me.

Ok, but do you think we could go with this design for now (given we already have a precedent) and then change once we agree if and how having individual clients for the various features in the Chromium/Platform Bridges would help? I think that's a discussion that could perhaps happen on its own bug and the solution that will be reached should be applied to both Android and Chromium. I don't think we should block the Android code upstreaming on this.

Thanks,
Andrei

[Eric Seidel (no email)]

Comment on attachment 43762 [details]
Add implementation of SSL Key Generator functions.

"url" is technically against webkit style, as it should be removed since it's not necessary:
 44     static String getSignedPublicKeyAndChallengeString(unsigned index, const String& challenge, const KURL& url);

I don't like the design as I currently understand it, but I agree, that's a discussion for another bug.  Can you document why it is that this needs to use a PlatformBridge shim like this?  Does Andriod use a sandbox like Chromium?  Does this call reach across an IPC layer?

[Andrei Popescu]

Created attachment 43798 [details]
Add implementation of SSL Key Generator functions v3

[Andrei Popescu]

Created attachment 43799 [details]
Add implementation of SSL Key Generator functions v3

Uploading the right patch this time.

[Andrei Popescu]

(In reply to comment #8)
> (From update of attachment 43762 [details])
> "url" is technically against webkit style, as it should be removed since it's
> not necessary:
>  44     static String getSignedPublicKeyAndChallengeString(unsigned index,
> const String& challenge, const KURL& url);
> 

Removed.

> I don't like the design as I currently understand it, but I agree, that's a
> discussion for another bug.  Can you document why it is that this needs to use
> a PlatformBridge shim like this?  Does Andriod use a sandbox like Chromium? 
> Does this call reach across an IPC layer?

Added appropriate comment in PlatformBridge.h

Thanks,
Andrei

[Eric Seidel (no email)]

Comment on attachment 43799 [details]
Add implementation of SSL Key Generator functions v3

OK.  Andrei is not in committers.py, so marking cq+ as well.

[WebKit Commit Bot]

Comment on attachment 43799 [details]
Add implementation of SSL Key Generator functions v3

Clearing flags on attachment: 43799

Committed r51384: <http://trac.webkit.org/changeset/51384>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.