Commit 408d6ef1 authored by Hiroki Nakagawa's avatar Hiroki Nakagawa Committed by Commit Bot

WebSocket: Reduce dependencies on Document

This is a preparation CL for enabling to establish WebSocket connections off the
main thread.

This CL reduces dependencies on Document in DocumentWebSocketChannel as a first
step for making it thread-safe. Specifically, this CL replaces GetDocument()
calls with GetExecutionContext() calls, and makes the channel get
SubresourceFilter not from DocumentLoader buf from BaseFetchContext.

Bug: 825740
Change-Id: Ibe49fa09a613818ec972b40a8141f7eecc5a3861
Reviewed-on: https://chromium-review.googlesource.com/981732
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: default avatarKinuko Yasuda <kinuko@chromium.org>
Reviewed-by: default avatarYutaka Hirano <yhirano@chromium.org>
Reviewed-by: default avatarAdam Rice <ricea@chromium.org>
Reviewed-by: default avatarTsuyoshi Horo <horo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546061}
parent 0eadbe37
...@@ -50,6 +50,7 @@ class CORE_EXPORT BaseFetchContext : public FetchContext { ...@@ -50,6 +50,7 @@ class CORE_EXPORT BaseFetchContext : public FetchContext {
void Trace(blink::Visitor*) override; void Trace(blink::Visitor*) override;
virtual KURL GetSiteForCookies() const = 0; virtual KURL GetSiteForCookies() const = 0;
virtual SubresourceFilter* GetSubresourceFilter() const = 0;
virtual void CountUsage(WebFeature) const = 0; virtual void CountUsage(WebFeature) const = 0;
virtual void CountDeprecation(WebFeature) const = 0; virtual void CountDeprecation(WebFeature) const = 0;
...@@ -62,7 +63,6 @@ class CORE_EXPORT BaseFetchContext : public FetchContext { ...@@ -62,7 +63,6 @@ class CORE_EXPORT BaseFetchContext : public FetchContext {
protected: protected:
// Used for security checks. // Used for security checks.
virtual bool AllowScriptFromSource(const KURL&) const = 0; virtual bool AllowScriptFromSource(const KURL&) const = 0;
virtual SubresourceFilter* GetSubresourceFilter() const = 0;
// Note: subclasses are expected to override following methods. // Note: subclasses are expected to override following methods.
// Used in the default implementation for CanRequest, CanFollowRedirect // Used in the default implementation for CanRequest, CanFollowRedirect
......
...@@ -333,6 +333,13 @@ KURL FrameFetchContext::GetSiteForCookies() const { ...@@ -333,6 +333,13 @@ KURL FrameFetchContext::GetSiteForCookies() const {
return document->SiteForCookies(); return document->SiteForCookies();
} }
SubresourceFilter* FrameFetchContext::GetSubresourceFilter() const {
if (IsDetached())
return nullptr;
DocumentLoader* document_loader = MasterDocumentLoader();
return document_loader ? document_loader->GetSubresourceFilter() : nullptr;
}
LocalFrame* FrameFetchContext::GetFrame() const { LocalFrame* FrameFetchContext::GetFrame() const {
DCHECK(!IsDetached()); DCHECK(!IsDetached());
...@@ -999,13 +1006,6 @@ bool FrameFetchContext::AllowScriptFromSourceWithoutNotifying( ...@@ -999,13 +1006,6 @@ bool FrameFetchContext::AllowScriptFromSourceWithoutNotifying(
return true; return true;
} }
SubresourceFilter* FrameFetchContext::GetSubresourceFilter() const {
if (IsDetached())
return nullptr;
DocumentLoader* document_loader = MasterDocumentLoader();
return document_loader ? document_loader->GetSubresourceFilter() : nullptr;
}
bool FrameFetchContext::ShouldBlockRequestByInspector(const KURL& url) const { bool FrameFetchContext::ShouldBlockRequestByInspector(const KURL& url) const {
if (IsDetached()) if (IsDetached())
return false; return false;
......
...@@ -197,8 +197,8 @@ class CORE_EXPORT FrameFetchContext final : public BaseFetchContext { ...@@ -197,8 +197,8 @@ class CORE_EXPORT FrameFetchContext final : public BaseFetchContext {
// BaseFetchContext overrides: // BaseFetchContext overrides:
KURL GetSiteForCookies() const override; KURL GetSiteForCookies() const override;
bool AllowScriptFromSource(const KURL&) const override;
SubresourceFilter* GetSubresourceFilter() const override; SubresourceFilter* GetSubresourceFilter() const override;
bool AllowScriptFromSource(const KURL&) const override;
bool ShouldBlockRequestByInspector(const KURL&) const override; bool ShouldBlockRequestByInspector(const KURL&) const override;
void DispatchDidBlockRequest(const ResourceRequest&, void DispatchDidBlockRequest(const ResourceRequest&,
const FetchInitiatorInfo&, const FetchInitiatorInfo&,
......
...@@ -109,6 +109,10 @@ KURL WorkerFetchContext::GetSiteForCookies() const { ...@@ -109,6 +109,10 @@ KURL WorkerFetchContext::GetSiteForCookies() const {
return web_context_->SiteForCookies(); return web_context_->SiteForCookies();
} }
SubresourceFilter* WorkerFetchContext::GetSubresourceFilter() const {
return subresource_filter_.Get();
}
bool WorkerFetchContext::AllowScriptFromSource(const KURL&) const { bool WorkerFetchContext::AllowScriptFromSource(const KURL&) const {
// Currently we don't use WorkerFetchContext for loading scripts. So this // Currently we don't use WorkerFetchContext for loading scripts. So this
// method must not be called. // method must not be called.
...@@ -119,10 +123,6 @@ bool WorkerFetchContext::AllowScriptFromSource(const KURL&) const { ...@@ -119,10 +123,6 @@ bool WorkerFetchContext::AllowScriptFromSource(const KURL&) const {
return false; return false;
} }
SubresourceFilter* WorkerFetchContext::GetSubresourceFilter() const {
return subresource_filter_.Get();
}
bool WorkerFetchContext::ShouldBlockRequestByInspector(const KURL& url) const { bool WorkerFetchContext::ShouldBlockRequestByInspector(const KURL& url) const {
bool should_block_request = false; bool should_block_request = false;
probe::shouldBlockRequest(global_scope_, url, &should_block_request); probe::shouldBlockRequest(global_scope_, url, &should_block_request);
......
...@@ -35,8 +35,8 @@ class WorkerFetchContext final : public BaseFetchContext { ...@@ -35,8 +35,8 @@ class WorkerFetchContext final : public BaseFetchContext {
// BaseFetchContext implementation: // BaseFetchContext implementation:
KURL GetSiteForCookies() const override; KURL GetSiteForCookies() const override;
bool AllowScriptFromSource(const KURL&) const override;
SubresourceFilter* GetSubresourceFilter() const override; SubresourceFilter* GetSubresourceFilter() const override;
bool AllowScriptFromSource(const KURL&) const override;
bool ShouldBlockRequestByInspector(const KURL&) const override; bool ShouldBlockRequestByInspector(const KURL&) const override;
void DispatchDidBlockRequest(const ResourceRequest&, void DispatchDidBlockRequest(const ResourceRequest&,
const FetchInitiatorInfo&, const FetchInitiatorInfo&,
......
...@@ -41,8 +41,6 @@ ...@@ -41,8 +41,6 @@
#include "core/frame/WebLocalFrameImpl.h" #include "core/frame/WebLocalFrameImpl.h"
#include "core/inspector/ConsoleMessage.h" #include "core/inspector/ConsoleMessage.h"
#include "core/loader/BaseFetchContext.h" #include "core/loader/BaseFetchContext.h"
#include "core/loader/DocumentLoader.h"
#include "core/loader/FrameLoader.h"
#include "core/loader/MixedContentChecker.h" #include "core/loader/MixedContentChecker.h"
#include "core/loader/SubresourceFilter.h" #include "core/loader/SubresourceFilter.h"
#include "core/loader/ThreadableLoadingContext.h" #include "core/loader/ThreadableLoadingContext.h"
...@@ -208,27 +206,24 @@ bool DocumentWebSocketChannel::Connect( ...@@ -208,27 +206,24 @@ bool DocumentWebSocketChannel::Connect(
if (!handle_) if (!handle_)
return false; return false;
if (GetDocument()) { if (GetDocument() && GetDocument()->GetFrame()) {
if (GetDocument()->GetFrame()) { if (MixedContentChecker::ShouldBlockWebSocket(GetDocument()->GetFrame(),
if (MixedContentChecker::ShouldBlockWebSocket(GetDocument()->GetFrame(), url)) {
url)) return false;
return false;
}
if (MixedContentChecker::IsMixedContent(GetDocument()->GetSecurityOrigin(),
url)) {
String message =
"Connecting to a non-secure WebSocket server from a secure origin is "
"deprecated.";
GetDocument()->AddConsoleMessage(ConsoleMessage::Create(
kJSMessageSource, kWarningMessageLevel, message));
} }
connection_handle_for_scheduler_ = GetDocument()
->GetFrame()
->GetFrameScheduler()
->OnActiveConnectionCreated();
}
if (GetDocument()->GetFrame()) { if (MixedContentChecker::IsMixedContent(
connection_handle_for_scheduler_ = GetDocument() GetExecutionContext()->GetSecurityOrigin(), url)) {
->GetFrame() String message =
->GetFrameScheduler() "Connecting to a non-secure WebSocket server from a secure origin is "
->OnActiveConnectionCreated(); "deprecated.";
} GetExecutionContext()->AddConsoleMessage(ConsoleMessage::Create(
kJSMessageSource, kWarningMessageLevel, message));
} }
url_ = url; url_ = url;
...@@ -245,7 +240,7 @@ bool DocumentWebSocketChannel::Connect( ...@@ -245,7 +240,7 @@ bool DocumentWebSocketChannel::Connect(
// failure blocks the worker thread which should be avoided. Note that // failure blocks the worker thread which should be avoided. Note that
// returning "true" just indicates that this was not a mixed content error. // returning "true" just indicates that this was not a mixed content error.
if (ShouldDisallowConnection(url)) { if (ShouldDisallowConnection(url)) {
GetDocument() GetExecutionContext()
->GetTaskRunner(TaskType::kNetworking) ->GetTaskRunner(TaskType::kNetworking)
->PostTask( ->PostTask(
FROM_HERE, FROM_HERE,
...@@ -387,13 +382,12 @@ void DocumentWebSocketChannel::Fail(const String& reason, ...@@ -387,13 +382,12 @@ void DocumentWebSocketChannel::Fail(const String& reason,
MessageLevel level, MessageLevel level,
std::unique_ptr<SourceLocation> location) { std::unique_ptr<SourceLocation> location) {
NETWORK_DVLOG(1) << this << " Fail(" << reason << ")"; NETWORK_DVLOG(1) << this << " Fail(" << reason << ")";
if (GetDocument()) { if (GetDocument())
probe::didReceiveWebSocketFrameError(GetDocument(), identifier_, reason); probe::didReceiveWebSocketFrameError(GetDocument(), identifier_, reason);
const String message = "WebSocket connection to '" + url_.ElidedString() + const String message =
"' failed: " + reason; "WebSocket connection to '" + url_.ElidedString() + "' failed: " + reason;
GetDocument()->AddConsoleMessage(ConsoleMessage::Create( GetExecutionContext()->AddConsoleMessage(ConsoleMessage::Create(
kJSMessageSource, level, message, std::move(location))); kJSMessageSource, level, message, std::move(location)));
}
// |reason| is only for logging and should not be provided for scripts, // |reason| is only for logging and should not be provided for scripts,
// hence close reason must be empty in tearDownFailedConnection. // hence close reason must be empty in tearDownFailedConnection.
TearDownFailedConnection(); TearDownFailedConnection();
...@@ -560,12 +554,16 @@ void DocumentWebSocketChannel::HandleDidClose(bool was_clean, ...@@ -560,12 +554,16 @@ void DocumentWebSocketChannel::HandleDidClose(bool was_clean,
} }
Document* DocumentWebSocketChannel::GetDocument() { Document* DocumentWebSocketChannel::GetDocument() {
ExecutionContext* context = loading_context_->GetExecutionContext(); ExecutionContext* context = GetExecutionContext();
if (context->IsDocument()) if (context->IsDocument())
return ToDocument(context); return ToDocument(context);
return nullptr; return nullptr;
} }
ExecutionContext* DocumentWebSocketChannel::GetExecutionContext() {
return loading_context_->GetExecutionContext();
}
void DocumentWebSocketChannel::DidConnect(WebSocketHandle* handle, void DocumentWebSocketChannel::DidConnect(WebSocketHandle* handle,
const String& selected_protocol, const String& selected_protocol,
const String& extensions) { const String& extensions) {
...@@ -821,10 +819,8 @@ void DocumentWebSocketChannel::TearDownFailedConnection() { ...@@ -821,10 +819,8 @@ void DocumentWebSocketChannel::TearDownFailedConnection() {
bool DocumentWebSocketChannel::ShouldDisallowConnection(const KURL& url) { bool DocumentWebSocketChannel::ShouldDisallowConnection(const KURL& url) {
DCHECK(handle_); DCHECK(handle_);
DocumentLoader* loader = GetDocument()->Loader(); BaseFetchContext* fetch_context = loading_context_->GetFetchContext();
if (!loader) SubresourceFilter* subresource_filter = fetch_context->GetSubresourceFilter();
return false;
SubresourceFilter* subresource_filter = loader->GetSubresourceFilter();
if (!subresource_filter) if (!subresource_filter)
return false; return false;
return !subresource_filter->AllowWebSocketConnection(url); return !subresource_filter->AllowWebSocketConnection(url);
......
...@@ -155,8 +155,9 @@ class MODULES_EXPORT DocumentWebSocketChannel final ...@@ -155,8 +155,9 @@ class MODULES_EXPORT DocumentWebSocketChannel final
const String& reason); const String& reason);
// This may return nullptr. // This may return nullptr.
// TODO(kinuko): Remove dependency to document. // TODO(nhiroki): Remove dependency to document (https://crbug.com/825740).
Document* GetDocument(); Document* GetDocument();
ExecutionContext* GetExecutionContext();
// WebSocketHandleClient functions. // WebSocketHandleClient functions.
void DidConnect(WebSocketHandle*, void DidConnect(WebSocketHandle*,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment