[Webkit-unassigned] [Bug 192375] HTTPS Upgrade: Figure out if/how to tell clients that the HTTPS upgrade happened

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=192375

Vivek Seth <v_seth at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |v_seth at apple.com

--- Comment #16 from Vivek Seth <v_seth at apple.com> ---
Comment on attachment 356838
  --> https://bugs.webkit.org/attachment.cgi?id=356838
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=356838&action=review

>> Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp:67
>> +    , m_requestLoadType(LoadType::Other)
> 
> Not needed with inline initialization I suggest below.

OK

>> Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp:258
>> +        if (result.value().request.url() != originalRequest.url()) {
> 
> Didn't we want to simulate redirects only for main-frame loads? Seems like we are now bypassing the checks in NetworkLoadChecker::continueCheckingRequest() for non-main frame loads (since those do not trigger a simulated redirect and a re-check).

Fixed this

>> Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp:267
>> +    if (request.url() != originalRequest.url()) {
> 
> Can we try to avoid code duplication with above?

Fixed it

>> Source/WebKit/NetworkProcess/NetworkLoadChecker.h:88
>> +    void setRequestLoadType(LoadType requestLoadType) { m_requestLoadType = requestLoadType; }
> 
> Should it be a constructor  parameter instead? Maybe with a default initialization value.

OK

>> Source/WebKit/NetworkProcess/NetworkLoadChecker.h:145
>> +    LoadType m_requestLoadType;
> 
> m_requestLoadType { LoadType::Other };

OK

>> Source/WebKit/NetworkProcess/NetworkLoadChecker.h:148
>> +    bool applyHTTPSUpgradeIfNeeded(WebCore::ResourceRequest&);
> 
> cannot this be "const"?

Yes it can. Fixed it.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20181207/54ad86f0/attachment-0001.html>