[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

--- Comment #14 from Chris Dumez <cdumez 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.

> 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).

> Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp:267
> +    if (request.url() != originalRequest.url()) {

Can we try to avoid code duplication with above?

> 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.

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

m_requestLoadType { LoadType::Other };

> Source/WebKit/NetworkProcess/NetworkLoadChecker.h:148
> +    bool applyHTTPSUpgradeIfNeeded(WebCore::ResourceRequest&);

cannot this be "const"?

-- 
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/1ea568d7/attachment.html>