[webkit-reviews] review canceled: [Bug 129740] [EFL][WK2] Add ewk APIs to control TLS error policy on WebContext. : [Attachment 227659] patch

[bugzilla-daemon at webkit.org]

Peter Molnar <pmolnar.u-szeged at partner.samsung.com> has canceled Peter Molnar
<pmolnar.u-szeged at partner.samsung.com>'s request for review:
Bug 129740: [EFL][WK2] Add ewk APIs to control TLS error policy on WebContext.
https://bugs.webkit.org/show_bug.cgi?id=129740

Attachment 227659: patch
https://bugs.webkit.org/attachment.cgi?id=227659&action=review

------- Additional Comments from Peter Molnar
<pmolnar.u-szeged at partner.samsung.com>
(In reply to comment #28)
> (From update of attachment 227659 [details])
> View in context:
https://bugs.webkit.org/attachment.cgi?id=227659&action=review
> 
> Patch looks getting better.
> 
> >
Source/WebKit2/UIProcess/API/efl/tests/UnitTestUtils/EWK2UnitTestServer.h:29
> > +	 EWK2UnitTestServer(GTlsCertificate* tlsCert = nullptr);
> 
> Missing *explicit* keyword ?

Fixed.

> 
> >
Source/WebKit2/UIProcess/API/efl/tests/UnitTestUtils/EWK2UnitTestServer.h:30
> >	 virtual ~EWK2UnitTestServer();
> 
> Unrelated to this patch though, why this dtor was declared as virtual :(

I assume that in this case I don't have to fix it in this patch.

> 
> > Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_ssl.cpp:182
> > +	 waitUntilTrue(finishTest, testTimeoutSeconds);
> 
> Any reason we have to wait until testTimeoutSeconds ?

Waiting for the timeout is just for the case if something really nasty happens.
If either onLoadFinished or onLoadProvisionalFailed event is triggered,
finishTest will become true, and the test finishes immediately (600-800 msec on
average). In normal test runs either of them will be very likely to happen.

So it's just for the case when none of these events happen, so finishTest would
stay false forever (e.g. the cert can't be created from the supplied PEM,
server can't start for some reason, etc.).