> 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?
If so, I'm OK with the current structure. 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
