Commit e973af85 authored by Hiroki Nakagawa's avatar Hiroki Nakagawa Committed by Commit Bot

Worker: Reduce usage of WorkerOrWorkletGlobalScope in OutsideSettingsCSPDelegate

OutsideSettingsCSPDelegate is designed as ExecutionContext-free as much
as possible. This CL reduces usage of WorkerOrWorkletGlobalScope there
by...

- replacing thread checks with base::ThreadChecker utility, and
- logging in the use counter using blink::UseCounter interface.

This doesn't change functional behavior.

Bug: n/a
Change-Id: Iae6ffc504e5f6513369525aeec8a3544cdb76b22
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2391892
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: default avatarHiroshige Hayashizaki <hiroshige@chromium.org>
Cr-Commit-Position: refs/heads/master@{#804767}
parent 30014d1b
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
#include "third_party/blink/renderer/core/workers/worker_or_worklet_global_scope.h" #include "third_party/blink/renderer/core/workers/worker_or_worklet_global_scope.h"
#include "base/threading/thread_checker.h"
#include "services/network/public/mojom/web_sandbox_flags.mojom-blink.h" #include "services/network/public/mojom/web_sandbox_flags.mojom-blink.h"
#include "third_party/blink/public/common/features.h" #include "third_party/blink/public/common/features.h"
#include "third_party/blink/public/mojom/devtools/inspector_issue.mojom-blink.h" #include "third_party/blink/public/mojom/devtools/inspector_issue.mojom-blink.h"
...@@ -58,40 +59,37 @@ class OutsideSettingsCSPDelegate final ...@@ -58,40 +59,37 @@ class OutsideSettingsCSPDelegate final
public: public:
OutsideSettingsCSPDelegate( OutsideSettingsCSPDelegate(
const FetchClientSettingsObject& outside_settings_object, const FetchClientSettingsObject& outside_settings_object,
UseCounter& use_counter,
WorkerOrWorkletGlobalScope& global_scope_for_logging) WorkerOrWorkletGlobalScope& global_scope_for_logging)
: outside_settings_object_(&outside_settings_object), : outside_settings_object_(&outside_settings_object),
global_scope_for_logging_(&global_scope_for_logging) { use_counter_(use_counter),
DCHECK(global_scope_for_logging_->IsContextThread()); global_scope_for_logging_(&global_scope_for_logging) {}
}
void Trace(Visitor* visitor) const override { void Trace(Visitor* visitor) const override {
visitor->Trace(global_scope_for_logging_); visitor->Trace(global_scope_for_logging_);
visitor->Trace(use_counter_);
visitor->Trace(outside_settings_object_); visitor->Trace(outside_settings_object_);
} }
const KURL& Url() const override { const KURL& Url() const override {
DCHECK(global_scope_for_logging_->IsContextThread()); DCHECK_CALLED_ON_VALID_THREAD(worker_thread_checker_);
return outside_settings_object_->GlobalObjectUrl(); return outside_settings_object_->GlobalObjectUrl();
} }
const SecurityOrigin* GetSecurityOrigin() override { const SecurityOrigin* GetSecurityOrigin() override {
DCHECK(global_scope_for_logging_->IsContextThread()); DCHECK_CALLED_ON_VALID_THREAD(worker_thread_checker_);
return outside_settings_object_->GetSecurityOrigin(); return outside_settings_object_->GetSecurityOrigin();
} }
// We don't have to do anything here, as we don't want to update // We don't have to do anything here, as we don't want to update
// SecurityContext of either parent context or WorkerOrWorkletGlobalScope. // SecurityContext of either parent context or WorkerOrWorkletGlobalScope.
// TODO(hiroshige): Revisit the relationship of ContentSecurityPolicy,
// SecurityContext and FetchClientSettingsObject, e.g. when doing
// off-the-main-thread shared worker/service worker top-level script fetch.
// https://crbug.com/924041 https://crbug.com/924043
void SetSandboxFlags(network::mojom::blink::WebSandboxFlags) override {} void SetSandboxFlags(network::mojom::blink::WebSandboxFlags) override {}
void SetRequireTrustedTypes() override {} void SetRequireTrustedTypes() override {}
void AddInsecureRequestPolicy(mojom::blink::InsecureRequestPolicy) override {} void AddInsecureRequestPolicy(mojom::blink::InsecureRequestPolicy) override {}
void DisableEval(const String& error_message) override {} void DisableEval(const String& error_message) override {}
std::unique_ptr<SourceLocation> GetSourceLocation() override { std::unique_ptr<SourceLocation> GetSourceLocation() override {
DCHECK(global_scope_for_logging_->IsContextThread()); DCHECK_CALLED_ON_VALID_THREAD(worker_thread_checker_);
// https://w3c.github.io/webappsec-csp/#create-violation-for-global // https://w3c.github.io/webappsec-csp/#create-violation-for-global
// Step 2. If the user agent is currently executing script, and can extract // Step 2. If the user agent is currently executing script, and can extract
// a source file's URL, line number, and column number from the global, set // a source file's URL, line number, and column number from the global, set
...@@ -104,20 +102,20 @@ class OutsideSettingsCSPDelegate final ...@@ -104,20 +102,20 @@ class OutsideSettingsCSPDelegate final
} }
base::Optional<uint16_t> GetStatusCode() override { base::Optional<uint16_t> GetStatusCode() override {
DCHECK(global_scope_for_logging_->IsContextThread()); DCHECK_CALLED_ON_VALID_THREAD(worker_thread_checker_);
// TODO(crbug/928965): Plumb the status code of the parent Document if any. // TODO(crbug/928965): Plumb the status code of the parent Document if any.
return base::nullopt; return base::nullopt;
} }
String GetDocumentReferrer() override { String GetDocumentReferrer() override {
DCHECK(global_scope_for_logging_->IsContextThread()); DCHECK_CALLED_ON_VALID_THREAD(worker_thread_checker_);
// TODO(crbug/928965): Plumb the referrer from the parent context. // TODO(crbug/928965): Plumb the referrer from the parent context.
return String(); return String();
} }
void DispatchViolationEvent(const SecurityPolicyViolationEventInit&, void DispatchViolationEvent(const SecurityPolicyViolationEventInit&,
Element*) override { Element*) override {
DCHECK(global_scope_for_logging_->IsContextThread()); DCHECK_CALLED_ON_VALID_THREAD(worker_thread_checker_);
// TODO(crbug/928964): Fire an event on the parent context. // TODO(crbug/928964): Fire an event on the parent context.
// Before OutsideSettingsCSPDelegate was introduced, the event had been // Before OutsideSettingsCSPDelegate was introduced, the event had been
// fired on WorkerGlobalScope, which had been virtually no-op because // fired on WorkerGlobalScope, which had been virtually no-op because
...@@ -130,17 +128,17 @@ class OutsideSettingsCSPDelegate final ...@@ -130,17 +128,17 @@ class OutsideSettingsCSPDelegate final
bool is_frame_ancestors_violation, bool is_frame_ancestors_violation,
const Vector<String>& report_endpoints, const Vector<String>& report_endpoints,
bool use_reporting_api) override { bool use_reporting_api) override {
DCHECK_CALLED_ON_VALID_THREAD(worker_thread_checker_);
// TODO(crbug/929370): Support POSTing violation reports from a Worker. // TODO(crbug/929370): Support POSTing violation reports from a Worker.
DCHECK(global_scope_for_logging_->IsContextThread());
} }
void Count(WebFeature feature) override { void Count(WebFeature feature) override {
DCHECK(global_scope_for_logging_->IsContextThread()); DCHECK_CALLED_ON_VALID_THREAD(worker_thread_checker_);
global_scope_for_logging_->CountUse(feature); use_counter_->CountUse(feature);
} }
void AddConsoleMessage(ConsoleMessage* message) override { void AddConsoleMessage(ConsoleMessage* message) override {
DCHECK(global_scope_for_logging_->IsContextThread()); DCHECK_CALLED_ON_VALID_THREAD(worker_thread_checker_);
global_scope_for_logging_->AddConsoleMessage(message); global_scope_for_logging_->AddConsoleMessage(message);
} }
...@@ -152,7 +150,7 @@ class OutsideSettingsCSPDelegate final ...@@ -152,7 +150,7 @@ class OutsideSettingsCSPDelegate final
void DidAddContentSecurityPolicies( void DidAddContentSecurityPolicies(
WTF::Vector<network::mojom::blink::ContentSecurityPolicyPtr>) override { WTF::Vector<network::mojom::blink::ContentSecurityPolicyPtr>) override {
DCHECK(global_scope_for_logging_->IsContextThread()); DCHECK_CALLED_ON_VALID_THREAD(worker_thread_checker_);
// We do nothing here, because if the added policies should be reported to // We do nothing here, because if the added policies should be reported to
// LocalFrameClient, then they are already reported on the parent // LocalFrameClient, then they are already reported on the parent
// Document. // Document.
...@@ -161,16 +159,19 @@ class OutsideSettingsCSPDelegate final ...@@ -161,16 +159,19 @@ class OutsideSettingsCSPDelegate final
} }
void AddInspectorIssue(mojom::blink::InspectorIssueInfoPtr info) override { void AddInspectorIssue(mojom::blink::InspectorIssueInfoPtr info) override {
DCHECK(global_scope_for_logging_->IsContextThread()); DCHECK_CALLED_ON_VALID_THREAD(worker_thread_checker_);
global_scope_for_logging_->AddInspectorIssue(std::move(info)); global_scope_for_logging_->AddInspectorIssue(std::move(info));
} }
private: private:
const Member<const FetchClientSettingsObject> outside_settings_object_; const Member<const FetchClientSettingsObject> outside_settings_object_;
const Member<UseCounter> use_counter_;
// |global_scope_for_logging_| should be used only for CountUse(), // |global_scope_for_logging_| should be used only for AddConsoleMessage() and
// AddConsoleMessage(), and AddInspectorIssue(). // AddInspectorIssue().
const Member<WorkerOrWorkletGlobalScope> global_scope_for_logging_; const Member<WorkerOrWorkletGlobalScope> global_scope_for_logging_;
THREAD_CHECKER(worker_thread_checker_);
}; };
} // namespace } // namespace
...@@ -380,7 +381,7 @@ ResourceFetcher* WorkerOrWorkletGlobalScope::CreateOutsideSettingsFetcher( ...@@ -380,7 +381,7 @@ ResourceFetcher* WorkerOrWorkletGlobalScope::CreateOutsideSettingsFetcher(
OutsideSettingsCSPDelegate* csp_delegate = OutsideSettingsCSPDelegate* csp_delegate =
MakeGarbageCollected<OutsideSettingsCSPDelegate>(outside_settings_object, MakeGarbageCollected<OutsideSettingsCSPDelegate>(outside_settings_object,
*this); *this, *this);
content_security_policy->BindToDelegate(*csp_delegate); content_security_policy->BindToDelegate(*csp_delegate);
return CreateFetcherInternal(outside_settings_object, return CreateFetcherInternal(outside_settings_object,
......
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