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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Dec 7 14:51:02 PST 2018


--- Comment #14 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 356838
  --> https://bugs.webkit.org/attachment.cgi?id=356838

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>

More information about the webkit-unassigned mailing list