On 6/7/2018 6:29 PM, Weijun Wang wrote:
Have you updated it yourself?
I did not.
I have a proposal at
http://cr.openjdk.java.net/~weijun/9999999/webrev.hello-cookie-manager/
Looks fine to me.
Minor nit: Would you mind limit the maximum line character to 80
characters (line 250)?
Thanks,
Xuelei
--Max
On Jun 6, 2018, at 10:40 PM, Xuelei Fan <xuelei....@oracle.com> wrote:
On 6/5/2018 8:48 PM, Weijun Wang wrote:
On 6/5/2018 6:54 AM, Weijun Wang wrote:
HelloCookieManager.java:
44 HelloCookieManager(SecureRandom secureRandom) {
45 this.secureRandom = secureRandom;
46 }
47
48 HelloCookieManager valueOf(ProtocolVersion protocolVersion) {
Why not just create a static method and make HelloCookieManager abstract?
static HelloCookieManager of(SecureRandom secureRandom, ProtocolVersion
protocolVersion)
I haven't seen "raw" HelloCookieManager used anywhere.
For every SSLContext, the SecureRandom may be different, so we need a different
HelloCookieManager instance and cookie managers for each SSLContext instance:
SSLContextImpl.java
-------------------
253 helloCookieManager = new HelloCookieManager(secureRandom);
So the main benefit of the current implementation is that once a
HelloCookieManager is created you can reuse the 3 child HelloCookieManager
objects inside it. Right?
Yes.
If so, I'm OK with the current structure.
I see you concerns. The structure looks weird in some sense, although it is
easier for a package private class. Let me see if I can make an update in the
next changeset.
Thanks,
Xuelei
My major concern is, since the "base" HelloCookieManager is never useful we
should take great care not to create an instance of it. How about this?
1. Make HelloCookieManager abstract.
2. Create an non-abstract inner class HelloCookieManager$Builder and move the 3
real instances there.
3. Inside SSLContextImpl, store a field of HelloCookieManager$Builder, and in
getHelloCookieManager(), call "builder.get(protocolVersion)" to return one of
the 3 real instances.
If the reuse of the 3 real instances is not a big benefit, you can just make
HelloCookieManager abstract and create a new child instance in every
getHelloCookieManager() call.
Anyway, I feel uncomfortable that the base HelloCookieManager class can be
instantiated.
Thanks
Max
On Jun 5, 2018, at 12:12 PM, Xuelei Fan <xuelei....@oracle.com> wrote:
http://cr.openjdk.java.net/~xuelei/8196584/webrev-full.01
