[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 #18 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 356844
  --> https://bugs.webkit.org/attachment.cgi?id=356844
Patch

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

> Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp:226
> +    auto continueCheckingRequestOrAllowSimulatedRedirect = [this](ResourceRequest&& originalRequest, ResourceRequest&& currentRequest, ValidationHandler&& handler) {

continueCheckingRequestOrAllowSimulatedRedirect -> continueCheckingRequestOrDoSimulatedRedirect ?

> Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp:228
> +        if ((m_requestLoadType != LoadType::MainFrame) && (currentRequest.url() != originalRequest.url())) {

Isn't this supposed to be m_requestLoadType == LoadType::MainFrame ?

Also, we prefer mot to avoid unnecessary parentheses.

> Source/WebKit/NetworkProcess/NetworkLoadChecker.h:88
> +    void setRequestLoadType(LoadType requestLoadType) { m_requestLoadType = requestLoadType; }

As I mentioned before, I think it may look better without a setter and passing it as a parameter with a default value.

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

Methods should come *before* data members for clarity.

> Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:212
> +            if (isMainFrameLoad() && (originalRequest().url() != currentRequest.url())) {

It is kind of sad that this check is duplicated from NetworkLoadChecker. I think it would look better it the check was done only in NetworkLoadChecker, and if we'd pass an extra "ResourceResponse redirect" to this lambda. We'd then have a if (!redirect.isNull()) check here instead.

-- 
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/20181208/8278c8aa/attachment-0001.html>