freebsd/ports
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
ports/www/qt6-webengine/files/patch-security-rollup
Jason E. Hale bef8e408ff Qt6: Update to 6.7.0 
Qt 6.7 is out with lots of large and small improvements for all of us
who like to have fun when building modern applications and user
experiences, focusing on the expansion of supported platforms and
industry standards. This makes code written with Qt more sustainable
and brings more value in Qt as a long-term investment.

www/qt6-webengine is now based on Chromium version 118.0.5993.220.

Announcement: https://www.qt.io/blog/qt-6.7-released
Release note: https://code.qt.io/cgit/qt/qtreleasenotes.git/about/qt/6.7.0/release-note.md

PySide6: Update to 6.7.0

PySide6 and its related components have been updated alongside the Qt
release for compatibility. Older versions will not build with Qt 6.7.x.

Announcement: https://www.qt.io/blog/qt-for-python-release-6.7

PyQt6: Update to 6.7.0

Minor Makefile cleanups and simplification of MASTER_SITES in pyqt.mk.
As with PySide6, older versions of PyQt6 will not build with Qt 6.7.x.

Announcement: https://www.riverbankcomputing.com/news/PyQt_v6.7.0_Released

PR:		278658
Exp-run by:	antoine
MFH:		2024Q2
Security:	e79cc4e2-12d7-11ef-83d8-4ccc6adda413,
		c6f03ea6-12de-11ef-83d8-4ccc6adda413
2024-05-15 13:55:48 -04:00

6092 lines
274 KiB
Plaintext
Raw Blame History

Add security patches to this file.
Addresses the following security issues:
- CVE-2023-7104
- CVE-2024-2625
- CVE-2024-2626
- Security bug 40066823
- Security bug 41495984
- CVE-2024-2885
- CVE-2024-2887
- Security bug 329674887
- Security bug 327183408
- CVE-2024-3159
- Security bug 326349405
- CVE-2024-3157
- CVE-2024-3516
- Security bug 326521449
- CVE-2024-3839
- CVE-2024-3837
- Security bug 327698060
- Security bug 40940917
- CVE-2024-3914
- Security bug 326498393
- CVE-2024-3840
- Security bug 323898565
- CVE-2024-4058
- CVE-2024-4060
- Security bug 332724843
- CVE-2024-4331
- CVE-2024-4368
- CVE-2024-4671
- Security bug 339458194
From b8c9622b71d032a48412e342cff91fc0f3f5e3d9 Mon Sep 17 00:00:00 2001
From: Michal Klocek <michal.klocek@qt.io>
Date: Tue, 19 Mar 2024 21:20:24 +0100
Subject: [PATCH] CVE-2023-7104
Fix a buffer overread in the sessions extension
that could occur when processing a corrupt changeset.
Backported from:
https://sqlite.org/src/info/0e4e7a05c4204b47
Change-Id: I670960f13234971f48e8837a3935495a0d69a026
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/549500
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
(cherry picked from commit 147f7790602ab5d43bfe19f759074258ca7e975b)
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/550068
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
---
.../sqlite/src/amalgamation/sqlite3.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/chromium/third_party/sqlite/src/amalgamation/sqlite3.c b/chromium/third_party/sqlite/src/amalgamation/sqlite3.c
index b353aa88348..a0feb5d200c 100644
--- src/3rdparty/chromium/third_party/sqlite/src/amalgamation/sqlite3.c
+++ src/3rdparty/chromium/third_party/sqlite/src/amalgamation/sqlite3.c
@@ -218682,15 +218682,19 @@ static int sessionReadRecord(
}
}
if( eType==SQLITE_INTEGER || eType==SQLITE_FLOAT ){
- sqlite3_int64 v = sessionGetI64(aVal);
- if( eType==SQLITE_INTEGER ){
- sqlite3VdbeMemSetInt64(apOut[i], v);
+ if( (pIn->nData-pIn->iNext)<8 ){
+ rc = SQLITE_CORRUPT_BKPT;
}else{
- double d;
- memcpy(&d, &v, 8);
- sqlite3VdbeMemSetDouble(apOut[i], d);
+ sqlite3_int64 v = sessionGetI64(aVal);
+ if( eType==SQLITE_INTEGER ){
+ sqlite3VdbeMemSetInt64(apOut[i], v);
+ }else{
+ double d;
+ memcpy(&d, &v, 8);
+ sqlite3VdbeMemSetDouble(apOut[i], d);
+ }
+ pIn->iNext += 8;
}
- pIn->iNext += 8;
}
}
}
From a907a6cbc18a04d791b0c97918a558d49f696cd4 Mon Sep 17 00:00:00 2001
From: Shu-yu Guo <syg@chromium.org>
Date: Thu, 7 Mar 2024 14:55:28 -0800
Subject: [PATCH] [Backport] CVE-2024-2625: Object lifecycle issue in V8
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Manual cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/v8/v8/+/5350482:
Fix home object proxy to work off-thread
Because the home object has special scope lookup rules due to class
heritage position, VariableProxies of the home object are currently
directly created on the correct scope during parsing. However, during
off-thread parsing the main thread is parked, and the correct scope
may try to dereference a main-thread Handle.
This CL moves the logic into ResolveVariable instead, which happens
during postprocessing, with the main thread unparked.
Fixed: chromium:327740539
Change-Id: I3a123d5e37b6764067e58255dd5a67c07e648d02
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5350482
Reviewed-by: Marja Hölttä <marja@chromium.org>
Commit-Queue: Marja Hölttä <marja@chromium.org>
Auto-Submit: Shu-yu Guo <syg@chromium.org>
Cr-Commit-Position: refs/heads/main@{#92722}
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/551089
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
---
chromium/v8/src/ast/ast.h | 10 +++-
chromium/v8/src/ast/scopes.cc | 78 ++++++++++++---------------
chromium/v8/src/ast/scopes.h | 7 +--
chromium/v8/src/parsing/parser-base.h | 4 +-
chromium/v8/src/parsing/parser.cc | 11 ++--
chromium/v8/src/parsing/parser.h | 2 +-
chromium/v8/src/parsing/preparser.h | 3 +-
7 files changed, 52 insertions(+), 63 deletions(-)
diff --git a/chromium/v8/src/ast/ast.h b/chromium/v8/src/ast/ast.h
index cf9d52eeed6..7bb30723607 100644
--- src/3rdparty/chromium/v8/src/ast/ast.h
+++ src/3rdparty/chromium/v8/src/ast/ast.h
@@ -1534,6 +1534,12 @@ class VariableProxy final : public Expression {
bit_field_ = IsRemovedFromUnresolvedField::update(bit_field_, true);
}
+ bool is_home_object() const { return IsHomeObjectField::decode(bit_field_); }
+
+ void set_is_home_object() {
+ bit_field_ = IsHomeObjectField::update(bit_field_, true);
+ }
+
// Provides filtered access to the unresolved variable proxy threaded list.
struct UnresolvedNext {
static VariableProxy** filter(VariableProxy** t) {
@@ -1565,6 +1571,7 @@ class VariableProxy final : public Expression {
bit_field_ |= IsAssignedField::encode(false) |
IsResolvedField::encode(false) |
IsRemovedFromUnresolvedField::encode(false) |
+ IsHomeObjectField::encode(false) |
HoleCheckModeField::encode(HoleCheckMode::kElided);
}
@@ -1574,7 +1581,8 @@ class VariableProxy final : public Expression {
using IsResolvedField = IsAssignedField::Next<bool, 1>;
using IsRemovedFromUnresolvedField = IsResolvedField::Next<bool, 1>;
using IsNewTargetField = IsRemovedFromUnresolvedField::Next<bool, 1>;
- using HoleCheckModeField = IsNewTargetField::Next<HoleCheckMode, 1>;
+ using IsHomeObjectField = IsNewTargetField::Next<bool, 1>;
+ using HoleCheckModeField = IsHomeObjectField::Next<HoleCheckMode, 1>;
union {
const AstRawString* raw_name_; // if !is_resolved_
diff --git a/chromium/v8/src/ast/scopes.cc b/chromium/v8/src/ast/scopes.cc
index 003bd0f2736..34415941556 100644
--- src/3rdparty/chromium/v8/src/ast/scopes.cc
+++ src/3rdparty/chromium/v8/src/ast/scopes.cc
@@ -491,7 +491,6 @@ Scope* Scope::DeserializeScopeChain(IsolateT* isolate, Zone* zone,
if (cache_scope_found) {
outer_scope->set_deserialized_scope_uses_external_cache();
} else {
- DCHECK(!cache_scope_found);
cache_scope_found =
outer_scope->is_declaration_scope() && !outer_scope->is_eval_scope();
}
@@ -970,9 +969,14 @@ Variable* Scope::LookupInScopeInfo(const AstRawString* name, Scope* cache) {
DCHECK(!cache->deserialized_scope_uses_external_cache());
// The case where where the cache can be another scope is when the cache scope
// is the last scope that doesn't use an external cache.
+ //
+ // The one exception to this is when looking up the home object, which may
+ // skip multiple scopes that don't use an external cache (e.g., several arrow
+ // functions).
DCHECK_IMPLIES(
cache != this,
- cache->outer_scope()->deserialized_scope_uses_external_cache());
+ cache->outer_scope()->deserialized_scope_uses_external_cache() ||
+ cache->GetHomeObjectScope() == this);
DCHECK_NULL(cache->variables_.Lookup(name));
DisallowGarbageCollection no_gc;
@@ -2282,7 +2286,33 @@ Variable* Scope::LookupSloppyEval(VariableProxy* proxy, Scope* scope,
void Scope::ResolveVariable(VariableProxy* proxy) {
DCHECK(!proxy->is_resolved());
- Variable* var = Lookup<kParsedScope>(proxy, this, nullptr);
+ Variable* var;
+ if (V8_UNLIKELY(proxy->is_home_object())) {
+ // VariableProxies of the home object cannot be resolved like a normal
+ // variable. Consider the case of a super.property usage in heritage
+ // position:
+ //
+ // class C extends super.foo { m() { super.bar(); } }
+ //
+ // The super.foo property access is logically nested under C's class scope,
+ // which also has a home object due to its own method m's usage of
+ // super.bar(). However, super.foo must resolve super in C's outer scope.
+ //
+ // Because of the above, start resolving home objects directly at the home
+ // object scope instead of the current scope.
+ Scope* scope = GetDeclarationScope()->GetHomeObjectScope();
+ DCHECK_NOT_NULL(scope);
+ if (scope->scope_info_.is_null()) {
+ var = Lookup<kParsedScope>(proxy, scope, nullptr);
+ } else {
+ Scope* entry_cache = scope->deserialized_scope_uses_external_cache()
+ ? GetNonEvalDeclarationScope()
+ : scope;
+ var = Lookup<kDeserializedScope>(proxy, scope, nullptr, entry_cache);
+ }
+ } else {
+ var = Lookup<kParsedScope>(proxy, this, nullptr);
+ }
DCHECK_NOT_NULL(var);
ResolveTo(proxy, var);
}
@@ -2752,48 +2782,6 @@ int Scope::ContextLocalCount() const {
(is_function_var_in_context ? 1 : 0);
}
-VariableProxy* Scope::NewHomeObjectVariableProxy(AstNodeFactory* factory,
- const AstRawString* name,
- int start_pos) {
- // VariableProxies of the home object cannot be resolved like a normal
- // variable. Consider the case of a super.property usage in heritage position:
- //
- // class C extends super.foo { m() { super.bar(); } }
- //
- // The super.foo property access is logically nested under C's class scope,
- // which also has a home object due to its own method m's usage of
- // super.bar(). However, super.foo must resolve super in C's outer scope.
- //
- // Because of the above, home object VariableProxies are always made directly
- // on the Scope that needs the home object instead of the innermost scope.
- DCHECK(needs_home_object());
- if (!scope_info_.is_null()) {
- // This is a lazy compile, so the home object's context slot is already
- // known.
- Variable* home_object = variables_.Lookup(name);
- if (home_object == nullptr) {
- VariableLookupResult lookup_result;
- int index = scope_info_->ContextSlotIndex(name->string(), &lookup_result);
- DCHECK_GE(index, 0);
- bool was_added;
- home_object = variables_.Declare(zone(), this, name, lookup_result.mode,
- NORMAL_VARIABLE, lookup_result.init_flag,
- lookup_result.maybe_assigned_flag,
- IsStaticFlag::kNotStatic, &was_added);
- DCHECK(was_added);
- home_object->AllocateTo(VariableLocation::CONTEXT, index);
- }
- return factory->NewVariableProxy(home_object, start_pos);
- }
- // This is not a lazy compile. Add the unresolved home object VariableProxy to
- // the unresolved list of the home object scope, which is not necessarily the
- // innermost scope.
- VariableProxy* proxy =
- factory->NewVariableProxy(name, NORMAL_VARIABLE, start_pos);
- AddUnresolved(proxy);
- return proxy;
-}
-
bool IsComplementaryAccessorPair(VariableMode a, VariableMode b) {
switch (a) {
case VariableMode::kPrivateGetterOnly:
diff --git a/chromium/v8/src/ast/scopes.h b/chromium/v8/src/ast/scopes.h
index b4c2e8b2136..751aaee3d11 100644
--- src/3rdparty/chromium/v8/src/ast/scopes.h
+++ src/3rdparty/chromium/v8/src/ast/scopes.h
@@ -603,10 +603,6 @@ class V8_EXPORT_PRIVATE Scope : public NON_EXPORTED_BASE(ZoneObject) {
needs_home_object_ = true;
}
- VariableProxy* NewHomeObjectVariableProxy(AstNodeFactory* factory,
- const AstRawString* name,
- int start_pos);
-
bool RemoveInnerScope(Scope* inner_scope) {
DCHECK_NOT_NULL(inner_scope);
if (inner_scope == inner_scope_) {
@@ -865,7 +861,7 @@ class V8_EXPORT_PRIVATE DeclarationScope : public Scope {
FunctionKind function_kind() const { return function_kind_; }
// Inform the scope that the corresponding code uses "super".
- Scope* RecordSuperPropertyUsage() {
+ void RecordSuperPropertyUsage() {
DCHECK(IsConciseMethod(function_kind()) ||
IsAccessorFunction(function_kind()) ||
IsClassConstructor(function_kind()));
@@ -873,7 +869,6 @@ class V8_EXPORT_PRIVATE DeclarationScope : public Scope {
Scope* home_object_scope = GetHomeObjectScope();
DCHECK_NOT_NULL(home_object_scope);
home_object_scope->set_needs_home_object();
- return home_object_scope;
}
bool uses_super_property() const { return uses_super_property_; }
diff --git a/chromium/v8/src/parsing/parser-base.h b/chromium/v8/src/parsing/parser-base.h
index 66e531dfe78..714406f6aa5 100644
--- src/3rdparty/chromium/v8/src/parsing/parser-base.h
+++ src/3rdparty/chromium/v8/src/parsing/parser-base.h
@@ -3800,9 +3800,9 @@ ParserBase<Impl>::ParseSuperExpression() {
impl()->ReportMessage(MessageTemplate::kOptionalChainingNoSuper);
return impl()->FailureExpression();
}
- Scope* home_object_scope = scope->RecordSuperPropertyUsage();
+ scope->RecordSuperPropertyUsage();
UseThis();
- return impl()->NewSuperPropertyReference(home_object_scope, pos);
+ return impl()->NewSuperPropertyReference(pos);
}
// super() is only allowed in derived constructor. new super() is never
// allowed; it's reported as an error by
diff --git a/chromium/v8/src/parsing/parser.cc b/chromium/v8/src/parsing/parser.cc
index da16f85234d..5e4b2d0461a 100644
--- src/3rdparty/chromium/v8/src/parsing/parser.cc
+++ src/3rdparty/chromium/v8/src/parsing/parser.cc
@@ -300,18 +300,17 @@ Expression* Parser::NewThrowError(Runtime::FunctionId id,
return factory()->NewThrow(call_constructor, pos);
}
-Expression* Parser::NewSuperPropertyReference(Scope* home_object_scope,
- int pos) {
+Expression* Parser::NewSuperPropertyReference(int pos) {
const AstRawString* home_object_name;
if (IsStatic(scope()->GetReceiverScope()->function_kind())) {
home_object_name = ast_value_factory_->dot_static_home_object_string();
} else {
home_object_name = ast_value_factory_->dot_home_object_string();
}
- return factory()->NewSuperPropertyReference(
- home_object_scope->NewHomeObjectVariableProxy(factory(), home_object_name,
- pos),
- pos);
+
+ VariableProxy* proxy = NewUnresolved(home_object_name, pos);
+ proxy->set_is_home_object();
+ return factory()->NewSuperPropertyReference(proxy, pos);
}
Expression* Parser::NewSuperCallReference(int pos) {
diff --git a/chromium/v8/src/parsing/parser.h b/chromium/v8/src/parsing/parser.h
index 8aede5d6a2c..0e92f0350b5 100644
--- src/3rdparty/chromium/v8/src/parsing/parser.h
+++ src/3rdparty/chromium/v8/src/parsing/parser.h
@@ -798,7 +798,7 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
return factory()->NewThisExpression(pos);
}
- Expression* NewSuperPropertyReference(Scope* home_object_scope, int pos);
+ Expression* NewSuperPropertyReference(int pos);
Expression* NewSuperCallReference(int pos);
Expression* NewTargetExpression(int pos);
Expression* ImportMetaExpression(int pos);
diff --git a/chromium/v8/src/parsing/preparser.h b/chromium/v8/src/parsing/preparser.h
index 6c4996bd06b..2ca6b9ac407 100644
--- src/3rdparty/chromium/v8/src/parsing/preparser.h
+++ src/3rdparty/chromium/v8/src/parsing/preparser.h
@@ -1536,8 +1536,7 @@ class PreParser : public ParserBase<PreParser> {
return PreParserExpression::This();
}
- V8_INLINE PreParserExpression
- NewSuperPropertyReference(Scope* home_object_scope, int pos) {
+ V8_INLINE PreParserExpression NewSuperPropertyReference(int pos) {
return PreParserExpression::Default();
}
From c4661dc646e45d06961cda71d00814ce878dbd97 Mon Sep 17 00:00:00 2001
From: Shahbaz Youssefi <syoussefi@google.com>
Date: Fri, 19 Jan 2024 15:36:25 -0500
Subject: [PATCH] [Backport] CVE-2024-2626: Out of bounds read in Swiftshader
(1/2)
Cherry-pick of patch originally reviewed on
https://swiftshader-review.googlesource.com/c/SwiftShader/+/72948:
Clamp LOD during image Fetch for robustness
Bug: chromium:1504556
Change-Id: Ie110fe4e1b065a815c09986ab91b1336ef4761ad
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/72948
Presubmit-Ready: Shahbaz Youssefi <syoussefi@google.com>
Kokoro-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/551090
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
---
.../third_party/swiftshader/src/Pipeline/SamplerCore.cpp | 1 +
.../swiftshader/src/Pipeline/SpirvShaderSampling.cpp | 5 +++++
chromium/third_party/swiftshader/src/Vulkan/VkImageView.cpp | 6 ++++--
chromium/third_party/swiftshader/src/Vulkan/VkImageView.hpp | 3 +++
4 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/chromium/third_party/swiftshader/src/Pipeline/SamplerCore.cpp b/chromium/third_party/swiftshader/src/Pipeline/SamplerCore.cpp
index 403ed3bdd560..d62936273fa3 100644
--- src/3rdparty/chromium/third_party/swiftshader/src/Pipeline/SamplerCore.cpp
+++ src/3rdparty/chromium/third_party/swiftshader/src/Pipeline/SamplerCore.cpp
@@ -133,6 +133,7 @@ Vector4f SamplerCore::sampleTexture128(Pointer<Byte> &texture, Float4 uvwa[4], c
{
// TODO: Eliminate int-float-int conversion.
lod = Float(As<Int>(lodOrBias));
+ lod = Min(lod, state.maxLod);
}
else if(function == Base || function == Gather)
{
diff --git a/chromium/third_party/swiftshader/src/Pipeline/SpirvShaderSampling.cpp b/chromium/third_party/swiftshader/src/Pipeline/SpirvShaderSampling.cpp
index 5225a79f3ba6..777f73e43786 100644
--- src/3rdparty/chromium/third_party/swiftshader/src/Pipeline/SpirvShaderSampling.cpp
+++ src/3rdparty/chromium/third_party/swiftshader/src/Pipeline/SpirvShaderSampling.cpp
@@ -108,6 +108,11 @@ SpirvEmitter::ImageSampler *SpirvEmitter::getImageSampler(const vk::Device *devi
samplerState.minLod = 0.0f;
samplerState.maxLod = 0.0f;
}
+ // Otherwise make sure LOD is clamped for robustness
+ else
+ {
+ samplerState.maxLod = imageViewState.maxLod;
+ }
}
else if(samplerMethod == Write)
{
diff --git a/chromium/third_party/swiftshader/src/Vulkan/VkImageView.cpp b/chromium/third_party/swiftshader/src/Vulkan/VkImageView.cpp
index 511c02cbbed9..26b69aef79b3 100644
--- src/3rdparty/chromium/third_party/swiftshader/src/Vulkan/VkImageView.cpp
+++ src/3rdparty/chromium/third_party/swiftshader/src/Vulkan/VkImageView.cpp
@@ -89,13 +89,13 @@ Identifier::Identifier(const VkImageViewCreateInfo *pCreateInfo)
const Image *sampledImage = image->getSampledImage(viewFormat);
vk::Format samplingFormat = (image == sampledImage) ? viewFormat : sampledImage->getFormat().getAspectFormat(subresource.aspectMask);
- pack({ pCreateInfo->viewType, samplingFormat, ResolveComponentMapping(pCreateInfo->components, viewFormat), subresource.levelCount <= 1u });
+ pack({ pCreateInfo->viewType, samplingFormat, ResolveComponentMapping(pCreateInfo->components, viewFormat), static_cast<uint8_t>(subresource.baseMipLevel + subresource.levelCount), subresource.levelCount <= 1u });
}
Identifier::Identifier(VkFormat bufferFormat)
{
constexpr VkComponentMapping identityMapping = { VK_COMPONENT_SWIZZLE_R, VK_COMPONENT_SWIZZLE_G, VK_COMPONENT_SWIZZLE_B, VK_COMPONENT_SWIZZLE_A };
- pack({ VK_IMAGE_VIEW_TYPE_1D, bufferFormat, ResolveComponentMapping(identityMapping, bufferFormat), true });
+ pack({ VK_IMAGE_VIEW_TYPE_1D, bufferFormat, ResolveComponentMapping(identityMapping, bufferFormat), 1, true });
}
void Identifier::pack(const State &state)
@@ -106,6 +106,7 @@ void Identifier::pack(const State &state)
g = static_cast<uint32_t>(state.mapping.g);
b = static_cast<uint32_t>(state.mapping.b);
a = static_cast<uint32_t>(state.mapping.a);
+ maxLod = state.maxLod;
singleMipLevel = state.singleMipLevel;
}
@@ -117,6 +118,7 @@ Identifier::State Identifier::getState() const
static_cast<VkComponentSwizzle>(g),
static_cast<VkComponentSwizzle>(b),
static_cast<VkComponentSwizzle>(a) },
+ static_cast<uint8_t>(maxLod),
static_cast<bool>(singleMipLevel) };
}
diff --git a/chromium/third_party/swiftshader/src/Vulkan/VkImageView.hpp b/chromium/third_party/swiftshader/src/Vulkan/VkImageView.hpp
index bf4d666a425d..5acb89639c12 100644
--- src/3rdparty/chromium/third_party/swiftshader/src/Vulkan/VkImageView.hpp
+++ src/3rdparty/chromium/third_party/swiftshader/src/Vulkan/VkImageView.hpp
@@ -53,6 +53,7 @@ union Identifier
VkImageViewType imageViewType;
VkFormat format;
VkComponentMapping mapping;
+ uint8_t maxLod;
bool singleMipLevel;
};
State getState() const;
@@ -61,6 +62,7 @@ union Identifier
void pack(const State &data);
// Identifier is a union of this struct and the integer below.
+ static_assert(sw::MIPMAP_LEVELS <= 15);
struct
{
uint32_t imageViewType : 3;
@@ -69,6 +71,7 @@ union Identifier
uint32_t g : 3;
uint32_t b : 3;
uint32_t a : 3;
+ uint32_t maxLod : 4;
uint32_t singleMipLevel : 1;
};
From a58826b55d30045bed8793dbcd20dc32a73716e2 Mon Sep 17 00:00:00 2001
From: Shahbaz Youssefi <syoussefi@google.com>
Date: Mon, 22 Jan 2024 09:49:16 -0500
Subject: [PATCH] [Backport] CVE-2024-2626: Out of bounds read in Swiftshader
(2/2)
Cherry-pick of patch originally reviewed on
https://swiftshader-review.googlesource.com/c/SwiftShader/+/72968:
Clamp min LOD during image Fetch for robustness
The previous change clamped max LOD only, but min LOD also needs
clamping because texelFetch takes an `int` as LOD instead of `uint`.
Bug: chromium:1504556
Change-Id: Ibae8250a877b3e04b71fac45a40b77c78756d6c8
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/72968
Kokoro-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Shahbaz Youssefi <syoussefi@google.com>
Presubmit-Ready: Shahbaz Youssefi <syoussefi@google.com>
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/551091
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
---
.../third_party/swiftshader/src/Pipeline/SamplerCore.cpp | 1 +
.../swiftshader/src/Pipeline/SpirvShaderSampling.cpp | 1 +
.../third_party/swiftshader/src/Vulkan/VkImageView.cpp | 8 ++++++--
.../third_party/swiftshader/src/Vulkan/VkImageView.hpp | 2 ++
4 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/chromium/third_party/swiftshader/src/Pipeline/SamplerCore.cpp b/chromium/third_party/swiftshader/src/Pipeline/SamplerCore.cpp
index d62936273fa..ab55c036a3f 100644
--- src/3rdparty/chromium/third_party/swiftshader/src/Pipeline/SamplerCore.cpp
+++ src/3rdparty/chromium/third_party/swiftshader/src/Pipeline/SamplerCore.cpp
@@ -133,6 +133,7 @@ Vector4f SamplerCore::sampleTexture128(Pointer<Byte> &texture, Float4 uvwa[4], c
{
// TODO: Eliminate int-float-int conversion.
lod = Float(As<Int>(lodOrBias));
+ lod = Max(lod, state.minLod);
lod = Min(lod, state.maxLod);
}
else if(function == Base || function == Gather)
diff --git a/chromium/third_party/swiftshader/src/Pipeline/SpirvShaderSampling.cpp b/chromium/third_party/swiftshader/src/Pipeline/SpirvShaderSampling.cpp
index 777f73e4378..fa88a192ab5 100644
--- src/3rdparty/chromium/third_party/swiftshader/src/Pipeline/SpirvShaderSampling.cpp
+++ src/3rdparty/chromium/third_party/swiftshader/src/Pipeline/SpirvShaderSampling.cpp
@@ -111,6 +111,7 @@ SpirvEmitter::ImageSampler *SpirvEmitter::getImageSampler(const vk::Device *devi
// Otherwise make sure LOD is clamped for robustness
else
{
+ samplerState.minLod = imageViewState.minLod;
samplerState.maxLod = imageViewState.maxLod;
}
}
diff --git a/chromium/third_party/swiftshader/src/Vulkan/VkImageView.cpp b/chromium/third_party/swiftshader/src/Vulkan/VkImageView.cpp
index 26b69aef79b..1b25544a57b 100644
--- src/3rdparty/chromium/third_party/swiftshader/src/Vulkan/VkImageView.cpp
+++ src/3rdparty/chromium/third_party/swiftshader/src/Vulkan/VkImageView.cpp
@@ -89,13 +89,15 @@ Identifier::Identifier(const VkImageViewCreateInfo *pCreateInfo)
const Image *sampledImage = image->getSampledImage(viewFormat);
vk::Format samplingFormat = (image == sampledImage) ? viewFormat : sampledImage->getFormat().getAspectFormat(subresource.aspectMask);
- pack({ pCreateInfo->viewType, samplingFormat, ResolveComponentMapping(pCreateInfo->components, viewFormat), static_cast<uint8_t>(subresource.baseMipLevel + subresource.levelCount), subresource.levelCount <= 1u });
+ pack({ pCreateInfo->viewType, samplingFormat, ResolveComponentMapping(pCreateInfo->components, viewFormat),
+ static_cast<uint8_t>(subresource.baseMipLevel),
+ static_cast<uint8_t>(subresource.baseMipLevel + subresource.levelCount), subresource.levelCount <= 1u });
}
Identifier::Identifier(VkFormat bufferFormat)
{
constexpr VkComponentMapping identityMapping = { VK_COMPONENT_SWIZZLE_R, VK_COMPONENT_SWIZZLE_G, VK_COMPONENT_SWIZZLE_B, VK_COMPONENT_SWIZZLE_A };
- pack({ VK_IMAGE_VIEW_TYPE_1D, bufferFormat, ResolveComponentMapping(identityMapping, bufferFormat), 1, true });
+ pack({ VK_IMAGE_VIEW_TYPE_1D, bufferFormat, ResolveComponentMapping(identityMapping, bufferFormat), 0, 1, true });
}
void Identifier::pack(const State &state)
@@ -106,6 +108,7 @@ void Identifier::pack(const State &state)
g = static_cast<uint32_t>(state.mapping.g);
b = static_cast<uint32_t>(state.mapping.b);
a = static_cast<uint32_t>(state.mapping.a);
+ minLod = state.minLod;
maxLod = state.maxLod;
singleMipLevel = state.singleMipLevel;
}
@@ -118,6 +121,7 @@ Identifier::State Identifier::getState() const
static_cast<VkComponentSwizzle>(g),
static_cast<VkComponentSwizzle>(b),
static_cast<VkComponentSwizzle>(a) },
+ static_cast<uint8_t>(minLod),
static_cast<uint8_t>(maxLod),
static_cast<bool>(singleMipLevel) };
}
diff --git a/chromium/third_party/swiftshader/src/Vulkan/VkImageView.hpp b/chromium/third_party/swiftshader/src/Vulkan/VkImageView.hpp
index 5acb89639c1..25feebc9e7e 100644
--- src/3rdparty/chromium/third_party/swiftshader/src/Vulkan/VkImageView.hpp
+++ src/3rdparty/chromium/third_party/swiftshader/src/Vulkan/VkImageView.hpp
@@ -53,6 +53,7 @@ union Identifier
VkImageViewType imageViewType;
VkFormat format;
VkComponentMapping mapping;
+ uint8_t minLod;
uint8_t maxLod;
bool singleMipLevel;
};
@@ -71,6 +72,7 @@ union Identifier
uint32_t g : 3;
uint32_t b : 3;
uint32_t a : 3;
+ uint32_t minLod : 4;
uint32_t maxLod : 4;
uint32_t singleMipLevel : 1;
};
From 336b5c4a31f9f976434adb2ecf1697c764f097cf Mon Sep 17 00:00:00 2001
From: rajendrant <rajendrant@chromium.org>
Date: Fri, 9 Feb 2024 17:29:51 +0000
Subject: [PATCH] [Backport] Security bug 40066823
Manual partial backport of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/5279279:
Remove holding local_state in model store
Avoids holding on to local_state in model store, by getting the local state when needed from chrome/. This delinks model store lifetime with local state lifetime.
Change-Id: Ifb036b43b8394202683d4ae1131ff1eae780fc17
Fixed: 40066823
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5279279
Reviewed-by: Sophie Chang <sophiechang@chromium.org>
Commit-Queue: Raj T <rajendrant@chromium.org>
Reviewed-by: Trevor Perrier <perrier@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1258580}
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/551092
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
---
.../core/prediction_model_store.cc | 61 ++++++-------------
.../core/prediction_model_store.h | 28 +++------
2 files changed, 28 insertions(+), 61 deletions(-)
diff --git a/chromium/components/optimization_guide/core/prediction_model_store.cc b/chromium/components/optimization_guide/core/prediction_model_store.cc
index d2252ea5cc4..a399ac06d7f 100644
--- src/3rdparty/chromium/components/optimization_guide/core/prediction_model_store.cc
+++ src/3rdparty/chromium/components/optimization_guide/core/prediction_model_store.cc
@@ -155,12 +155,6 @@ void RecordModelStorageMetrics(const base::FilePath& base_store_dir) {
} // namespace
-// static
-PredictionModelStore* PredictionModelStore::GetInstance() {
- static base::NoDestructor<PredictionModelStore> model_store;
- return model_store.get();
-}
-
PredictionModelStore::PredictionModelStore()
: background_task_runner_(base::ThreadPool::CreateSequencedTaskRunner(
{base::MayBlock(), base::TaskPriority::BEST_EFFORT})) {
@@ -169,19 +163,14 @@ PredictionModelStore::PredictionModelStore()
PredictionModelStore::~PredictionModelStore() = default;
-void PredictionModelStore::Initialize(PrefService* local_state,
- const base::FilePath& base_store_dir) {
+void PredictionModelStore::Initialize(const base::FilePath& base_store_dir) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
- DCHECK(local_state);
DCHECK(!base_store_dir.empty());
// Should not be initialized already.
- DCHECK(!local_state_);
DCHECK(base_store_dir_.empty());
- local_state_ = local_state;
base_store_dir_ = base_store_dir;
-
PurgeInactiveModels();
// Clean up any model files that were slated for deletion in previous
@@ -189,29 +178,19 @@ void PredictionModelStore::Initialize(PrefService* local_state,
CleanUpOldModelFiles();
background_task_runner_->PostTask(
- FROM_HERE,
- base::BindOnce(&RemoveInvalidModelDirs, base_store_dir_,
- ModelStoreMetadataEntry::GetValidModelDirs(local_state_)));
+ FROM_HERE, base::BindOnce(&RemoveInvalidModelDirs, base_store_dir_,
+ ModelStoreMetadataEntry::GetValidModelDirs(
+ GetLocalState())));
background_task_runner_->PostTask(
FROM_HERE, base::BindOnce(&RecordModelStorageMetrics, base_store_dir_));
}
-// static
-std::unique_ptr<PredictionModelStore>
-PredictionModelStore::CreatePredictionModelStoreForTesting(
- PrefService* local_state,
- const base::FilePath& base_store_dir) {
- auto store = base::WrapUnique(new PredictionModelStore());
- store->Initialize(local_state, base_store_dir);
- return store;
-}
-
bool PredictionModelStore::HasModel(
proto::OptimizationTarget optimization_target,
const proto::ModelCacheKey& model_cache_key) const {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
auto metadata = ModelStoreMetadataEntry::GetModelMetadataEntryIfExists(
- local_state_, optimization_target, model_cache_key);
+ GetLocalState(), optimization_target, model_cache_key);
if (!metadata) {
return false;
}
@@ -226,7 +205,7 @@ bool PredictionModelStore::HasModelWithVersion(
int64_t version) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
auto metadata = ModelStoreMetadataEntry::GetModelMetadataEntryIfExists(
- local_state_, optimization_target, model_cache_key);
+ GetLocalState(), optimization_target, model_cache_key);
if (!metadata) {
return false;
}
@@ -251,7 +230,7 @@ void PredictionModelStore::LoadModel(
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
auto metadata = ModelStoreMetadataEntry::GetModelMetadataEntryIfExists(
- local_state_, optimization_target, model_cache_key);
+ GetLocalState(), optimization_target, model_cache_key);
if (!metadata) {
std::move(callback).Run(nullptr);
return;
@@ -334,7 +313,7 @@ void PredictionModelStore::UpdateMetadataForExistingModel(
if (!HasModel(optimization_target, model_cache_key))
return;
- ModelStoreMetadataEntryUpdater metadata(local_state_, optimization_target,
+ ModelStoreMetadataEntryUpdater metadata(GetLocalState(), optimization_target,
model_cache_key);
DCHECK(!metadata.GetModelBaseDir()->IsAbsolute());
metadata.SetVersion(model_info.version());
@@ -357,7 +336,7 @@ void PredictionModelStore::UpdateModel(
DCHECK_EQ(optimization_target, model_info.optimization_target());
DCHECK(base_store_dir_.IsParent(base_model_dir));
- ModelStoreMetadataEntryUpdater metadata(local_state_, optimization_target,
+ ModelStoreMetadataEntryUpdater metadata(GetLocalState(), optimization_target,
model_cache_key);
metadata.SetVersion(model_info.version());
metadata.SetExpiryTime(
@@ -420,7 +399,7 @@ void PredictionModelStore::UpdateModelCacheKeyMapping(
const proto::ModelCacheKey& server_model_cache_key) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
ModelStoreMetadataEntryUpdater::UpdateModelCacheKeyMapping(
- local_state_, optimization_target, client_model_cache_key,
+ GetLocalState(), optimization_target, client_model_cache_key,
server_model_cache_key);
}
@@ -429,13 +408,13 @@ void PredictionModelStore::RemoveModel(
const proto::ModelCacheKey& model_cache_key,
PredictionModelStoreModelRemovalReason model_remove_reason) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
- if (!local_state_) {
+ if (!GetLocalState()) {
return;
}
RecordPredictionModelStoreModelRemovalVersionHistogram(optimization_target,
model_remove_reason);
- ModelStoreMetadataEntryUpdater metadata(local_state_, optimization_target,
+ ModelStoreMetadataEntryUpdater metadata(GetLocalState(), optimization_target,
model_cache_key);
auto base_model_dir = metadata.GetModelBaseDir();
if (base_model_dir) {
@@ -458,16 +437,17 @@ void PredictionModelStore::ScheduleModelDirRemoval(
base_model_dir.IsAbsolute()
? ConvertToRelativePath(base_store_dir_, base_model_dir)
: base_model_dir;
- ScopedDictPrefUpdate pref_update(local_state_,
+ ScopedDictPrefUpdate pref_update(GetLocalState(),
prefs::localstate::kStoreFilePathsToDelete);
pref_update->Set(FilePathToString(relative_model_dir), true);
}
void PredictionModelStore::PurgeInactiveModels() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
- DCHECK(local_state_);
+ DCHECK(GetLocalState());
for (const auto& expired_model_dir :
- ModelStoreMetadataEntryUpdater::PurgeAllInactiveMetadata(local_state_)) {
+ ModelStoreMetadataEntryUpdater::PurgeAllInactiveMetadata(
+ GetLocalState())) {
// Backward compatibility: Model dirs were absolute in the earlier versions,
// and it was only in experiment. The latest versions use relative paths.
DCHECK(!expired_model_dir.IsAbsolute() ||
@@ -485,9 +465,9 @@ void PredictionModelStore::PurgeInactiveModels() {
void PredictionModelStore::CleanUpOldModelFiles() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
- DCHECK(local_state_);
+ DCHECK(GetLocalState());
for (const auto entry :
- local_state_->GetDict(prefs::localstate::kStoreFilePathsToDelete)) {
+ GetLocalState()->GetDict(prefs::localstate::kStoreFilePathsToDelete)) {
// Backward compatibility: Model dirs were absolute in the earlier versions.
// The latest versions use relative paths.
auto path_to_delete = StringToFilePath(entry.first);
@@ -508,13 +488,13 @@ void PredictionModelStore::CleanUpOldModelFiles() {
void PredictionModelStore::OnFilePathDeleted(const std::string& path_to_delete,
bool success) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
- DCHECK(local_state_);
+ DCHECK(GetLocalState());
if (!success) {
// Try to delete again later.
return;
}
- ScopedDictPrefUpdate pref_update(local_state_,
+ ScopedDictPrefUpdate pref_update(GetLocalState(),
prefs::localstate::kStoreFilePathsToDelete);
pref_update->Remove(path_to_delete);
}
@@ -527,7 +507,6 @@ base::FilePath PredictionModelStore::GetBaseStoreDirForTesting() const {
void PredictionModelStore::ResetForTesting() {
DETACH_FROM_SEQUENCE(sequence_checker_);
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
- local_state_ = nullptr;
base_store_dir_ = base::FilePath();
background_task_runner_ = base::ThreadPool::CreateSequencedTaskRunner(
{base::MayBlock(), base::TaskPriority::BEST_EFFORT});
diff --git a/chromium/components/optimization_guide/core/prediction_model_store.h b/chromium/components/optimization_guide/core/prediction_model_store.h
index eb39780fd37..720e50dc128 100644
--- src/3rdparty/chromium/components/optimization_guide/core/prediction_model_store.h
+++ src/3rdparty/chromium/components/optimization_guide/core/prediction_model_store.h
@@ -8,7 +8,6 @@
#include "base/files/file_path.h"
#include "base/memory/scoped_refptr.h"
#include "base/memory/weak_ptr.h"
-#include "base/no_destructor.h"
#include "base/sequence_checker.h"
#include "base/task/sequenced_task_runner.h"
#include "base/values.h"
@@ -34,21 +33,15 @@ class PredictionModelStore {
using PredictionModelLoadedCallback =
base::OnceCallback<void(std::unique_ptr<proto::PredictionModel>)>;
- // Returns the singleton model store.
- static PredictionModelStore* GetInstance();
-
- static std::unique_ptr<PredictionModelStore>
- CreatePredictionModelStoreForTesting(PrefService* local_state,
- const base::FilePath& base_store_dir);
+ PredictionModelStore();
- // Initializes the model store with |local_state| and the |base_store_dir|.
- // Model store will be usable only after it is initialized.
- void Initialize(PrefService* local_state,
- const base::FilePath& base_store_dir);
+ // Initializes the model store with |base_store_dir|. Model store will be
+ // usable only after it is initialized.
+ void Initialize(const base::FilePath& base_store_dir);
PredictionModelStore(const PredictionModelStore&) = delete;
PredictionModelStore& operator=(const PredictionModelStore&) = delete;
- ~PredictionModelStore();
+ virtual ~PredictionModelStore();
// Initializes the model store with |local_state| and the |base_store_dir|, if
// initialization hasn't happened already. Model store will be usable only
@@ -111,6 +104,9 @@ class PredictionModelStore {
const proto::ModelCacheKey& model_cache_key,
PredictionModelStoreModelRemovalReason model_removal_reason);
+ // Returns the local state that stores the prefs across all profiles.
+ virtual PrefService* GetLocalState() const = 0;
+
base::FilePath GetBaseStoreDirForTesting() const;
// Allows tests to reset the store for subsequent tests since the store is a
@@ -118,11 +114,8 @@ class PredictionModelStore {
void ResetForTesting();
private:
- friend base::NoDestructor<PredictionModelStore>;
friend class PredictionModelStoreBrowserTestBase;
- PredictionModelStore();
-
// Loads the model and verifies if the model files exist and returns the
// model. Otherwise nullptr is returned on any failures.
static std::unique_ptr<proto::PredictionModel>
@@ -159,11 +152,6 @@ class PredictionModelStore {
// Invoked when model files gets deleted.
void OnFilePathDeleted(const std::string& path_to_delete, bool success);
- // Local state that stores the prefs across all profiles. Not owned and
- // outlives |this|.
- raw_ptr<PrefService, LeakedDanglingUntriaged> local_state_
- GUARDED_BY_CONTEXT(sequence_checker_) = nullptr;
-
// The base dir where the prediction model dirs are saved.
base::FilePath base_store_dir_ GUARDED_BY_CONTEXT(sequence_checker_);
From ce8633a185cd8c2e819898d3a6cba63d1e8089c4 Mon Sep 17 00:00:00 2001
From: John Stiles <johnstiles@google.com>
Date: Wed, 31 Jan 2024 14:28:47 +0000
Subject: [PATCH] [Backport] Security bug 41495984
Cherry-pick of patch originally reviewed on:
https://chromium-review.googlesource.com/c/chromium/src/+/5249171
Improve handling of malformed BMP palettes.
Add CHECKs to guarantee that clr_used is reasonably sized when
ProcessColorTable() is called. Out-of-bounds values are capped
by ProcessInfoHeader() already, but since this happens at a
distance, it's better to be sure.
Additionally, we would previously add padding elements to a
palette if it was shorter than expected. We already had bounds
checks at the places where the palette was accessed, so we now
rely on those checks instead.
Bug: 1523030
Change-Id: I579c67d1029e1effba2036e9ec0c871418b140e2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5249171
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Auto-Submit: John Stiles <johnstiles@google.com>
Cr-Commit-Position: refs/heads/main@{#1254490}
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/551093
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
---
.../image-decoders/bmp/bmp_image_reader.cc | 25 ++++++++++---------
1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/chromium/third_party/blink/renderer/platform/image-decoders/bmp/bmp_image_reader.cc b/chromium/third_party/blink/renderer/platform/image-decoders/bmp/bmp_image_reader.cc
index b7bbbf51a1b..9c319b4be01 100644
--- src/3rdparty/chromium/third_party/blink/renderer/platform/image-decoders/bmp/bmp_image_reader.cc
+++ src/3rdparty/chromium/third_party/blink/renderer/platform/image-decoders/bmp/bmp_image_reader.cc
@@ -774,24 +774,31 @@ bool BMPImageReader::ProcessColorTable() {
const wtf_size_t header_end = header_offset_ + info_header_.size;
wtf_size_t colors_in_palette = info_header_.clr_used;
+ CHECK_LE(colors_in_palette, 256u); // Enforced by ProcessInfoHeader().
wtf_size_t table_size_in_bytes = colors_in_palette * bytes_per_color;
const wtf_size_t table_end = header_end + table_size_in_bytes;
if (table_end < header_end) {
return parent_->SetFailed();
}
- // Some BMPs don't contain a complete palette. Avoid reading off the end.
+ // Some BMPs don't contain a complete palette. Truncate it instead of reading
+ // off the end of the palette.
if (img_data_offset_ && (img_data_offset_ < table_end)) {
- colors_in_palette = (img_data_offset_ - header_end) / bytes_per_color;
+ wtf_size_t colors_in_truncated_palette =
+ (img_data_offset_ - header_end) / bytes_per_color;
+ CHECK_LE(colors_in_truncated_palette, colors_in_palette);
+ colors_in_palette = colors_in_truncated_palette;
table_size_in_bytes = colors_in_palette * bytes_per_color;
}
- // Read color table.
+ // If we don't have enough data to read in the whole palette yet, stop here.
if ((decoded_offset_ > data_->size()) ||
((data_->size() - decoded_offset_) < table_size_in_bytes)) {
return false;
}
- color_table_.resize(info_header_.clr_used);
+
+ // Read the color table.
+ color_table_.resize(colors_in_palette);
for (wtf_size_t i = 0; i < colors_in_palette; ++i) {
color_table_[i].rgb_blue = ReadUint8(0);
@@ -799,12 +806,6 @@ bool BMPImageReader::ProcessColorTable() {
color_table_[i].rgb_red = ReadUint8(2);
decoded_offset_ += bytes_per_color;
}
- // Explicitly zero any colors past the end of a truncated palette.
- for (wtf_size_t i = colors_in_palette; i < info_header_.clr_used; ++i) {
- color_table_[i].rgb_blue = 0;
- color_table_[i].rgb_green = 0;
- color_table_[i].rgb_red = 0;
- }
// We've now decoded all the non-image data we care about. Skip anything
// else before the actual raster data.
@@ -992,7 +993,7 @@ BMPImageReader::ProcessingResult BMPImageReader::ProcessRLEData() {
for (wtf_size_t which = 0; coord_.x() < end_x;) {
// Some images specify color values past the end of the
// color table; set these pixels to black.
- if (color_indexes[which] < info_header_.clr_used) {
+ if (color_indexes[which] < color_table_.size()) {
SetI(color_indexes[which]);
} else {
SetRGBA(0, 0, 0, 255);
@@ -1071,7 +1072,7 @@ BMPImageReader::ProcessingResult BMPImageReader::ProcessNonRLEData(
}
} else {
// See comments near the end of ProcessRLEData().
- if (color_index < info_header_.clr_used) {
+ if (color_index < color_table_.size()) {
SetI(color_index);
} else {
SetRGBA(0, 0, 0, 255);
From 4349868d9af8ef7175125f53e441b12df5a22927 Mon Sep 17 00:00:00 2001
From: Antonio Maiorano <amaiorano@google.com>
Date: Wed, 20 Mar 2024 17:15:40 -0400
Subject: [PATCH] [Backport] CVE-2024-2885: Use after free in Dawn
Manual cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/external/github.com/microsoft/DirectXShaderCompiler/+/5383595:
Fix HLMatrixLowerPass leaving call to dangling FunctionVal
When lowering an hl.cast, when the operand was an undef matrix, the pass would insert a call to a mat2vec stub, but since the undef value is not
an alloca, it never gets handled, and the call to the temporary stub
remains. Since the stub FunctionVal gets deleted, when the instruction
is accessed in a future pass, it reads a dangling pointer.
The fix is to handle undef similarly to how constant 0 is handled, and
to return an undef vector from lowerHLCast.
Bug: chromium:328958020
Change-Id: Id31e3aa326d9cb9f03ea97139f14dc5292cd6f7b
Reviewed-on: https://chromium-review.googlesource.com/c/external/github.com/microsoft/DirectXShaderCompiler/+/5383595
Reviewed-by: Ben Clayton <bclayton@chromium.org>
Reviewed-by: David Neto <dneto@google.com>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/553291
Reviewed-by: Michal Klocek <michal.klocek@qt.io>
---
.../dawn/third_party/dxc/lib/HLSL/HLMatrixLowerPass.cpp | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/chromium/third_party/dawn/third_party/dxc/lib/HLSL/HLMatrixLowerPass.cpp b/chromium/third_party/dawn/third_party/dxc/lib/HLSL/HLMatrixLowerPass.cpp
index e35ff832ecf..c3a7254ef2b 100644
--- src/3rdparty/chromium/third_party/dawn/third_party/dxc/lib/HLSL/HLMatrixLowerPass.cpp
+++ src/3rdparty/chromium/third_party/dawn/third_party/dxc/lib/HLSL/HLMatrixLowerPass.cpp
@@ -381,6 +381,11 @@ Value* HLMatrixLowerPass::getLoweredByValOperand(Value *Val, IRBuilder<> &Builde
if (isa<ConstantAggregateZero>(Val))
return ConstantAggregateZero::get(LoweredTy);
+ // Lower undef mat as undef vec
+ if (isa<UndefValue>(Val)) {
+ return UndefValue::get(LoweredTy);
+ }
+
// Return a mat-to-vec translation stub
FunctionType *TranslationStubTy = FunctionType::get(LoweredTy, { Ty }, /* isVarArg */ false);
Function *TranslationStub = m_matToVecStubs->get(TranslationStubTy);
From 0c7f8cd69b6065fbc9a2af8927182ffe529e052e Mon Sep 17 00:00:00 2001
From: Manos Koukoutos <manoskouk@chromium.org>
Date: Thu, 21 Mar 2024 11:38:08 +0100
Subject: [PATCH] [Backport] CVE-2024-2887: Type Confusion in WebAssembly
Cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/v8/v8/+/5380190:
Merged: [wasm] Check for type-definition count limit
(cherry picked from commit b852ad701db21d6db5b34e66f4ec1cdccd2ec4d4)
Bug: chromium:330575498
Change-Id: I395f0ed6d823b7d1e139da6551486e3627d65724
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5378419
Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Auto-Submit: Manos Koukoutos <manoskouk@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#92941}
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5380190
Reviewed-by: Francis McCabe <fgm@chromium.org>
Commit-Queue: Adam Klein <adamk@chromium.org>
Reviewed-by: Adam Klein <adamk@chromium.org>
Cr-Commit-Position: refs/branch-heads/12.2@{#50}
Cr-Branched-From: 6eb5a9616aa6f8c705217aeb7c7ab8c037a2f676-refs/heads/12.2.281@{#1}
Cr-Branched-From: 44cf56d850167c6988522f8981730462abc04bcc-refs/heads/main@{#91934}
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/553292
Reviewed-by: Michal Klocek <michal.klocek@qt.io>
---
chromium/v8/src/wasm/module-decoder-impl.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/chromium/v8/src/wasm/module-decoder-impl.h b/chromium/v8/src/wasm/module-decoder-impl.h
index 97554288b55..75ca3a630a2 100644
--- src/3rdparty/chromium/v8/src/wasm/module-decoder-impl.h
+++ src/3rdparty/chromium/v8/src/wasm/module-decoder-impl.h
@@ -687,6 +687,11 @@ class ModuleDecoderImpl : public Decoder {
}
} else {
if (tracer_) tracer_->TypeOffset(pc_offset());
+ if (initial_size + 1 > kV8MaxWasmTypes) {
+ errorf(pc(), "Type definition count exceeds maximum %zu",
+ kV8MaxWasmTypes);
+ return;
+ }
// Similarly to above, we need to resize types for a group of size 1.
module_->types.resize(initial_size + 1);
module_->isorecursive_canonical_type_ids.resize(initial_size + 1);
From d414fd5b22455b7fcbddfdee22cf2b1f446ce3c4 Mon Sep 17 00:00:00 2001
From: Marco Paniconi <marpan@google.com>
Date: Wed, 13 Mar 2024 10:58:17 -0700
Subject: [PATCH] [Backport] Security bug 329674887 (1/2)
Cherry-pick of patch orignally reviewed on
https://chromium-review.googlesource.com/c/webm/libvpx/+/5370376:
Fix to buffer alloc for vp9_bitstream_worker_data
The code was using the bitstream_worker_data when it
wasn't allocated for big enough size. This is because
the existing condition was to only re-alloc the
bitstream_worker_data when current dest_size was larger
than the current frame_size. But under resolution change
where frame_size is increased, beyond the current dest_size,
we need to allow re-alloc to the new size.
The existing condition to re-alloc when dest_size is
larger than frame_size (which is not required) is kept
for now.
Also increase the dest_size to account for image format.
Added tests, for both ROW_MT=0 and 1, that reproduce
the failures in the bugs below.
Note: this issue only affects the REALTIME encoding path.
Bug: b/329088759, b/329674887, b/329179808
Change-Id: Icd65dbc5317120304d803f648d4bd9405710db6f
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/553293
Reviewed-by: Michal Klocek <michal.klocek@qt.io>
---
.../source/libvpx/vp9/encoder/vp9_bitstream.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/chromium/third_party/libvpx/source/libvpx/vp9/encoder/vp9_bitstream.c b/chromium/third_party/libvpx/source/libvpx/vp9/encoder/vp9_bitstream.c
index ca56d14aa1e..88a031e5fc1 100644
--- src/3rdparty/chromium/third_party/libvpx/source/libvpx/vp9/encoder/vp9_bitstream.c
+++ src/3rdparty/chromium/third_party/libvpx/source/libvpx/vp9/encoder/vp9_bitstream.c
@@ -962,6 +962,14 @@ void vp9_bitstream_encode_tiles_buffer_dealloc(VP9_COMP *const cpi) {
}
}
+static int encode_tiles_buffer_alloc_size(VP9_COMP *const cpi) {
+ VP9_COMMON *const cm = &cpi->common;
+ const int image_bps =
+ (8 + 2 * (8 >> (cm->subsampling_x + cm->subsampling_y))) *
+ (1 + (cm->bit_depth > 8));
+ return cpi->oxcf.width * cpi->oxcf.height * image_bps / 8;
+}
+
static void encode_tiles_buffer_alloc(VP9_COMP *const cpi) {
VP9_COMMON *const cm = &cpi->common;
int i;
@@ -972,7 +980,7 @@ static void encode_tiles_buffer_alloc(VP9_COMP *const cpi) {
memset(cpi->vp9_bitstream_worker_data, 0, worker_data_size);
for (i = 1; i < cpi->num_workers; ++i) {
cpi->vp9_bitstream_worker_data[i].dest_size =
- cpi->oxcf.width * cpi->oxcf.height;
+ encode_tiles_buffer_alloc_size(cpi);
CHECK_MEM_ERROR(&cm->error, cpi->vp9_bitstream_worker_data[i].dest,
vpx_malloc(cpi->vp9_bitstream_worker_data[i].dest_size));
}
@@ -987,8 +995,8 @@ static size_t encode_tiles_mt(VP9_COMP *cpi, uint8_t *data_ptr) {
int tile_col = 0;
if (!cpi->vp9_bitstream_worker_data ||
- cpi->vp9_bitstream_worker_data[1].dest_size >
- (cpi->oxcf.width * cpi->oxcf.height)) {
+ cpi->vp9_bitstream_worker_data[1].dest_size !=
+ encode_tiles_buffer_alloc_size(cpi)) {
vp9_bitstream_encode_tiles_buffer_dealloc(cpi);
encode_tiles_buffer_alloc(cpi);
}
From 4f90911c049d0278e900b94947fd0055b26d646e Mon Sep 17 00:00:00 2001
From: Marco Paniconi <marpan@google.com>
Date: Sat, 16 Mar 2024 10:39:28 -0700
Subject: [PATCH] [Backport] Security bug 329674887 (2/2)
Cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/webm/libvpx/+/5375794:vp9: fix to integer overflow test
failure for the 16k test: issue introduced
in: c29e637283
Bug: b/329088759, b/329674887, b/329179808
Change-Id: I88e8a36b7f13223997c3006c84aec9cfa48c0bcf
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/553294
Reviewed-by: Michal Klocek <michal.klocek@qt.io>
---
.../libvpx/source/libvpx/vp9/encoder/vp9_bitstream.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/chromium/third_party/libvpx/source/libvpx/vp9/encoder/vp9_bitstream.c b/chromium/third_party/libvpx/source/libvpx/vp9/encoder/vp9_bitstream.c
index 88a031e5fc1..d3c029da4ba 100644
--- src/3rdparty/chromium/third_party/libvpx/source/libvpx/vp9/encoder/vp9_bitstream.c
+++ src/3rdparty/chromium/third_party/libvpx/source/libvpx/vp9/encoder/vp9_bitstream.c
@@ -967,7 +967,9 @@ static int encode_tiles_buffer_alloc_size(VP9_COMP *const cpi) {
const int image_bps =
(8 + 2 * (8 >> (cm->subsampling_x + cm->subsampling_y))) *
(1 + (cm->bit_depth > 8));
- return cpi->oxcf.width * cpi->oxcf.height * image_bps / 8;
+ const int64_t size =
+ (int64_t)cpi->oxcf.width * cpi->oxcf.height * image_bps / 8;
+ return (int)size;
}
static void encode_tiles_buffer_alloc(VP9_COMP *const cpi) {
From 5af5e96fba0c40d3ddef2720de9117b6a4d6c267 Mon Sep 17 00:00:00 2001
From: Peng Huang <penghuang@chromium.org>
Date: Wed, 20 Mar 2024 16:22:16 +0000
Subject: [PATCH] [Backport] Security bug 327183408
Cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/5382202:
Fix PaintImage deserialization arbitrary-read issue
(cherry picked from commit 47e8386c97ac7a84a96866fbd35422b99a01de5a)
Bug: 327183408
Change-Id: I09927fbae60b666aaa370e3aba01607cdb977a25
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5370455
Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org>
Commit-Queue: Peng Huang <penghuang@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1272930}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5382202
Auto-Submit: Peng Huang <penghuang@chromium.org>
Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org>
Cr-Commit-Position: refs/branch-heads/6261@{#1106}
Cr-Branched-From: 9755d9d81e4a8cb5b4f76b23b761457479dbb06b-refs/heads/main@{#1250580}
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/553295
Reviewed-by: Michal Klocek <michal.klocek@qt.io>
---
chromium/cc/paint/paint_op_reader.cc | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/chromium/cc/paint/paint_op_reader.cc b/chromium/cc/paint/paint_op_reader.cc
index 935dbb05a50..6d920c33562 100644
--- src/3rdparty/chromium/cc/paint/paint_op_reader.cc
+++ src/3rdparty/chromium/cc/paint/paint_op_reader.cc
@@ -1532,9 +1532,10 @@ inline void PaintOpReader::DidRead(size_t bytes_read) {
// All data are aligned with PaintOpWriter::kDefaultAlignment at least.
size_t aligned_bytes =
base::bits::AlignUp(bytes_read, PaintOpWriter::kDefaultAlignment);
- memory_ += aligned_bytes;
DCHECK_LE(aligned_bytes, remaining_bytes_);
- remaining_bytes_ -= aligned_bytes;
+ bytes_read = std::min(aligned_bytes, remaining_bytes_);
+ memory_ += bytes_read;
+ remaining_bytes_ -= bytes_read;
}
} // namespace cc
From 0a17b9024c84404e4693718bbbd056517a2874a3 Mon Sep 17 00:00:00 2001
From: Darius Mercadier <dmercadier@chromium.org>
Date: Fri, 22 Mar 2024 17:55:04 +0100
Subject: [PATCH] [Backport] CVE-2024-3159: Out of bounds memory access in V8
Cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/v8/v8/+/5401859:
Merged: [runtime] Recreate enum cache on map update if any previous map had one
If any previous map in the transition tree had an enum cache, then we
recreate one when updating the map.
Bug: 330760873
(cherry picked from commit 807cf7d0b7d96212c98ed2119e07f9b2c6a23f61)
Change-Id: Ia9ea4cf17fef60166a0c037318eb539866aac37a
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5401859
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Auto-Submit: Darius Mercadier <dmercadier@chromium.org>
Cr-Commit-Position: refs/branch-heads/12.2@{#52}
Cr-Branched-From: 6eb5a9616aa6f8c705217aeb7c7ab8c037a2f676-refs/heads/12.2.281@{#1}
Cr-Branched-From: 44cf56d850167c6988522f8981730462abc04bcc-refs/heads/main@{#91934}
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/553296
Reviewed-by: Michal Klocek <michal.klocek@qt.io>
---
chromium/v8/src/objects/map-updater.cc | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/chromium/v8/src/objects/map-updater.cc b/chromium/v8/src/objects/map-updater.cc
index 613a4921637..1d649373274 100644
--- src/3rdparty/chromium/v8/src/objects/map-updater.cc
+++ src/3rdparty/chromium/v8/src/objects/map-updater.cc
@@ -1038,14 +1038,21 @@ MapUpdater::State MapUpdater::ConstructNewMap() {
Handle<Map> new_map =
Map::AddMissingTransitions(isolate_, split_map, new_descriptors);
+ bool had_any_enum_cache =
+ split_map->instance_descriptors(isolate_)
+ ->enum_cache()
+ ->keys()
+ ->length() > 0 ||
+ old_descriptors_->enum_cache()->keys()->length() > 0;
+
// Deprecated part of the transition tree is no longer reachable, so replace
// current instance descriptors in the "survived" part of the tree with
// the new descriptors to maintain descriptors sharing invariant.
split_map->ReplaceDescriptors(isolate_, *new_descriptors);
- // If the old descriptors had an enum cache, make sure the new ones do too.
- if (old_descriptors_->enum_cache()->keys()->length() > 0 &&
- new_map->NumberOfEnumerableProperties() > 0) {
+ // If the old descriptors had an enum cache (or if {split_map}'s descriptors
+ // had one), make sure the new ones do too.
+ if (had_any_enum_cache && new_map->NumberOfEnumerableProperties() > 0) {
FastKeyAccumulator::InitializeFastPropertyEnumCache(
isolate_, new_map, new_map->NumberOfEnumerableProperties());
}
From e76cac29493b1cb4b055f8944ea1e4e1284a12e6 Mon Sep 17 00:00:00 2001
From: Raphael Kubo Da Costa <raphael.kubo.da.costa@intel.com>
Date: Thu, 21 Mar 2024 16:50:44 +0000
Subject: [PATCH] [Backport] Security bug 326349405
Cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/5383440:
Reland "sensors WinRT: Call OnReadingChangedCallback() via PostTask()"
This reverts commit 745ca2de005c052f58097ef9cb7aa2deff095cf4.
Reason for revert: Reapproved by pgrace@ in the bug.
Original change's description:
> Revert "sensors WinRT: Call OnReadingChangedCallback() via PostTask()"
>
> This reverts commit 7e93b6a926ab65cc7ff1293bdcf0099c6e6e4e4e.
>
> Reason for revert: Requested in the bug by pgrace@
>
> Original change's description:
> > sensors WinRT: Call OnReadingChangedCallback() via PostTask()
> >
> > While here, also add checks to make sure each method is running on the
> > right sequence and make |minimum_report_interval_| guarded by |lock_|
> > since it is accessed by the main task runner just like |client_|.
> >
> > There is a significant amount of changes in the unit tests for two
> > reasons:
> > 1. We now use multiple task runners and create Reader objects in a COM
> > STA task runner to better simulate what happens in production.
> > 2. Doing so has uncovered bugs in the exist tests that had to be fixed.
> > Namely:
> > - One of the biggest offenders was the use of EXPECT_CALL() with
> > WillRepeatedly() for expecting calls to OnReadingUpdated(). Using
> > only WillRepeatedly() meant the control over the cardinality of the
> > expectations was not very strict, and sometimes callbacks were
> > simply not being run.
> > Now that TriggerFakeSensorReading() is asynchronous and we need to
> > use a base::RunLoop to ensure, we are also using WillOnce() a lot
> > more than WillRepeatedly() so that we set one expectation, call
> > TriggerFakeSensorReading() and consume it immediately.
> > - The *Thresholding tests were affected by the problem above, and
> > fixing them showed that several callbacks were not being invoked.
> > Many checks where values were increased by the exact threshold
> > amount were broken because the manipulated values are floats and
> > doubles, and the math operations on them in the
> > OnReadingChangedCallback() implementations caused the comparisons
> > with exact values to fail. In this case, it was simpler to just
> > remove those specific tests, as the "values bigger than the
> > threshold" case are already covered by other checks.
> > - Also as a consequence of the above, *Thresholding tests with
> > multi-axis values were also broken when they went from testing that
> > values did not pass the threshold checks to the first test that
> > verifies that an axis passes the threshold check. This caused all
> > previous |last_sent_*| variables to be stored in the Reader, and
> > other calls to threshold_helper(true) would fail. The fix here was
> > to reorder the calls so that each axis is tested entirely before
> > the next.
> >
> > (cherry picked from commit 2aafa000795519b5153125673f87c734f7b8ae9f)
> >
> > Bug: 326349405
> > Change-Id: Ief67720e8c449af1ce4f450002103a20ca1830ee
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5340216
> > Auto-Submit: Raphael Kubo Da Costa <raphael.kubo.da.costa@intel.com>
> > Commit-Queue: Raphael Kubo Da Costa <raphael.kubo.da.costa@intel.com>
> > Reviewed-by: Reilly Grant <reillyg@chromium.org>
> > Cr-Original-Commit-Position: refs/heads/main@{#1268797}
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5372846
> > Reviewed-by: Colin Blundell <blundell@chromium.org>
> > Cr-Commit-Position: refs/branch-heads/6261@{#1079}
> > Cr-Branched-From: 9755d9d81e4a8cb5b4f76b23b761457479dbb06b-refs/heads/main@{#1250580}
>
> Bug: 326349405
> Change-Id: I49d61cf7bdf2a00004aa565a5439ad813b1c379e
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5378404
> Commit-Queue: Raphael Kubo Da Costa <raphael.kubo.da.costa@intel.com>
> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Cr-Commit-Position: refs/branch-heads/6261@{#1090}
> Cr-Branched-From: 9755d9d81e4a8cb5b4f76b23b761457479dbb06b-refs/heads/main@{#1250580}
Bug: 326349405
Change-Id: I3bcba8840a3a10cd4660ec287fa24623bcf87657
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5383440
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Raphael Kubo Da Costa <raphael.kubo.da.costa@intel.com>
Cr-Commit-Position: refs/branch-heads/6261@{#1111}
Cr-Branched-From: 9755d9d81e4a8cb5b4f76b23b761457479dbb06b-refs/heads/main@{#1250580}
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/553297
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
---
.../platform_sensor_reader_winrt.cc | 79 +-
.../platform_sensor_reader_winrt.h | 32 +-
.../platform_sensor_reader_winrt_unittests.cc | 787 ++++++++++--------
.../generic_sensor/platform_sensor_win.cc | 3 +
4 files changed, 549 insertions(+), 352 deletions(-)
diff --git a/chromium/services/device/generic_sensor/platform_sensor_reader_winrt.cc b/chromium/services/device/generic_sensor/platform_sensor_reader_winrt.cc
index 6c778b7edf0c..673225e398a2 100644
--- src/3rdparty/chromium/services/device/generic_sensor/platform_sensor_reader_winrt.cc
+++ src/3rdparty/chromium/services/device/generic_sensor/platform_sensor_reader_winrt.cc
@@ -8,6 +8,7 @@
#include "base/numerics/math_constants.h"
#include "base/time/time.h"
+#include "base/win/com_init_util.h"
#include "base/win/core_winrt_util.h"
#include "services/device/generic_sensor/generic_sensor_consts.h"
#include "services/device/public/mojom/sensor.mojom.h"
@@ -99,7 +100,11 @@ PlatformSensorReaderWinrtBase<
ISensorWinrtStatics,
ISensorWinrtClass,
ISensorReadingChangedHandler,
- ISensorReadingChangedEventArgs>::PlatformSensorReaderWinrtBase() {
+ ISensorReadingChangedEventArgs>::PlatformSensorReaderWinrtBase()
+ : com_sta_task_runner_(base::SingleThreadTaskRunner::GetCurrentDefault()) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(com_sta_sequence_checker_);
+ DETACH_FROM_SEQUENCE(main_sequence_checker_);
+
get_sensor_factory_callback_ =
base::BindRepeating([](ISensorWinrtStatics** sensor_factory) -> HRESULT {
return base::win::GetActivationFactory<ISensorWinrtStatics,
@@ -119,6 +124,8 @@ void PlatformSensorReaderWinrtBase<
ISensorWinrtClass,
ISensorReadingChangedHandler,
ISensorReadingChangedEventArgs>::SetClient(Client* client) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(main_sequence_checker_);
+
base::AutoLock autolock(lock_);
client_ = client;
}
@@ -136,6 +143,8 @@ HRESULT PlatformSensorReaderWinrtBase<runtime_class_id,
ISensorReadingChangedEventArgs>::
ConvertSensorReadingTimeStamp(ComPtr<ISensorReading> sensor_reading,
base::TimeDelta* timestamp_delta) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(com_sta_sequence_checker_);
+
DateTime timestamp;
HRESULT hr = sensor_reading->get_Timestamp(&timestamp);
if (FAILED(hr))
@@ -157,6 +166,8 @@ bool PlatformSensorReaderWinrtBase<
ISensorWinrtClass,
ISensorReadingChangedHandler,
ISensorReadingChangedEventArgs>::Initialize() {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(com_sta_sequence_checker_);
+
ComPtr<ISensorWinrtStatics> sensor_statics;
HRESULT hr = get_sensor_factory_callback_.Run(&sensor_statics);
@@ -180,10 +191,14 @@ bool PlatformSensorReaderWinrtBase<
return false;
}
- minimum_report_interval_ = GetMinimumReportIntervalFromSensor();
+ {
+ base::AutoLock autolock(lock_);
+ minimum_report_interval_ = GetMinimumReportIntervalFromSensor();
- if (minimum_report_interval_.is_zero())
- DLOG(WARNING) << "Failed to get sensor minimum report interval";
+ if (minimum_report_interval_.is_zero()) {
+ DLOG(WARNING) << "Failed to get sensor minimum report interval";
+ }
+ }
return true;
}
@@ -199,6 +214,8 @@ base::TimeDelta PlatformSensorReaderWinrtBase<
ISensorWinrtClass,
ISensorReadingChangedHandler,
ISensorReadingChangedEventArgs>::GetMinimumReportIntervalFromSensor() {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(com_sta_sequence_checker_);
+
UINT32 minimum_report_interval_ms = 0;
HRESULT hr = sensor_->get_MinimumReportInterval(&minimum_report_interval_ms);
@@ -225,6 +242,9 @@ base::TimeDelta PlatformSensorReaderWinrtBase<
ISensorWinrtClass,
ISensorReadingChangedHandler,
ISensorReadingChangedEventArgs>::GetMinimalReportingInterval() const {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(main_sequence_checker_);
+
+ base::AutoLock autolock(lock_);
return minimum_report_interval_;
}
@@ -239,6 +259,8 @@ bool PlatformSensorReaderWinrtBase<runtime_class_id,
ISensorReadingChangedHandler,
ISensorReadingChangedEventArgs>::
StartSensor(const PlatformSensorConfiguration& configuration) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(main_sequence_checker_);
+
base::AutoLock autolock(lock_);
if (!reading_callback_token_) {
@@ -256,7 +278,39 @@ bool PlatformSensorReaderWinrtBase<runtime_class_id,
}
auto reading_changed_handler = Callback<ISensorReadingChangedHandler>(
- this, &PlatformSensorReaderWinrtBase::OnReadingChangedCallback);
+ [weak_ptr(weak_ptr_factory_.GetWeakPtr()),
+ com_sta_task_runner(com_sta_task_runner_)](
+ ISensorWinrtClass* sender, ISensorReadingChangedEventArgs* args) {
+ // We cannot invoke OnReadingChangedCallback() directly because this
+ // callback is run on a COM MTA thread spawned by Windows (on tests,
+ // we mimic the behavior by using base::ThreadPool, as the task
+ // scheduler threads live in the MTA).
+ //
+ // This callback is invoked on an MTA thread because the
+ // ISensorReadingChangedHandler declarations explicitly inherit from
+ // Microsoft::WRL::FtmBase, which makes them agile, free-threaded
+ // objects.
+ //
+ // We could CHECK() this behavior here, but ::CoGetApartmentType()
+ // depends on ole32.dll and base::win::GetComApartmentTypeForThread()
+ // returns NONE in the non-test code path even though
+ // ::GoGetApartmentType() returns MTA, so the best we can do is just
+ // double-check that this is not running on an STA.
+ DCHECK_NE(base::win::GetComApartmentTypeForThread(),
+ base::win::ComApartmentType::STA);
+ com_sta_task_runner->PostTask(
+ FROM_HERE,
+ base::BindOnce(
+ // TODO(crbug.com/326349405): base::IgnoreResult is being used
+ // temporarily to reduce the amount of changes required for
+ // this bug. OnReadingChangedCallback() must be changed to
+ // have a void return type.
+ base::IgnoreResult(
+ &PlatformSensorReaderWinrtBase::OnReadingChangedCallback),
+ weak_ptr, ComPtr<ISensorWinrtClass>(sender),
+ ComPtr<ISensorReadingChangedEventArgs>(args)));
+ return S_OK;
+ });
EventRegistrationToken event_token;
hr = sensor_->add_ReadingChanged(reading_changed_handler.Get(),
@@ -285,6 +339,9 @@ void PlatformSensorReaderWinrtBase<
ISensorWinrtClass,
ISensorReadingChangedHandler,
ISensorReadingChangedEventArgs>::StopSensor() {
+ // This function is called in the main task runner by PlatformSensorWin as
+ // well as in the com_sta_task_runner_ by the destructor.
+
base::AutoLock autolock(lock_);
if (reading_callback_token_) {
@@ -316,6 +373,8 @@ PlatformSensorReaderWinrtLightSensor::PlatformSensorReaderWinrtLightSensor() =
HRESULT PlatformSensorReaderWinrtLightSensor::OnReadingChangedCallback(
ILightSensor* light_sensor,
ILightSensorReadingChangedEventArgs* reading_changed_args) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(com_sta_sequence_checker_);
+
ComPtr<ILightSensorReading> light_sensor_reading;
HRESULT hr = reading_changed_args->get_Reading(&light_sensor_reading);
if (FAILED(hr)) {
@@ -379,6 +438,8 @@ PlatformSensorReaderWinrtAccelerometer::
HRESULT PlatformSensorReaderWinrtAccelerometer::OnReadingChangedCallback(
IAccelerometer* accelerometer,
IAccelerometerReadingChangedEventArgs* reading_changed_args) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(com_sta_sequence_checker_);
+
ComPtr<IAccelerometerReading> accelerometer_reading;
HRESULT hr = reading_changed_args->get_Reading(&accelerometer_reading);
if (FAILED(hr)) {
@@ -464,6 +525,8 @@ PlatformSensorReaderWinrtGyrometer::PlatformSensorReaderWinrtGyrometer() =
HRESULT PlatformSensorReaderWinrtGyrometer::OnReadingChangedCallback(
IGyrometer* gyrometer,
IGyrometerReadingChangedEventArgs* reading_changed_args) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(com_sta_sequence_checker_);
+
ComPtr<IGyrometerReading> gyrometer_reading;
HRESULT hr = reading_changed_args->get_Reading(&gyrometer_reading);
if (FAILED(hr)) {
@@ -548,6 +611,8 @@ PlatformSensorReaderWinrtMagnetometer::PlatformSensorReaderWinrtMagnetometer() =
HRESULT PlatformSensorReaderWinrtMagnetometer::OnReadingChangedCallback(
IMagnetometer* magnetometer,
IMagnetometerReadingChangedEventArgs* reading_changed_args) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(com_sta_sequence_checker_);
+
ComPtr<IMagnetometerReading> magnetometer_reading;
HRESULT hr = reading_changed_args->get_Reading(&magnetometer_reading);
if (FAILED(hr)) {
@@ -631,6 +696,8 @@ HRESULT
PlatformSensorReaderWinrtAbsOrientationEulerAngles::OnReadingChangedCallback(
IInclinometer* inclinometer,
IInclinometerReadingChangedEventArgs* reading_changed_args) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(com_sta_sequence_checker_);
+
ComPtr<IInclinometerReading> inclinometer_reading;
HRESULT hr = reading_changed_args->get_Reading(&inclinometer_reading);
if (FAILED(hr)) {
@@ -717,6 +784,8 @@ HRESULT
PlatformSensorReaderWinrtAbsOrientationQuaternion::OnReadingChangedCallback(
IOrientationSensor* orientation_sensor,
IOrientationSensorReadingChangedEventArgs* reading_changed_args) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(com_sta_sequence_checker_);
+
ComPtr<IOrientationSensorReading> orientation_sensor_reading;
HRESULT hr = reading_changed_args->get_Reading(&orientation_sensor_reading);
if (FAILED(hr)) {
diff --git a/chromium/services/device/generic_sensor/platform_sensor_reader_winrt.h b/chromium/services/device/generic_sensor/platform_sensor_reader_winrt.h
index 66c40adc59e8..e4521a7816fa 100644
--- src/3rdparty/chromium/services/device/generic_sensor/platform_sensor_reader_winrt.h
+++ src/3rdparty/chromium/services/device/generic_sensor/platform_sensor_reader_winrt.h
@@ -14,7 +14,10 @@
#include "base/functional/callback.h"
#include "base/memory/raw_ptr.h"
+#include "base/memory/weak_ptr.h"
+#include "base/sequence_checker.h"
#include "base/synchronization/lock.h"
+#include "base/task/single_thread_task_runner.h"
#include "base/thread_annotations.h"
#include "base/time/time.h"
#include "services/device/generic_sensor/platform_sensor_reader_win_base.h"
@@ -77,7 +80,10 @@ class PlatformSensorReaderWinrtBase : public PlatformSensorReaderWinBase {
protected:
PlatformSensorReaderWinrtBase();
- virtual ~PlatformSensorReaderWinrtBase() { StopSensor(); }
+ virtual ~PlatformSensorReaderWinrtBase() {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(com_sta_sequence_checker_);
+ StopSensor();
+ }
// Derived classes should implement this function to handle sensor specific
// parsing of the sensor reading.
@@ -93,11 +99,15 @@ class PlatformSensorReaderWinrtBase : public PlatformSensorReaderWinBase {
Microsoft::WRL::ComPtr<ISensorReading> sensor_reading,
base::TimeDelta* timestamp_delta);
- // Following class member is protected by lock since SetClient,
- // StartSensor, and StopSensor can all be called from different
- // threads by PlatformSensorWin.
- base::Lock lock_;
- // Null if there is no client to notify, non-null otherwise.
+ SEQUENCE_CHECKER(com_sta_sequence_checker_);
+ SEQUENCE_CHECKER(main_sequence_checker_);
+
+ mutable base::Lock lock_;
+
+ // Null if there is no client to notify, non-null otherwise. Protected by
+ // |lock_| because SetClient() and StartSensor() are called from the main
+ // task runner rather than the thread where this object is created, and
+ // StopSensor() may be called from the main task runner too.
raw_ptr<Client, DanglingUntriaged> client_ GUARDED_BY(lock_);
// Always report the first sample received after starting the sensor.
@@ -106,13 +116,21 @@ class PlatformSensorReaderWinrtBase : public PlatformSensorReaderWinBase {
private:
base::TimeDelta GetMinimumReportIntervalFromSensor();
+ // Task runner where this object was created.
+ scoped_refptr<base::SingleThreadTaskRunner> com_sta_task_runner_;
+
GetSensorFactoryFunctor get_sensor_factory_callback_;
// absl::nullopt if the sensor has not been started, non-empty otherwise.
absl::optional<EventRegistrationToken> reading_callback_token_;
- base::TimeDelta minimum_report_interval_;
+ // Protected by |lock_| because GetMinimalReportingInterval() is called from
+ // the main task runner.
+ base::TimeDelta minimum_report_interval_ GUARDED_BY(lock_);
+
Microsoft::WRL::ComPtr<ISensorWinrtClass> sensor_;
+
+ base::WeakPtrFactory<PlatformSensorReaderWinrtBase> weak_ptr_factory_{this};
};
class PlatformSensorReaderWinrtLightSensor final
diff --git a/chromium/services/device/generic_sensor/platform_sensor_reader_winrt_unittests.cc b/chromium/services/device/generic_sensor/platform_sensor_reader_winrt_unittests.cc
index 2283de90c75d..f6add828b496 100644
--- src/3rdparty/chromium/services/device/generic_sensor/platform_sensor_reader_winrt_unittests.cc
+++ src/3rdparty/chromium/services/device/generic_sensor/platform_sensor_reader_winrt_unittests.cc
@@ -6,8 +6,13 @@
#include <objbase.h>
+#include "base/notreached.h"
#include "base/numerics/math_constants.h"
+#include "base/run_loop.h"
+#include "base/task/task_traits.h"
+#include "base/task/thread_pool.h"
#include "base/test/bind.h"
+#include "base/test/gmock_callback_support.h"
#include "base/test/task_environment.h"
#include "base/win/core_winrt_util.h"
#include "base/win/scoped_com_initializer.h"
@@ -428,15 +433,26 @@ class FakeSensorWinrt
return remove_reading_changed_return_code_;
}
- // Makes any clients registered via add_ReadingChanged() to trigger with
- // the given sensor reading.
+ // Invokes the handler added via add_ReadingChanged() with the reading
+ // described by |reading|.
+ //
+ // The invocation is asynchronous to better simulate real behavior, where
+ // Windows delivers the reading notifications in a separate MTA thread.
void TriggerFakeSensorReading(
Microsoft::WRL::ComPtr<ISensorReading> reading) {
- EXPECT_TRUE(handler_);
Microsoft::WRL::ComPtr<ISensorReadingChangedEventArgs> reading_event_args =
Microsoft::WRL::Make<FakeSensorReadingChangedEventArgsWinrt<
ISensorReading, ISensorReadingChangedEventArgs>>(reading);
- EXPECT_HRESULT_SUCCEEDED(handler_->Invoke(this, reading_event_args.Get()));
+ base::ThreadPool::PostTask(
+ FROM_HERE, {base::MayBlock()},
+ base::BindLambdaForTesting(
+ // Copy |handler_| and |reading_event_args| to ensure they do not
+ // lose their values when TriggerFakeSensorReading() exits.
+ [this, handler = handler_, reading_event_args]() {
+ ASSERT_TRUE(handler);
+ EXPECT_HRESULT_SUCCEEDED(
+ handler->Invoke(this, reading_event_args.Get()));
+ }));
}
// Returns true if any clients are registered for readings via
@@ -547,7 +563,47 @@ class FakeSensorFactoryWinrt
};
class PlatformSensorReaderTestWinrt : public testing::Test {
- private:
+ public:
+ void SetUp() override {
+ // Ensure each test starts with a fresh task runner.
+ com_sta_task_runner_ =
+ base::ThreadPool::CreateCOMSTATaskRunner({base::MayBlock()});
+ }
+
+ // Synchronously creates a new PlatformSensorReaderWinrtBase-derived class on
+ // |com_sta_task_runner_| and returns it.
+ //
+ // This better simulates real behavior, as PlatformSensorProviderWinrt
+ // creates readers on a COM STA task runner.
+ template <typename SensorReader,
+ typename ISensorStatics,
+ typename... OtherFactoryTypes>
+ auto CreateAndInitializeSensor(
+ const Microsoft::WRL::ComPtr<
+ FakeSensorFactoryWinrt<ISensorStatics, OtherFactoryTypes...>>&
+ fake_sensor_factory) {
+ std::unique_ptr<SensorReader, base::OnTaskRunnerDeleter> reader(
+ nullptr, base::OnTaskRunnerDeleter(com_sta_task_runner_));
+
+ base::RunLoop run_loop;
+ com_sta_task_runner_->PostTaskAndReply(
+ FROM_HERE, base::BindLambdaForTesting([&]() {
+ reader.reset(new SensorReader);
+ reader->InitForTesting(base::BindLambdaForTesting(
+ [&](ISensorStatics** sensor_factory) -> HRESULT {
+ return fake_sensor_factory.CopyTo(sensor_factory);
+ }));
+ ASSERT_TRUE(reader->Initialize());
+ }),
+ run_loop.QuitClosure());
+ run_loop.Run();
+
+ return reader;
+ }
+
+ protected:
+ scoped_refptr<base::SingleThreadTaskRunner> com_sta_task_runner_;
+
base::test::TaskEnvironment task_environment_;
base::win::ScopedCOMInitializer scoped_com_initializer_;
};
@@ -602,11 +658,9 @@ TEST_F(PlatformSensorReaderTestWinrt, SensorMinimumReportInterval) {
ABI::Windows::Devices::Sensors::LightSensorReadingChangedEventArgs>>();
auto fake_sensor = fake_sensor_factory->fake_sensor_;
- auto sensor = std::make_unique<PlatformSensorReaderWinrtLightSensor>();
- sensor->InitForTesting(base::BindLambdaForTesting(
- [&](ABI::Windows::Devices::Sensors::ILightSensorStatics** sensor_factory)
- -> HRESULT { return fake_sensor_factory.CopyTo(sensor_factory); }));
- EXPECT_TRUE(sensor->Initialize());
+ auto sensor = CreateAndInitializeSensor<PlatformSensorReaderWinrtLightSensor>(
+ fake_sensor_factory);
+ ASSERT_TRUE(sensor);
EXPECT_EQ(sensor->GetMinimalReportingInterval().InMilliseconds(),
kExpectedMinimumReportInterval);
@@ -623,20 +677,42 @@ TEST_F(PlatformSensorReaderTestWinrt, FailedSensorMinimumReportInterval) {
ABI::Windows::Devices::Sensors::ILightSensorReadingChangedEventArgs,
ABI::Windows::Devices::Sensors::LightSensorReadingChangedEventArgs>>();
auto fake_sensor = fake_sensor_factory->fake_sensor_;
-
- auto sensor = std::make_unique<PlatformSensorReaderWinrtLightSensor>();
fake_sensor->SetGetMinimumReportIntervalReturnCode(E_FAIL);
- sensor->InitForTesting(base::BindLambdaForTesting(
- [&](ABI::Windows::Devices::Sensors::ILightSensorStatics** sensor_factory)
- -> HRESULT { return fake_sensor_factory.CopyTo(sensor_factory); }));
- EXPECT_TRUE(sensor->Initialize());
+
+ auto sensor = CreateAndInitializeSensor<PlatformSensorReaderWinrtLightSensor>(
+ fake_sensor_factory);
+ ASSERT_TRUE(sensor);
EXPECT_EQ(sensor->GetMinimalReportingInterval().InMilliseconds(), 0);
}
-// Tests that PlatformSensorReaderWinrtBase converts the timestamp correctly
-TEST_F(PlatformSensorReaderTestWinrt, SensorTimestampConversion) {
- static constexpr double expectedTimestampDeltaSecs = 19.0;
+TEST_F(PlatformSensorReaderTestWinrt, ReadingChangedCallbackAndPostTask) {
+ // Instead of using PlatformSensorReaderWinrtLightSensor, declare a custom
+ // implementation that does less and whose sole purpose is to assert that its
+ // OnReadingChangedCallback() implementation is never called.
+ struct CustomLightSensor
+ : public PlatformSensorReaderWinrtBase<
+ RuntimeClass_Windows_Devices_Sensors_LightSensor,
+ ABI::Windows::Devices::Sensors::ILightSensorStatics,
+ ABI::Windows::Devices::Sensors::ILightSensor,
+ Microsoft::WRL::Implements<
+ Microsoft::WRL::RuntimeClassFlags<Microsoft::WRL::ClassicCom>,
+ ABI::Windows::Foundation::ITypedEventHandler<
+ ABI::Windows::Devices::Sensors::LightSensor*,
+ ABI::Windows::Devices::Sensors::
+ LightSensorReadingChangedEventArgs*>,
+ Microsoft::WRL::FtmBase>,
+ ABI::Windows::Devices::Sensors::
+ ILightSensorReadingChangedEventArgs> {
+ ~CustomLightSensor() override = default;
+
+ HRESULT OnReadingChangedCallback(
+ ABI::Windows::Devices::Sensors::ILightSensor*,
+ ABI::Windows::Devices::Sensors::ILightSensorReadingChangedEventArgs*)
+ override {
+ NOTREACHED_NORETURN() << "This function should not have been reached";
+ }
+ };
auto fake_sensor_factory = Microsoft::WRL::Make<FakeSensorFactoryWinrt<
ABI::Windows::Devices::Sensors::ILightSensorStatics,
@@ -647,21 +723,57 @@ TEST_F(PlatformSensorReaderTestWinrt, SensorTimestampConversion) {
ABI::Windows::Devices::Sensors::LightSensorReadingChangedEventArgs>>();
auto fake_sensor = fake_sensor_factory->fake_sensor_;
- auto sensor = std::make_unique<PlatformSensorReaderWinrtLightSensor>();
- sensor->InitForTesting(base::BindLambdaForTesting(
+ // Instead of using CreateAndInitializeSensor(), for simplicity and for
+ // better control over |light_sensor|'s lifetime we invert things and create
+ // the object in the main task runner and invoke StartSensor() from another
+ // one. The effect on the Reader is the same -- the calls are still made from
+ // different task runners.
+ auto light_sensor = std::make_unique<CustomLightSensor>();
+ light_sensor->InitForTesting(base::BindLambdaForTesting(
[&](ABI::Windows::Devices::Sensors::ILightSensorStatics** sensor_factory)
-> HRESULT { return fake_sensor_factory.CopyTo(sensor_factory); }));
- EXPECT_TRUE(sensor->Initialize());
+ ASSERT_TRUE(light_sensor->Initialize());
+
+ base::RunLoop run_loop;
+ base::ThreadPool::PostTaskAndReply(
+ FROM_HERE, base::BindLambdaForTesting([&]() {
+ ASSERT_TRUE(light_sensor->StartSensor(
+ PlatformSensorConfiguration(kExpectedReportFrequencySet)));
+ }),
+ run_loop.QuitClosure());
+ run_loop.Run();
+
+ // The idea here is to rely on the fact that TriggerFakeSensorReading() is
+ // asynchronous: we call it while it has a valid handler, it schedules a
+ // task, we destroy the handler object synchronoustly and then it Invoke()s
+ // the callback, which should never reach
+ // CustomLightSensor::OnReadingChangedCallback().
+ Microsoft::WRL::ComPtr<ABI::Windows::Devices::Sensors::ILightSensorReading>
+ reading = Microsoft::WRL::Make<FakeLightSensorReadingWinrt>(
+ ABI::Windows::Foundation::DateTime{}, 0.0f);
+ fake_sensor->TriggerFakeSensorReading(reading);
+ light_sensor.reset();
+ task_environment_.RunUntilIdle();
+}
- auto mock_client = std::make_unique<testing::NiceMock<MockClient>>();
+// Tests that PlatformSensorReaderWinrtBase converts the timestamp correctly
+TEST_F(PlatformSensorReaderTestWinrt, SensorTimestampConversion) {
+ static constexpr double expectedTimestampDeltaSecs = 19.0;
- double lastReportedTimestamp = 0.0;
- EXPECT_CALL(*mock_client, OnReadingUpdated(::testing::_))
- .WillRepeatedly(testing::Invoke([&](const SensorReading& reading) {
- lastReportedTimestamp = reading.als.timestamp;
- EXPECT_EQ(reading.als.value, 0.0f);
- }));
+ auto fake_sensor_factory = Microsoft::WRL::Make<FakeSensorFactoryWinrt<
+ ABI::Windows::Devices::Sensors::ILightSensorStatics,
+ ABI::Windows::Devices::Sensors::ILightSensor,
+ ABI::Windows::Devices::Sensors::LightSensor,
+ ABI::Windows::Devices::Sensors::ILightSensorReading,
+ ABI::Windows::Devices::Sensors::ILightSensorReadingChangedEventArgs,
+ ABI::Windows::Devices::Sensors::LightSensorReadingChangedEventArgs>>();
+ auto fake_sensor = fake_sensor_factory->fake_sensor_;
+
+ auto sensor = CreateAndInitializeSensor<PlatformSensorReaderWinrtLightSensor>(
+ fake_sensor_factory);
+ ASSERT_TRUE(sensor);
+ auto mock_client = std::make_unique<testing::NiceMock<MockClient>>();
sensor->SetClient(mock_client.get());
PlatformSensorConfiguration sensor_config(kExpectedReportFrequencySet);
@@ -672,18 +784,35 @@ TEST_F(PlatformSensorReaderTestWinrt, SensorTimestampConversion) {
Microsoft::WRL::ComPtr<ABI::Windows::Devices::Sensors::ILightSensorReading>
reading = Microsoft::WRL::Make<FakeLightSensorReadingWinrt>(
ABI::Windows::Foundation::DateTime{}, 0.0f);
- fake_sensor->TriggerFakeSensorReading(reading);
- EXPECT_EQ(lastReportedTimestamp, 0);
+ {
+ base::RunLoop run_loop;
+ EXPECT_CALL(*mock_client, OnReadingUpdated(::testing::_))
+ .WillOnce(testing::Invoke([&](const SensorReading& reading) {
+ EXPECT_EQ(reading.als.timestamp, 0.0);
+ EXPECT_EQ(reading.als.value, 0.0f);
+ run_loop.Quit();
+ }));
+ fake_sensor->TriggerFakeSensorReading(reading);
+ run_loop.Run();
+ }
+ // Verify the reported time stamp has ticked forward
+ // expectedTimestampDeltaSecs
auto second_timestamp =
base::Seconds(expectedTimestampDeltaSecs).ToWinrtDateTime();
reading =
Microsoft::WRL::Make<FakeLightSensorReadingWinrt>(second_timestamp, 0.0f);
- fake_sensor->TriggerFakeSensorReading(reading);
-
- // Verify the reported time stamp has ticked forward
- // expectedTimestampDeltaSecs
- EXPECT_EQ(lastReportedTimestamp, expectedTimestampDeltaSecs);
+ {
+ base::RunLoop run_loop;
+ EXPECT_CALL(*mock_client, OnReadingUpdated(::testing::_))
+ .WillOnce(testing::Invoke([&](const SensorReading& reading) {
+ EXPECT_EQ(reading.als.timestamp, expectedTimestampDeltaSecs);
+ EXPECT_EQ(reading.als.value, 0.0f);
+ run_loop.Quit();
+ }));
+ fake_sensor->TriggerFakeSensorReading(reading);
+ run_loop.Run();
+ }
}
// Tests that PlatformSensorReaderWinrtBase starts and stops the
@@ -698,11 +827,9 @@ TEST_F(PlatformSensorReaderTestWinrt, StartStopSensorCallbacks) {
ABI::Windows::Devices::Sensors::LightSensorReadingChangedEventArgs>>();
auto fake_sensor = fake_sensor_factory->fake_sensor_;
- auto sensor = std::make_unique<PlatformSensorReaderWinrtLightSensor>();
- sensor->InitForTesting(base::BindLambdaForTesting(
- [&](ABI::Windows::Devices::Sensors::ILightSensorStatics** sensor_factory)
- -> HRESULT { return fake_sensor_factory.CopyTo(sensor_factory); }));
- EXPECT_TRUE(sensor->Initialize());
+ auto sensor = CreateAndInitializeSensor<PlatformSensorReaderWinrtLightSensor>(
+ fake_sensor_factory);
+ ASSERT_TRUE(sensor);
PlatformSensorConfiguration sensor_config(kExpectedReportFrequencySet);
EXPECT_TRUE(sensor->StartSensor(sensor_config));
@@ -732,17 +859,19 @@ TEST_F(PlatformSensorReaderTestWinrt, StartWithoutStopSensorCallbacks) {
ABI::Windows::Devices::Sensors::LightSensorReadingChangedEventArgs>>();
auto fake_sensor = fake_sensor_factory->fake_sensor_;
- auto sensor = std::make_unique<PlatformSensorReaderWinrtLightSensor>();
- sensor->InitForTesting(base::BindLambdaForTesting(
- [&](ABI::Windows::Devices::Sensors::ILightSensorStatics** sensor_factory)
- -> HRESULT { return fake_sensor_factory.CopyTo(sensor_factory); }));
- EXPECT_TRUE(sensor->Initialize());
+ auto sensor = CreateAndInitializeSensor<PlatformSensorReaderWinrtLightSensor>(
+ fake_sensor_factory);
+ ASSERT_TRUE(sensor);
PlatformSensorConfiguration sensor_config(kExpectedReportFrequencySet);
EXPECT_TRUE(sensor->StartSensor(sensor_config));
EXPECT_TRUE(fake_sensor->IsSensorStarted());
+ // *sensor is deleted in |com_sta_task_runner_|, so we need to wait for it to
+ // happen asynchronously.
sensor.reset();
+ task_environment_.RunUntilIdle();
+
EXPECT_FALSE(fake_sensor->IsSensorStarted());
}
@@ -758,11 +887,9 @@ TEST_F(PlatformSensorReaderTestWinrt, FailedSensorStart) {
ABI::Windows::Devices::Sensors::LightSensorReadingChangedEventArgs>>();
auto fake_sensor = fake_sensor_factory->fake_sensor_;
- auto sensor = std::make_unique<PlatformSensorReaderWinrtLightSensor>();
- sensor->InitForTesting(base::BindLambdaForTesting(
- [&](ABI::Windows::Devices::Sensors::ILightSensorStatics** sensor_factory)
- -> HRESULT { return fake_sensor_factory.CopyTo(sensor_factory); }));
- EXPECT_TRUE(sensor->Initialize());
+ auto sensor = CreateAndInitializeSensor<PlatformSensorReaderWinrtLightSensor>(
+ fake_sensor_factory);
+ ASSERT_TRUE(sensor);
fake_sensor->SetPutReportIntervalReturnCode(E_FAIL);
@@ -787,11 +914,9 @@ TEST_F(PlatformSensorReaderTestWinrt, FailedSensorStop) {
ABI::Windows::Devices::Sensors::LightSensorReadingChangedEventArgs>>();
auto fake_sensor = fake_sensor_factory->fake_sensor_;
- auto sensor = std::make_unique<PlatformSensorReaderWinrtLightSensor>();
- sensor->InitForTesting(base::BindLambdaForTesting(
- [&](ABI::Windows::Devices::Sensors::ILightSensorStatics** sensor_factory)
- -> HRESULT { return fake_sensor_factory.CopyTo(sensor_factory); }));
- EXPECT_TRUE(sensor->Initialize());
+ auto sensor = CreateAndInitializeSensor<PlatformSensorReaderWinrtLightSensor>(
+ fake_sensor_factory);
+ ASSERT_TRUE(sensor);
PlatformSensorConfiguration sensor_config(kExpectedReportFrequencySet);
EXPECT_TRUE(sensor->StartSensor(sensor_config));
@@ -813,11 +938,9 @@ TEST_F(PlatformSensorReaderTestWinrt, FailedLightSensorSampleParse) {
ABI::Windows::Devices::Sensors::LightSensorReadingChangedEventArgs>>();
auto fake_sensor = fake_sensor_factory->fake_sensor_;
- auto sensor = std::make_unique<PlatformSensorReaderWinrtLightSensor>();
- sensor->InitForTesting(base::BindLambdaForTesting(
- [&](ABI::Windows::Devices::Sensors::ILightSensorStatics** sensor_factory)
- -> HRESULT { return fake_sensor_factory.CopyTo(sensor_factory); }));
- EXPECT_TRUE(sensor->Initialize());
+ auto sensor = CreateAndInitializeSensor<PlatformSensorReaderWinrtLightSensor>(
+ fake_sensor_factory);
+ ASSERT_TRUE(sensor);
auto mock_client = std::make_unique<testing::NiceMock<MockClient>>();
@@ -832,12 +955,17 @@ TEST_F(PlatformSensorReaderTestWinrt, FailedLightSensorSampleParse) {
auto reading = Microsoft::WRL::Make<FakeLightSensorReadingWinrt>(
ABI::Windows::Foundation::DateTime{}, 0.0f);
+ // We cannot use a base::RunLoop in the checks below because we are expecting
+ // that MockClient::OnReadingUpdate() does _not_ get called.
+
reading->SetGetTimestampReturnCode(E_FAIL);
fake_sensor->TriggerFakeSensorReading(reading);
reading->SetGetTimestampReturnCode(S_OK);
reading->SetGetIlluminanceInLuxReturnCode(E_FAIL);
fake_sensor->TriggerFakeSensorReading(reading);
+
+ task_environment_.RunUntilIdle();
}
// Tests that PlatformSensorReaderWinrtLightSensor notifies the client
@@ -854,19 +982,11 @@ TEST_F(PlatformSensorReaderTestWinrt, SensorClientNotification) {
ABI::Windows::Devices::Sensors::LightSensorReadingChangedEventArgs>>();
auto fake_sensor = fake_sensor_factory->fake_sensor_;
- auto sensor = std::make_unique<PlatformSensorReaderWinrtLightSensor>();
- sensor->InitForTesting(base::BindLambdaForTesting(
- [&](ABI::Windows::Devices::Sensors::ILightSensorStatics** sensor_factory)
- -> HRESULT { return fake_sensor_factory.CopyTo(sensor_factory); }));
- EXPECT_TRUE(sensor->Initialize());
+ auto sensor = CreateAndInitializeSensor<PlatformSensorReaderWinrtLightSensor>(
+ fake_sensor_factory);
+ ASSERT_TRUE(sensor);
auto mock_client = std::make_unique<testing::NiceMock<MockClient>>();
-
- EXPECT_CALL(*mock_client, OnReadingUpdated(::testing::_))
- .WillOnce(testing::Invoke([&](const SensorReading& reading) {
- EXPECT_EQ(expected_lux, reading.als.value);
- }));
-
sensor->SetClient(mock_client.get());
PlatformSensorConfiguration sensor_config(kExpectedReportFrequencySet);
@@ -874,8 +994,16 @@ TEST_F(PlatformSensorReaderTestWinrt, SensorClientNotification) {
auto reading = Microsoft::WRL::Make<FakeLightSensorReadingWinrt>(
ABI::Windows::Foundation::DateTime{}, expected_lux);
- fake_sensor->TriggerFakeSensorReading(reading);
-
+ {
+ base::RunLoop run_loop;
+ EXPECT_CALL(*mock_client, OnReadingUpdated(::testing::_))
+ .WillOnce(testing::Invoke([&](const SensorReading& reading) {
+ EXPECT_EQ(expected_lux, reading.als.value);
+ run_loop.Quit();
+ }));
+ fake_sensor->TriggerFakeSensorReading(reading);
+ run_loop.Run();
+ }
sensor->StopSensor();
}
@@ -896,30 +1024,31 @@ TEST_F(PlatformSensorReaderTestWinrt, CheckAccelerometerReadingConversion) {
Microsoft::WRL::Make<FakeAccelerometerSensorWinrt>());
auto fake_sensor = fake_sensor_factory->fake_sensor_;
- auto sensor = std::make_unique<PlatformSensorReaderWinrtAccelerometer>();
- sensor->InitForTesting(base::BindLambdaForTesting(
- [&](ABI::Windows::Devices::Sensors::IAccelerometerStatics**
- sensor_factory) -> HRESULT {
- return fake_sensor_factory.CopyTo(sensor_factory);
- }));
- EXPECT_TRUE(sensor->Initialize());
+ auto sensor =
+ CreateAndInitializeSensor<PlatformSensorReaderWinrtAccelerometer>(
+ fake_sensor_factory);
+ ASSERT_TRUE(sensor);
auto mock_client = std::make_unique<testing::NiceMock<MockClient>>();
- EXPECT_CALL(*mock_client, OnReadingUpdated(::testing::_))
- .WillOnce(testing::Invoke([&](const SensorReading& reading) {
- EXPECT_EQ(-expected_x * base::kMeanGravityDouble, reading.accel.x);
- EXPECT_EQ(-expected_y * base::kMeanGravityDouble, reading.accel.y);
- EXPECT_EQ(-expected_z * base::kMeanGravityDouble, reading.accel.z);
- }));
-
sensor->SetClient(mock_client.get());
PlatformSensorConfiguration sensor_config(kExpectedReportFrequencySet);
-
EXPECT_TRUE(sensor->StartSensor(sensor_config));
+
auto reading = Microsoft::WRL::Make<FakeAccelerometerReadingWinrt>(
ABI::Windows::Foundation::DateTime{}, expected_x, expected_y, expected_z);
- fake_sensor->TriggerFakeSensorReading(reading);
+ {
+ base::RunLoop run_loop;
+ EXPECT_CALL(*mock_client, OnReadingUpdated(::testing::_))
+ .WillOnce(testing::Invoke([&](const SensorReading& reading) {
+ EXPECT_EQ(-expected_x * base::kMeanGravityDouble, reading.accel.x);
+ EXPECT_EQ(-expected_y * base::kMeanGravityDouble, reading.accel.y);
+ EXPECT_EQ(-expected_z * base::kMeanGravityDouble, reading.accel.z);
+ run_loop.Quit();
+ }));
+ fake_sensor->TriggerFakeSensorReading(reading);
+ run_loop.Run();
+ }
sensor->StopSensor();
}
@@ -937,13 +1066,10 @@ TEST_F(PlatformSensorReaderTestWinrt, FailedAccelerometerSampleParse) {
Microsoft::WRL::Make<FakeAccelerometerSensorWinrt>());
auto fake_sensor = fake_sensor_factory->fake_sensor_;
- auto sensor = std::make_unique<PlatformSensorReaderWinrtAccelerometer>();
- sensor->InitForTesting(base::BindLambdaForTesting(
- [&](ABI::Windows::Devices::Sensors::IAccelerometerStatics**
- sensor_factory) -> HRESULT {
- return fake_sensor_factory.CopyTo(sensor_factory);
- }));
- EXPECT_TRUE(sensor->Initialize());
+ auto sensor =
+ CreateAndInitializeSensor<PlatformSensorReaderWinrtAccelerometer>(
+ fake_sensor_factory);
+ ASSERT_TRUE(sensor);
auto mock_client = std::make_unique<testing::NiceMock<MockClient>>();
sensor->SetClient(mock_client.get());
@@ -954,6 +1080,9 @@ TEST_F(PlatformSensorReaderTestWinrt, FailedAccelerometerSampleParse) {
auto reading = Microsoft::WRL::Make<FakeAccelerometerReadingWinrt>(
ABI::Windows::Foundation::DateTime{}, 0, 0, 0);
+ // We cannot use a base::RunLoop in the checks below because we are expecting
+ // that MockClient::OnReadingUpdate() does _not_ get called.
+
reading->SetGetTimestampReturnCode(E_FAIL);
fake_sensor->TriggerFakeSensorReading(reading);
@@ -968,6 +1097,8 @@ TEST_F(PlatformSensorReaderTestWinrt, FailedAccelerometerSampleParse) {
reading->SetGetYReturnCode(S_OK);
reading->SetGetZReturnCode(E_FAIL);
fake_sensor->TriggerFakeSensorReading(reading);
+
+ task_environment_.RunUntilIdle();
}
// Tests if PlatformSensorReaderWinrtGyrometer correctly converts sensor
@@ -986,27 +1117,30 @@ TEST_F(PlatformSensorReaderTestWinrt, CheckGyrometerReadingConversion) {
ABI::Windows::Devices::Sensors::GyrometerReadingChangedEventArgs>>();
auto fake_sensor = fake_sensor_factory->fake_sensor_;
- auto sensor = std::make_unique<PlatformSensorReaderWinrtGyrometer>();
- sensor->InitForTesting(base::BindLambdaForTesting(
- [&](ABI::Windows::Devices::Sensors::IGyrometerStatics** sensor_factory)
- -> HRESULT { return fake_sensor_factory.CopyTo(sensor_factory); }));
- EXPECT_TRUE(sensor->Initialize());
+ auto sensor = CreateAndInitializeSensor<PlatformSensorReaderWinrtGyrometer>(
+ fake_sensor_factory);
+ ASSERT_TRUE(sensor);
auto mock_client = std::make_unique<testing::NiceMock<MockClient>>();
- EXPECT_CALL(*mock_client, OnReadingUpdated(::testing::_))
- .WillOnce(testing::Invoke([&](const SensorReading& reading) {
- EXPECT_EQ(gfx::DegToRad(expected_x), reading.gyro.x);
- EXPECT_EQ(gfx::DegToRad(expected_y), reading.gyro.y);
- EXPECT_EQ(gfx::DegToRad(expected_z), reading.gyro.z);
- }));
-
sensor->SetClient(mock_client.get());
+
PlatformSensorConfiguration sensor_config(kExpectedReportFrequencySet);
EXPECT_TRUE(sensor->StartSensor(sensor_config));
auto reading = Microsoft::WRL::Make<FakeGyrometerReadingWinrt>(
ABI::Windows::Foundation::DateTime{}, expected_x, expected_y, expected_z);
- fake_sensor->TriggerFakeSensorReading(reading);
+ {
+ base::RunLoop run_loop;
+ EXPECT_CALL(*mock_client, OnReadingUpdated(::testing::_))
+ .WillOnce(testing::Invoke([&](const SensorReading& reading) {
+ EXPECT_EQ(gfx::DegToRad(expected_x), reading.gyro.x);
+ EXPECT_EQ(gfx::DegToRad(expected_y), reading.gyro.y);
+ EXPECT_EQ(gfx::DegToRad(expected_z), reading.gyro.z);
+ run_loop.Quit();
+ }));
+ fake_sensor->TriggerFakeSensorReading(reading);
+ run_loop.Run();
+ }
sensor->StopSensor();
}
@@ -1023,11 +1157,9 @@ TEST_F(PlatformSensorReaderTestWinrt, FailedGyrometerSampleParse) {
ABI::Windows::Devices::Sensors::GyrometerReadingChangedEventArgs>>();
auto fake_sensor = fake_sensor_factory->fake_sensor_;
- auto sensor = std::make_unique<PlatformSensorReaderWinrtGyrometer>();
- sensor->InitForTesting(base::BindLambdaForTesting(
- [&](ABI::Windows::Devices::Sensors::IGyrometerStatics** sensor_factory)
- -> HRESULT { return fake_sensor_factory.CopyTo(sensor_factory); }));
- EXPECT_TRUE(sensor->Initialize());
+ auto sensor = CreateAndInitializeSensor<PlatformSensorReaderWinrtGyrometer>(
+ fake_sensor_factory);
+ ASSERT_TRUE(sensor);
auto mock_client = std::make_unique<testing::NiceMock<MockClient>>();
sensor->SetClient(mock_client.get());
@@ -1038,6 +1170,9 @@ TEST_F(PlatformSensorReaderTestWinrt, FailedGyrometerSampleParse) {
auto reading = Microsoft::WRL::Make<FakeGyrometerReadingWinrt>(
ABI::Windows::Foundation::DateTime{}, 0, 0, 0);
+ // We cannot use a base::RunLoop in the checks below because we are expecting
+ // that MockClient::OnReadingUpdate() does _not_ get called.
+
reading->SetGetTimestampReturnCode(E_FAIL);
fake_sensor->TriggerFakeSensorReading(reading);
@@ -1052,6 +1187,8 @@ TEST_F(PlatformSensorReaderTestWinrt, FailedGyrometerSampleParse) {
reading->SetGetYReturnCode(S_OK);
reading->SetGetZReturnCode(E_FAIL);
fake_sensor->TriggerFakeSensorReading(reading);
+
+ task_environment_.RunUntilIdle();
}
// Tests if PlatformSensorReaderWinrtMagnetometer correctly converts sensor
@@ -1070,27 +1207,31 @@ TEST_F(PlatformSensorReaderTestWinrt, CheckMagnetometerReadingConversion) {
ABI::Windows::Devices::Sensors::MagnetometerReadingChangedEventArgs>>();
auto fake_sensor = fake_sensor_factory->fake_sensor_;
- auto sensor = std::make_unique<PlatformSensorReaderWinrtMagnetometer>();
- sensor->InitForTesting(base::BindLambdaForTesting(
- [&](ABI::Windows::Devices::Sensors::IMagnetometerStatics** sensor_factory)
- -> HRESULT { return fake_sensor_factory.CopyTo(sensor_factory); }));
- EXPECT_TRUE(sensor->Initialize());
+ auto sensor =
+ CreateAndInitializeSensor<PlatformSensorReaderWinrtMagnetometer>(
+ fake_sensor_factory);
+ ASSERT_TRUE(sensor);
auto mock_client = std::make_unique<testing::NiceMock<MockClient>>();
- EXPECT_CALL(*mock_client, OnReadingUpdated(::testing::_))
- .WillOnce(testing::Invoke([&](const SensorReading& reading) {
- EXPECT_EQ(expected_x, reading.magn.x);
- EXPECT_EQ(expected_y, reading.magn.y);
- EXPECT_EQ(expected_z, reading.magn.z);
- }));
-
sensor->SetClient(mock_client.get());
+
PlatformSensorConfiguration sensor_config(kExpectedReportFrequencySet);
EXPECT_TRUE(sensor->StartSensor(sensor_config));
auto reading = Microsoft::WRL::Make<FakeMagnetometerReadingWinrt>(
ABI::Windows::Foundation::DateTime{}, expected_x, expected_y, expected_z);
- fake_sensor->TriggerFakeSensorReading(reading);
+ {
+ base::RunLoop run_loop;
+ EXPECT_CALL(*mock_client, OnReadingUpdated(::testing::_))
+ .WillOnce(testing::Invoke([&](const SensorReading& reading) {
+ EXPECT_EQ(expected_x, reading.magn.x);
+ EXPECT_EQ(expected_y, reading.magn.y);
+ EXPECT_EQ(expected_z, reading.magn.z);
+ run_loop.Quit();
+ }));
+ fake_sensor->TriggerFakeSensorReading(reading);
+ run_loop.Run();
+ }
sensor->StopSensor();
}
@@ -1107,11 +1248,10 @@ TEST_F(PlatformSensorReaderTestWinrt, FailedMagnetometerSampleParse) {
ABI::Windows::Devices::Sensors::MagnetometerReadingChangedEventArgs>>();
auto fake_sensor = fake_sensor_factory->fake_sensor_;
- auto sensor = std::make_unique<PlatformSensorReaderWinrtMagnetometer>();
- sensor->InitForTesting(base::BindLambdaForTesting(
- [&](ABI::Windows::Devices::Sensors::IMagnetometerStatics** sensor_factory)
- -> HRESULT { return fake_sensor_factory.CopyTo(sensor_factory); }));
- EXPECT_TRUE(sensor->Initialize());
+ auto sensor =
+ CreateAndInitializeSensor<PlatformSensorReaderWinrtMagnetometer>(
+ fake_sensor_factory);
+ ASSERT_TRUE(sensor);
auto mock_client = std::make_unique<testing::NiceMock<MockClient>>();
sensor->SetClient(mock_client.get());
@@ -1122,6 +1262,9 @@ TEST_F(PlatformSensorReaderTestWinrt, FailedMagnetometerSampleParse) {
auto reading = Microsoft::WRL::Make<FakeMagnetometerReadingWinrt>(
ABI::Windows::Foundation::DateTime{}, 0, 0, 0);
+ // We cannot use a base::RunLoop in the checks below because we are expecting
+ // that MockClient::OnReadingUpdate() does _not_ get called.
+
reading->SetGetTimestampReturnCode(E_FAIL);
fake_sensor->TriggerFakeSensorReading(reading);
@@ -1136,6 +1279,8 @@ TEST_F(PlatformSensorReaderTestWinrt, FailedMagnetometerSampleParse) {
reading->SetGetYReturnCode(S_OK);
reading->SetGetZReturnCode(E_FAIL);
fake_sensor->TriggerFakeSensorReading(reading);
+
+ task_environment_.RunUntilIdle();
}
// Tests if PlatformSensorReaderWinrtAbsOrientationEulerAngles correctly
@@ -1154,28 +1299,30 @@ TEST_F(PlatformSensorReaderTestWinrt, CheckInclinometerReadingConversion) {
ABI::Windows::Devices::Sensors::InclinometerReadingChangedEventArgs>>();
auto fake_sensor = fake_sensor_factory->fake_sensor_;
- auto sensor =
- std::make_unique<PlatformSensorReaderWinrtAbsOrientationEulerAngles>();
- sensor->InitForTesting(base::BindLambdaForTesting(
- [&](ABI::Windows::Devices::Sensors::IInclinometerStatics** sensor_factory)
- -> HRESULT { return fake_sensor_factory.CopyTo(sensor_factory); }));
- EXPECT_TRUE(sensor->Initialize());
+ auto sensor = CreateAndInitializeSensor<
+ PlatformSensorReaderWinrtAbsOrientationEulerAngles>(fake_sensor_factory);
+ ASSERT_TRUE(sensor);
auto mock_client = std::make_unique<testing::NiceMock<MockClient>>();
- EXPECT_CALL(*mock_client, OnReadingUpdated(::testing::_))
- .WillOnce(testing::Invoke([&](const SensorReading& reading) {
- EXPECT_EQ(expected_x, reading.orientation_euler.x);
- EXPECT_EQ(expected_y, reading.orientation_euler.y);
- EXPECT_EQ(expected_z, reading.orientation_euler.z);
- }));
-
sensor->SetClient(mock_client.get());
+
PlatformSensorConfiguration sensor_config(kExpectedReportFrequencySet);
EXPECT_TRUE(sensor->StartSensor(sensor_config));
auto reading = Microsoft::WRL::Make<FakeInclinometerReadingWinrt>(
ABI::Windows::Foundation::DateTime{}, expected_x, expected_y, expected_z);
- fake_sensor->TriggerFakeSensorReading(reading);
+ {
+ base::RunLoop run_loop;
+ EXPECT_CALL(*mock_client, OnReadingUpdated(::testing::_))
+ .WillOnce(testing::Invoke([&](const SensorReading& reading) {
+ EXPECT_EQ(expected_x, reading.orientation_euler.x);
+ EXPECT_EQ(expected_y, reading.orientation_euler.y);
+ EXPECT_EQ(expected_z, reading.orientation_euler.z);
+ run_loop.Quit();
+ }));
+ fake_sensor->TriggerFakeSensorReading(reading);
+ run_loop.Run();
+ }
sensor->StopSensor();
}
@@ -1192,12 +1339,9 @@ TEST_F(PlatformSensorReaderTestWinrt, FailedInclinometerSampleParse) {
ABI::Windows::Devices::Sensors::InclinometerReadingChangedEventArgs>>();
auto fake_sensor = fake_sensor_factory->fake_sensor_;
- auto sensor =
- std::make_unique<PlatformSensorReaderWinrtAbsOrientationEulerAngles>();
- sensor->InitForTesting(base::BindLambdaForTesting(
- [&](ABI::Windows::Devices::Sensors::IInclinometerStatics** sensor_factory)
- -> HRESULT { return fake_sensor_factory.CopyTo(sensor_factory); }));
- EXPECT_TRUE(sensor->Initialize());
+ auto sensor = CreateAndInitializeSensor<
+ PlatformSensorReaderWinrtAbsOrientationEulerAngles>(fake_sensor_factory);
+ ASSERT_TRUE(sensor);
auto mock_client = std::make_unique<testing::NiceMock<MockClient>>();
sensor->SetClient(mock_client.get());
@@ -1208,6 +1352,9 @@ TEST_F(PlatformSensorReaderTestWinrt, FailedInclinometerSampleParse) {
auto reading = Microsoft::WRL::Make<FakeInclinometerReadingWinrt>(
ABI::Windows::Foundation::DateTime{}, 0, 0, 0);
+ // We cannot use a base::RunLoop in the checks below because we are expecting
+ // that MockClient::OnReadingUpdate() does _not_ get called.
+
reading->SetGetTimestampReturnCode(E_FAIL);
fake_sensor->TriggerFakeSensorReading(reading);
@@ -1222,6 +1369,8 @@ TEST_F(PlatformSensorReaderTestWinrt, FailedInclinometerSampleParse) {
reading->SetGetYReturnCode(S_OK);
reading->SetGetZReturnCode(E_FAIL);
fake_sensor->TriggerFakeSensorReading(reading);
+
+ task_environment_.RunUntilIdle();
}
// Tests if PlatformSensorReaderWinrtAbsOrientationQuaternion correctly
@@ -1242,32 +1391,32 @@ TEST_F(PlatformSensorReaderTestWinrt, CheckOrientationSensorReadingConversion) {
OrientationSensorReadingChangedEventArgs>>();
auto fake_sensor = fake_sensor_factory->fake_sensor_;
- auto sensor =
- std::make_unique<PlatformSensorReaderWinrtAbsOrientationQuaternion>();
- sensor->InitForTesting(base::BindLambdaForTesting(
- [&](ABI::Windows::Devices::Sensors::IOrientationSensorStatics**
- sensor_factory) -> HRESULT {
- return fake_sensor_factory.CopyTo(sensor_factory);
- }));
- EXPECT_TRUE(sensor->Initialize());
+ auto sensor = CreateAndInitializeSensor<
+ PlatformSensorReaderWinrtAbsOrientationQuaternion>(fake_sensor_factory);
+ ASSERT_TRUE(sensor);
auto mock_client = std::make_unique<testing::NiceMock<MockClient>>();
- EXPECT_CALL(*mock_client, OnReadingUpdated(::testing::_))
- .WillOnce(testing::Invoke([&](const SensorReading& reading) {
- EXPECT_EQ(expected_w, reading.orientation_quat.w);
- EXPECT_EQ(expected_x, reading.orientation_quat.x);
- EXPECT_EQ(expected_y, reading.orientation_quat.y);
- EXPECT_EQ(expected_z, reading.orientation_quat.z);
- }));
-
sensor->SetClient(mock_client.get());
+
PlatformSensorConfiguration sensor_config(kExpectedReportFrequencySet);
EXPECT_TRUE(sensor->StartSensor(sensor_config));
auto reading = Microsoft::WRL::Make<FakeOrientationSensorReadingWinrt>(
ABI::Windows::Foundation::DateTime{}, expected_w, expected_x, expected_y,
expected_z);
- fake_sensor->TriggerFakeSensorReading(reading);
+ {
+ base::RunLoop run_loop;
+ EXPECT_CALL(*mock_client, OnReadingUpdated(::testing::_))
+ .WillOnce(testing::Invoke([&](const SensorReading& reading) {
+ EXPECT_EQ(expected_w, reading.orientation_quat.w);
+ EXPECT_EQ(expected_x, reading.orientation_quat.x);
+ EXPECT_EQ(expected_y, reading.orientation_quat.y);
+ EXPECT_EQ(expected_z, reading.orientation_quat.z);
+ run_loop.Quit();
+ }));
+ fake_sensor->TriggerFakeSensorReading(reading);
+ run_loop.Run();
+ }
sensor->StopSensor();
}
@@ -1285,14 +1434,9 @@ TEST_F(PlatformSensorReaderTestWinrt, FailedOrientationSampleParse) {
OrientationSensorReadingChangedEventArgs>>();
auto fake_sensor = fake_sensor_factory->fake_sensor_;
- auto sensor =
- std::make_unique<PlatformSensorReaderWinrtAbsOrientationQuaternion>();
- sensor->InitForTesting(base::BindLambdaForTesting(
- [&](ABI::Windows::Devices::Sensors::IOrientationSensorStatics**
- sensor_factory) -> HRESULT {
- return fake_sensor_factory.CopyTo(sensor_factory);
- }));
- EXPECT_TRUE(sensor->Initialize());
+ auto sensor = CreateAndInitializeSensor<
+ PlatformSensorReaderWinrtAbsOrientationQuaternion>(fake_sensor_factory);
+ ASSERT_TRUE(sensor);
auto mock_client = std::make_unique<testing::NiceMock<MockClient>>();
sensor->SetClient(mock_client.get());
@@ -1303,12 +1447,17 @@ TEST_F(PlatformSensorReaderTestWinrt, FailedOrientationSampleParse) {
auto reading = Microsoft::WRL::Make<FakeOrientationSensorReadingWinrt>(
ABI::Windows::Foundation::DateTime{}, 0, 0, 0, 0);
+ // We cannot use a base::RunLoop in the checks below because we are expecting
+ // that MockClient::OnReadingUpdate() does _not_ get called.
+
reading->SetGetTimestampReturnCode(E_FAIL);
fake_sensor->TriggerFakeSensorReading(reading);
reading->SetGetTimestampReturnCode(S_OK);
reading->SetGetQuaternionReturnCode(E_FAIL);
fake_sensor->TriggerFakeSensorReading(reading);
+
+ task_environment_.RunUntilIdle();
}
TEST_F(PlatformSensorReaderTestWinrt, LightSensorThresholding) {
@@ -1321,29 +1470,30 @@ TEST_F(PlatformSensorReaderTestWinrt, LightSensorThresholding) {
ABI::Windows::Devices::Sensors::LightSensorReadingChangedEventArgs>>();
auto fake_sensor = fake_sensor_factory->fake_sensor_;
- auto sensor = std::make_unique<PlatformSensorReaderWinrtLightSensor>();
- sensor->InitForTesting(base::BindLambdaForTesting(
- [&](ABI::Windows::Devices::Sensors::ILightSensorStatics** sensor_factory)
- -> HRESULT { return fake_sensor_factory.CopyTo(sensor_factory); }));
- EXPECT_TRUE(sensor->Initialize());
+ auto sensor = CreateAndInitializeSensor<PlatformSensorReaderWinrtLightSensor>(
+ fake_sensor_factory);
+ ASSERT_TRUE(sensor);
auto mock_client = std::make_unique<testing::NiceMock<MockClient>>();
-
- bool expected_callback = false;
- EXPECT_CALL(*mock_client, OnReadingUpdated(::testing::_))
- .WillRepeatedly(testing::Invoke(
- [&](const SensorReading&) { EXPECT_TRUE(expected_callback); }));
-
sensor->SetClient(mock_client.get());
+
PlatformSensorConfiguration sensor_config(kExpectedReportFrequencySet);
EXPECT_TRUE(sensor->StartSensor(sensor_config));
float last_sent_lux = 1.0f;
auto threshold_helper = [&](bool expect_callback) {
- expected_callback = expect_callback;
auto reading = Microsoft::WRL::Make<FakeLightSensorReadingWinrt>(
ABI::Windows::Foundation::DateTime{}, last_sent_lux);
- fake_sensor->TriggerFakeSensorReading(reading);
+ if (expect_callback) {
+ base::RunLoop run_loop;
+ EXPECT_CALL(*mock_client, OnReadingUpdated(::testing::_))
+ .WillOnce(base::test::RunClosure(run_loop.QuitClosure()));
+ fake_sensor->TriggerFakeSensorReading(reading);
+ run_loop.Run();
+ } else {
+ fake_sensor->TriggerFakeSensorReading(reading);
+ task_environment_.RunUntilIdle();
+ }
};
// Expect callback, first sample
@@ -1359,10 +1509,6 @@ TEST_F(PlatformSensorReaderTestWinrt, LightSensorThresholding) {
PlatformSensorReaderWinrtLightSensor::kLuxPercentThreshold * 0.6f;
threshold_helper(true);
- // Expect callback, threshold has been met exactly
- last_sent_lux += PlatformSensorReaderWinrtLightSensor::kLuxPercentThreshold;
- threshold_helper(true);
-
sensor->StopSensor();
}
@@ -1377,22 +1523,14 @@ TEST_F(PlatformSensorReaderTestWinrt, AccelerometerThresholding) {
Microsoft::WRL::Make<FakeAccelerometerSensorWinrt>());
auto fake_sensor = fake_sensor_factory->fake_sensor_;
- auto sensor = std::make_unique<PlatformSensorReaderWinrtAccelerometer>();
- sensor->InitForTesting(base::BindLambdaForTesting(
- [&](ABI::Windows::Devices::Sensors::IAccelerometerStatics**
- sensor_factory) -> HRESULT {
- return fake_sensor_factory.CopyTo(sensor_factory);
- }));
- EXPECT_TRUE(sensor->Initialize());
+ auto sensor =
+ CreateAndInitializeSensor<PlatformSensorReaderWinrtAccelerometer>(
+ fake_sensor_factory);
+ ASSERT_TRUE(sensor);
auto mock_client = std::make_unique<testing::NiceMock<MockClient>>();
-
- bool expected_callback = false;
- EXPECT_CALL(*mock_client, OnReadingUpdated(::testing::_))
- .WillRepeatedly(testing::Invoke(
- [&](const SensorReading&) { EXPECT_TRUE(expected_callback); }));
-
sensor->SetClient(mock_client.get());
+
PlatformSensorConfiguration sensor_config(kExpectedReportFrequencySet);
EXPECT_TRUE(sensor->StartSensor(sensor_config));
@@ -1400,40 +1538,40 @@ TEST_F(PlatformSensorReaderTestWinrt, AccelerometerThresholding) {
double last_sent_y = 2.0f;
double last_sent_z = 3.0f;
auto threshold_helper = [&](bool expect_callback) {
- expected_callback = expect_callback;
auto reading = Microsoft::WRL::Make<FakeAccelerometerReadingWinrt>(
ABI::Windows::Foundation::DateTime{}, last_sent_x, last_sent_y,
last_sent_z);
- fake_sensor->TriggerFakeSensorReading(reading);
+ if (expect_callback) {
+ base::RunLoop run_loop;
+ EXPECT_CALL(*mock_client, OnReadingUpdated(::testing::_))
+ .WillOnce(base::test::RunClosure(run_loop.QuitClosure()));
+ fake_sensor->TriggerFakeSensorReading(reading);
+ run_loop.Run();
+ } else {
+ fake_sensor->TriggerFakeSensorReading(reading);
+ task_environment_.RunUntilIdle();
+ }
};
// Expect callback, first sample
threshold_helper(true);
- // No callback, threshold has not been met
+ // For each axis, increase its value by an amount lower than the threshold so
+ // that no callback is invoked, then meet the threshold and do expect a
+ // callback.
last_sent_x += PlatformSensorReaderWinrtAccelerometer::kAxisThreshold * 0.5f;
threshold_helper(false);
- last_sent_y += PlatformSensorReaderWinrtAccelerometer::kAxisThreshold * 0.5f;
- threshold_helper(false);
- last_sent_z += PlatformSensorReaderWinrtAccelerometer::kAxisThreshold * 0.5f;
- threshold_helper(false);
-
- // Expect callback, threshold has been met since last reported sample
last_sent_x += PlatformSensorReaderWinrtAccelerometer::kAxisThreshold * 0.6f;
threshold_helper(true);
+ last_sent_y += PlatformSensorReaderWinrtAccelerometer::kAxisThreshold * 0.5f;
+ threshold_helper(false);
last_sent_y += PlatformSensorReaderWinrtAccelerometer::kAxisThreshold * 0.6f;
threshold_helper(true);
+ last_sent_z += PlatformSensorReaderWinrtAccelerometer::kAxisThreshold * 0.5f;
+ threshold_helper(false);
last_sent_z += PlatformSensorReaderWinrtAccelerometer::kAxisThreshold * 0.6f;
threshold_helper(true);
- // Expect callback, threshold has been met exactly
- last_sent_x += PlatformSensorReaderWinrtAccelerometer::kAxisThreshold;
- threshold_helper(true);
- last_sent_y += PlatformSensorReaderWinrtAccelerometer::kAxisThreshold;
- threshold_helper(true);
- last_sent_z += PlatformSensorReaderWinrtAccelerometer::kAxisThreshold;
- threshold_helper(true);
-
sensor->StopSensor();
}
@@ -1447,20 +1585,13 @@ TEST_F(PlatformSensorReaderTestWinrt, GyrometerThresholding) {
ABI::Windows::Devices::Sensors::GyrometerReadingChangedEventArgs>>();
auto fake_sensor = fake_sensor_factory->fake_sensor_;
- auto sensor = std::make_unique<PlatformSensorReaderWinrtGyrometer>();
- sensor->InitForTesting(base::BindLambdaForTesting(
- [&](ABI::Windows::Devices::Sensors::IGyrometerStatics** sensor_factory)
- -> HRESULT { return fake_sensor_factory.CopyTo(sensor_factory); }));
- EXPECT_TRUE(sensor->Initialize());
+ auto sensor = CreateAndInitializeSensor<PlatformSensorReaderWinrtGyrometer>(
+ fake_sensor_factory);
+ ASSERT_TRUE(sensor);
auto mock_client = std::make_unique<testing::NiceMock<MockClient>>();
-
- bool expected_callback = false;
- EXPECT_CALL(*mock_client, OnReadingUpdated(::testing::_))
- .WillRepeatedly(testing::Invoke(
- [&](const SensorReading&) { EXPECT_TRUE(expected_callback); }));
-
sensor->SetClient(mock_client.get());
+
PlatformSensorConfiguration sensor_config(kExpectedReportFrequencySet);
EXPECT_TRUE(sensor->StartSensor(sensor_config));
@@ -1468,40 +1599,40 @@ TEST_F(PlatformSensorReaderTestWinrt, GyrometerThresholding) {
double last_sent_y = 4.0f;
double last_sent_z = 5.0f;
auto threshold_helper = [&](bool expect_callback) {
- expected_callback = expect_callback;
auto reading = Microsoft::WRL::Make<FakeGyrometerReadingWinrt>(
ABI::Windows::Foundation::DateTime{}, last_sent_x, last_sent_y,
last_sent_z);
- fake_sensor->TriggerFakeSensorReading(reading);
+ if (expect_callback) {
+ base::RunLoop run_loop;
+ EXPECT_CALL(*mock_client, OnReadingUpdated(::testing::_))
+ .WillOnce(base::test::RunClosure(run_loop.QuitClosure()));
+ fake_sensor->TriggerFakeSensorReading(reading);
+ run_loop.Run();
+ } else {
+ fake_sensor->TriggerFakeSensorReading(reading);
+ task_environment_.RunUntilIdle();
+ }
};
// Expect callback, first sample
threshold_helper(true);
- // No callback, threshold has not been met
+ // For each axis, increase its value by an amount lower than the threshold so
+ // that no callback is invoked, then meet the threshold and do expect a
+ // callback.
last_sent_x += PlatformSensorReaderWinrtGyrometer::kDegreeThreshold * 0.5f;
threshold_helper(false);
- last_sent_y += PlatformSensorReaderWinrtGyrometer::kDegreeThreshold * 0.5f;
- threshold_helper(false);
- last_sent_z += PlatformSensorReaderWinrtGyrometer::kDegreeThreshold * 0.5f;
- threshold_helper(false);
-
- // Expect callback, threshold has been met since last reported sample
last_sent_x += PlatformSensorReaderWinrtGyrometer::kDegreeThreshold * 0.6f;
threshold_helper(true);
+ last_sent_y += PlatformSensorReaderWinrtGyrometer::kDegreeThreshold * 0.5f;
+ threshold_helper(false);
last_sent_y += PlatformSensorReaderWinrtGyrometer::kDegreeThreshold * 0.6f;
threshold_helper(true);
+ last_sent_z += PlatformSensorReaderWinrtGyrometer::kDegreeThreshold * 0.5f;
+ threshold_helper(false);
last_sent_z += PlatformSensorReaderWinrtGyrometer::kDegreeThreshold * 0.6f;
threshold_helper(true);
- // Expect callback, threshold has been met exactly
- last_sent_x += PlatformSensorReaderWinrtGyrometer::kDegreeThreshold;
- threshold_helper(true);
- last_sent_y += PlatformSensorReaderWinrtGyrometer::kDegreeThreshold;
- threshold_helper(true);
- last_sent_z += PlatformSensorReaderWinrtGyrometer::kDegreeThreshold;
- threshold_helper(true);
-
sensor->StopSensor();
}
@@ -1515,20 +1646,14 @@ TEST_F(PlatformSensorReaderTestWinrt, MagnetometerThresholding) {
ABI::Windows::Devices::Sensors::MagnetometerReadingChangedEventArgs>>();
auto fake_sensor = fake_sensor_factory->fake_sensor_;
- auto sensor = std::make_unique<PlatformSensorReaderWinrtMagnetometer>();
- sensor->InitForTesting(base::BindLambdaForTesting(
- [&](ABI::Windows::Devices::Sensors::IMagnetometerStatics** sensor_factory)
- -> HRESULT { return fake_sensor_factory.CopyTo(sensor_factory); }));
- EXPECT_TRUE(sensor->Initialize());
+ auto sensor =
+ CreateAndInitializeSensor<PlatformSensorReaderWinrtMagnetometer>(
+ fake_sensor_factory);
+ ASSERT_TRUE(sensor);
auto mock_client = std::make_unique<testing::NiceMock<MockClient>>();
-
- bool expected_callback = false;
- EXPECT_CALL(*mock_client, OnReadingUpdated(::testing::_))
- .WillRepeatedly(testing::Invoke(
- [&](const SensorReading&) { EXPECT_TRUE(expected_callback); }));
-
sensor->SetClient(mock_client.get());
+
PlatformSensorConfiguration sensor_config(kExpectedReportFrequencySet);
EXPECT_TRUE(sensor->StartSensor(sensor_config));
@@ -1536,46 +1661,46 @@ TEST_F(PlatformSensorReaderTestWinrt, MagnetometerThresholding) {
double last_sent_y = 4.0f;
double last_sent_z = 5.0f;
auto threshold_helper = [&](bool expect_callback) {
- expected_callback = expect_callback;
auto reading = Microsoft::WRL::Make<FakeMagnetometerReadingWinrt>(
ABI::Windows::Foundation::DateTime{}, last_sent_x, last_sent_y,
last_sent_z);
- fake_sensor->TriggerFakeSensorReading(reading);
+ if (expect_callback) {
+ base::RunLoop run_loop;
+ EXPECT_CALL(*mock_client, OnReadingUpdated(::testing::_))
+ .WillOnce(base::test::RunClosure(run_loop.QuitClosure()));
+ fake_sensor->TriggerFakeSensorReading(reading);
+ run_loop.Run();
+ } else {
+ fake_sensor->TriggerFakeSensorReading(reading);
+ task_environment_.RunUntilIdle();
+ }
};
// Expect callback, first sample
threshold_helper(true);
- // No callback, threshold has not been met
+ // For each axis, increase its value by an amount lower than the threshold so
+ // that no callback is invoked, then meet the threshold and do expect a
+ // callback.
last_sent_x +=
PlatformSensorReaderWinrtMagnetometer::kMicroteslaThreshold * 0.5f;
threshold_helper(false);
- last_sent_y +=
- PlatformSensorReaderWinrtMagnetometer::kMicroteslaThreshold * 0.5f;
- threshold_helper(false);
- last_sent_z +=
- PlatformSensorReaderWinrtMagnetometer::kMicroteslaThreshold * 0.5f;
- threshold_helper(false);
-
- // Expect callback, threshold has been met since last reported sample
last_sent_x +=
PlatformSensorReaderWinrtMagnetometer::kMicroteslaThreshold * 0.6f;
threshold_helper(true);
+ last_sent_y +=
+ PlatformSensorReaderWinrtMagnetometer::kMicroteslaThreshold * 0.5f;
+ threshold_helper(false);
last_sent_y +=
PlatformSensorReaderWinrtMagnetometer::kMicroteslaThreshold * 0.6f;
threshold_helper(true);
+ last_sent_z +=
+ PlatformSensorReaderWinrtMagnetometer::kMicroteslaThreshold * 0.5f;
+ threshold_helper(false);
last_sent_z +=
PlatformSensorReaderWinrtMagnetometer::kMicroteslaThreshold * 0.6f;
threshold_helper(true);
- // Expect callback, threshold has been met exactly
- last_sent_x += PlatformSensorReaderWinrtMagnetometer::kMicroteslaThreshold;
- threshold_helper(true);
- last_sent_y += PlatformSensorReaderWinrtMagnetometer::kMicroteslaThreshold;
- threshold_helper(true);
- last_sent_z += PlatformSensorReaderWinrtMagnetometer::kMicroteslaThreshold;
- threshold_helper(true);
-
sensor->StopSensor();
}
@@ -1589,21 +1714,13 @@ TEST_F(PlatformSensorReaderTestWinrt, AbsOrientationEulerThresholding) {
ABI::Windows::Devices::Sensors::InclinometerReadingChangedEventArgs>>();
auto fake_sensor = fake_sensor_factory->fake_sensor_;
- auto sensor =
- std::make_unique<PlatformSensorReaderWinrtAbsOrientationEulerAngles>();
- sensor->InitForTesting(base::BindLambdaForTesting(
- [&](ABI::Windows::Devices::Sensors::IInclinometerStatics** sensor_factory)
- -> HRESULT { return fake_sensor_factory.CopyTo(sensor_factory); }));
- EXPECT_TRUE(sensor->Initialize());
+ auto sensor = CreateAndInitializeSensor<
+ PlatformSensorReaderWinrtAbsOrientationEulerAngles>(fake_sensor_factory);
+ ASSERT_TRUE(sensor);
auto mock_client = std::make_unique<testing::NiceMock<MockClient>>();
-
- bool expected_callback = false;
- EXPECT_CALL(*mock_client, OnReadingUpdated(::testing::_))
- .WillRepeatedly(testing::Invoke(
- [&](const SensorReading&) { EXPECT_TRUE(expected_callback); }));
-
sensor->SetClient(mock_client.get());
+
PlatformSensorConfiguration sensor_config(kExpectedReportFrequencySet);
EXPECT_TRUE(sensor->StartSensor(sensor_config));
@@ -1611,53 +1728,50 @@ TEST_F(PlatformSensorReaderTestWinrt, AbsOrientationEulerThresholding) {
double last_sent_y = 4.0f;
double last_sent_z = 5.0f;
auto threshold_helper = [&](bool expect_callback) {
- expected_callback = expect_callback;
auto reading = Microsoft::WRL::Make<FakeInclinometerReadingWinrt>(
ABI::Windows::Foundation::DateTime{}, last_sent_x, last_sent_y,
last_sent_z);
- fake_sensor->TriggerFakeSensorReading(reading);
+ if (expect_callback) {
+ base::RunLoop run_loop;
+ EXPECT_CALL(*mock_client, OnReadingUpdated(::testing::_))
+ .WillOnce(base::test::RunClosure(run_loop.QuitClosure()));
+ fake_sensor->TriggerFakeSensorReading(reading);
+ run_loop.Run();
+ } else {
+ fake_sensor->TriggerFakeSensorReading(reading);
+ task_environment_.RunUntilIdle();
+ }
};
// Expect callback, first sample
threshold_helper(true);
- // No callback, threshold has not been met
+ // For each axis, increase its value by an amount lower than the threshold so
+ // that no callback is invoked, then meet the threshold and do expect a
+ // callback.
last_sent_x +=
PlatformSensorReaderWinrtAbsOrientationEulerAngles::kDegreeThreshold *
0.5f;
threshold_helper(false);
- last_sent_y +=
- PlatformSensorReaderWinrtAbsOrientationEulerAngles::kDegreeThreshold *
- 0.5f;
- threshold_helper(false);
- last_sent_z +=
- PlatformSensorReaderWinrtAbsOrientationEulerAngles::kDegreeThreshold *
- 0.5f;
- threshold_helper(false);
-
- // Expect callback, threshold has been met since last reported sample
last_sent_x +=
PlatformSensorReaderWinrtAbsOrientationEulerAngles::kDegreeThreshold *
0.6f;
threshold_helper(true);
+ last_sent_y +=
+ PlatformSensorReaderWinrtAbsOrientationEulerAngles::kDegreeThreshold *
+ 0.5f;
+ threshold_helper(false);
last_sent_y +=
PlatformSensorReaderWinrtAbsOrientationEulerAngles::kDegreeThreshold *
0.6f;
threshold_helper(true);
last_sent_z +=
PlatformSensorReaderWinrtAbsOrientationEulerAngles::kDegreeThreshold *
- 0.6f;
- threshold_helper(true);
-
- // Expect callback, threshold has been met exactly
- last_sent_x +=
- PlatformSensorReaderWinrtAbsOrientationEulerAngles::kDegreeThreshold;
- threshold_helper(true);
- last_sent_y +=
- PlatformSensorReaderWinrtAbsOrientationEulerAngles::kDegreeThreshold;
- threshold_helper(true);
+ 0.5f;
+ threshold_helper(false);
last_sent_z +=
- PlatformSensorReaderWinrtAbsOrientationEulerAngles::kDegreeThreshold;
+ PlatformSensorReaderWinrtAbsOrientationEulerAngles::kDegreeThreshold *
+ 0.6f;
threshold_helper(true);
sensor->StopSensor();
@@ -1674,34 +1788,32 @@ TEST_F(PlatformSensorReaderTestWinrt, AbsOrientationQuatThresholding) {
OrientationSensorReadingChangedEventArgs>>();
auto fake_sensor = fake_sensor_factory->fake_sensor_;
- auto sensor =
- std::make_unique<PlatformSensorReaderWinrtAbsOrientationQuaternion>();
- sensor->InitForTesting(base::BindLambdaForTesting(
- [&](ABI::Windows::Devices::Sensors::IOrientationSensorStatics**
- sensor_factory) -> HRESULT {
- return fake_sensor_factory.CopyTo(sensor_factory);
- }));
- EXPECT_TRUE(sensor->Initialize());
+ auto sensor = CreateAndInitializeSensor<
+ PlatformSensorReaderWinrtAbsOrientationQuaternion>(fake_sensor_factory);
+ ASSERT_TRUE(sensor);
auto mock_client = std::make_unique<testing::NiceMock<MockClient>>();
-
- bool expected_callback = false;
- EXPECT_CALL(*mock_client, OnReadingUpdated(::testing::_))
- .WillRepeatedly(testing::Invoke(
- [&](const SensorReading&) { EXPECT_TRUE(expected_callback); }));
-
sensor->SetClient(mock_client.get());
+
PlatformSensorConfiguration sensor_config(kExpectedReportFrequencySet);
EXPECT_TRUE(sensor->StartSensor(sensor_config));
double last_sent_rad = 1.0;
auto threshold_helper = [&](bool expect_callback) {
- expected_callback = expect_callback;
auto quat = gfx::Quaternion(gfx::Vector3dF(1.0, 0, 0), last_sent_rad);
auto reading = Microsoft::WRL::Make<FakeOrientationSensorReadingWinrt>(
ABI::Windows::Foundation::DateTime{}, quat.w(), quat.x(), quat.y(),
quat.z());
- fake_sensor->TriggerFakeSensorReading(reading);
+ if (expect_callback) {
+ base::RunLoop run_loop;
+ EXPECT_CALL(*mock_client, OnReadingUpdated(::testing::_))
+ .WillOnce(base::test::RunClosure(run_loop.QuitClosure()));
+ fake_sensor->TriggerFakeSensorReading(reading);
+ run_loop.Run();
+ } else {
+ fake_sensor->TriggerFakeSensorReading(reading);
+ task_environment_.RunUntilIdle();
+ }
};
// Expect callback, first sample
@@ -1719,11 +1831,6 @@ TEST_F(PlatformSensorReaderTestWinrt, AbsOrientationQuatThresholding) {
0.6f;
threshold_helper(true);
- // Expect callback, threshold has been met exactly
- last_sent_rad +=
- PlatformSensorReaderWinrtAbsOrientationQuaternion::kRadianThreshold;
- threshold_helper(true);
-
sensor->StopSensor();
}
diff --git a/chromium/services/device/generic_sensor/platform_sensor_win.cc b/chromium/services/device/generic_sensor/platform_sensor_win.cc
index 3ee3c1fa86a8..e9a78687250f 100644
--- src/3rdparty/chromium/services/device/generic_sensor/platform_sensor_win.cc
+++ src/3rdparty/chromium/services/device/generic_sensor/platform_sensor_win.cc
@@ -43,6 +43,8 @@ double PlatformSensorWin::GetMaximumSupportedFrequency() {
}
void PlatformSensorWin::OnReadingUpdated(const SensorReading& reading) {
+ // This function is normally called from |sensor_thread_runner_|, except on
+ // PlatformSensorAndProviderTestWin.
UpdateSharedBufferAndNotifyClients(reading);
}
@@ -75,6 +77,7 @@ bool PlatformSensorWin::CheckSensorConfiguration(
}
PlatformSensorWin::~PlatformSensorWin() {
+ DCHECK(main_task_runner()->RunsTasksInCurrentSequence());
sensor_reader_->SetClient(nullptr);
sensor_thread_runner_->DeleteSoon(FROM_HERE, sensor_reader_.get());
}
From dbafaa10eb690d68e0531313663f47c422dc8771 Mon Sep 17 00:00:00 2001
From: kylechar <kylechar@chromium.org>
Date: Tue, 9 Apr 2024 17:14:26 +0000
Subject: [PATCH] [Backport] CVE-2024-3157: Out of bounds write in Compositing
Cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/5420432:
Validate buffer length
The BitmapInSharedMemory mojo traits were only validating row length and
not total buffer length.
(cherry picked from commit 1a19ff70bd54847d818566bd7a1e7c384c419746)
(cherry picked from commit f15315f1cb7897e208947a40d538aac693283d7f)
Bug: 331237485
Change-Id: Ia2318899c44e9e7ac72fc7183954e6ce2c702179
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5396796
Commit-Queue: Kyle Charbonneau <kylechar@chromium.org>
Cr-Original-Original-Commit-Position: refs/heads/main@{#1278417}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5420432
Commit-Queue: danakj <danakj@chromium.org>
Cr-Original-Commit-Position: refs/branch-heads/6312@{#786}
Cr-Original-Branched-From: 6711dcdae48edaf98cbc6964f90fac85b7d9986e-refs/heads/main@{#1262506}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5433678
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: Kyle Charbonneau <kylechar@chromium.org>
Cr-Commit-Position: refs/branch-heads/6099@{#2003}
Cr-Branched-From: e6ee4500f7d6549a9ac1354f8d056da49ef406be-refs/heads/main@{#1217362}
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/554656
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
---
.../cpp/compositing/bitmap_in_shared_memory_mojom_traits.cc | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/chromium/services/viz/public/cpp/compositing/bitmap_in_shared_memory_mojom_traits.cc b/chromium/services/viz/public/cpp/compositing/bitmap_in_shared_memory_mojom_traits.cc
index a6e5f45d9e72..519d554055e5 100644
--- src/3rdparty/chromium/services/viz/public/cpp/compositing/bitmap_in_shared_memory_mojom_traits.cc
+++ src/3rdparty/chromium/services/viz/public/cpp/compositing/bitmap_in_shared_memory_mojom_traits.cc
@@ -76,6 +76,10 @@ bool StructTraits<viz::mojom::BitmapInSharedMemoryDataView, SkBitmap>::Read(
if (!mapping_ptr->IsValid())
return false;
+ if (mapping_ptr->size() < image_info.computeByteSize(data.row_bytes())) {
+ return false;
+ }
+
if (!sk_bitmap->installPixels(image_info, mapping_ptr->memory(),
data.row_bytes(), &DeleteSharedMemoryMapping,
mapping_ptr.get())) {
From ac780f41ba9b3eb8329ca8e09b8857a070aed8c1 Mon Sep 17 00:00:00 2001
From: Shahbaz Youssefi <syoussefi@chromium.org>
Date: Mon, 25 Mar 2024 14:46:56 -0400
Subject: [PATCH] [Backport] CVE-2024-3516: Heap buffer overflow in ANGLE
Cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/angle/angle/+/5391986:
Translator: Disallow samplers in structs in interface blocks
As disallowed by the spec:
> Types and declarators are the same as for other uniform variable
> declarations outside blocks, with these exceptions:
>
> * opaque types are not allowed
Bug: chromium:328859176
Change-Id: Ib94977860102329e520e635c3757827c93ca2163
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5391986
Auto-Submit: Shahbaz Youssefi <syoussefi@chromium.org>
Reviewed-by: Geoff Lang <geofflang@chromium.org>
Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/554657
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
---
.../src/compiler/translator/ParseContext.cpp | 33 ++++++++++++-------
1 file changed, 21 insertions(+), 12 deletions(-)
diff --git a/chromium/third_party/angle/src/compiler/translator/ParseContext.cpp b/chromium/third_party/angle/src/compiler/translator/ParseContext.cpp
index e174725beb7..cb9eb3a4a56 100644
--- src/3rdparty/chromium/third_party/angle/src/compiler/translator/ParseContext.cpp
+++ src/3rdparty/chromium/third_party/angle/src/compiler/translator/ParseContext.cpp
@@ -34,27 +34,39 @@ namespace
const int kWebGLMaxStructNesting = 4;
-bool ContainsSampler(const TStructure *structType);
+struct IsSamplerFunc
+{
+ bool operator()(TBasicType type) { return IsSampler(type); }
+};
+struct IsOpaqueFunc
+{
+ bool operator()(TBasicType type) { return IsOpaqueType(type); }
+};
+
+template <typename OpaqueFunc>
+bool ContainsOpaque(const TStructure *structType);
-bool ContainsSampler(const TType &type)
+template <typename OpaqueFunc>
+bool ContainsOpaque(const TType &type)
{
- if (IsSampler(type.getBasicType()))
+ if (OpaqueFunc{}(type.getBasicType()))
{
return true;
}
if (type.getBasicType() == EbtStruct)
{
- return ContainsSampler(type.getStruct());
+ return ContainsOpaque<OpaqueFunc>(type.getStruct());
}
return false;
}
-bool ContainsSampler(const TStructure *structType)
+template <typename OpaqueFunc>
+bool ContainsOpaque(const TStructure *structType)
{
for (const auto &field : structType->fields())
{
- if (ContainsSampler(*field->type()))
+ if (ContainsOpaque<OpaqueFunc>(*field->type()))
return true;
}
return false;
@@ -1057,7 +1069,7 @@ bool TParseContext::checkIsNotOpaqueType(const TSourceLoc &line,
{
if (pType.type == EbtStruct)
{
- if (ContainsSampler(pType.userDef))
+ if (ContainsOpaque<IsSamplerFunc>(pType.userDef))
{
std::stringstream reasonStream = sh::InitializeStream<std::stringstream>();
reasonStream << reason << " (structure contains a sampler)";
@@ -4923,12 +4935,9 @@ TIntermDeclaration *TParseContext::addInterfaceBlock(
{
TField *field = (*fieldList)[memberIndex];
TType *fieldType = field->type();
- if (IsOpaqueType(fieldType->getBasicType()))
+ if (ContainsOpaque<IsOpaqueFunc>(*fieldType))
{
- std::string reason("unsupported type - ");
- reason += fieldType->getBasicString();
- reason += " types are not allowed in interface blocks";
- error(field->line(), reason.c_str(), fieldType->getBasicString());
+ error(field->line(), "Opaque types are not allowed in interface blocks", blockName);
}
const TQualifier qualifier = fieldType->getQualifier();
From a766045f65f934df3b5f1aa63bc86fbb3e003a09 Mon Sep 17 00:00:00 2001
From: Anu Aliyas <anu.aliyas@qt.io>
Date: Wed, 17 Apr 2024 12:35:58 +0200
Subject: [PATCH] Add missing dependencies to avoid compilation errors
Missing dependencies are added to resolve the dependencies issues
encountered when compiling with Ninja 1.12.0
Fixes:QTBUG-124375
Change-Id: I0f23ef6c60a6f01ee11bffd46121fd7f98dc022c
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/555586
Reviewed-by: Michal Klocek <michal.klocek@qt.io>
---
chromium/content/public/browser/BUILD.gn | 1 +
chromium/extensions/browser/api/declarative_net_request/BUILD.gn | 1 +
2 files changed, 2 insertions(+)
diff --git a/chromium/content/public/browser/BUILD.gn b/chromium/content/public/browser/BUILD.gn
index d38fa8d303a..c5813115e88 100644
--- src/3rdparty/chromium/content/public/browser/BUILD.gn
+++ src/3rdparty/chromium/content/public/browser/BUILD.gn
@@ -539,6 +539,7 @@ jumbo_source_set("browser_sources") {
"//cc",
"//components/services/storage/public/cpp",
"//components/viz/host",
+ "//components/spellcheck:buildflags",
"//content/browser", # Must not be public_deps!
"//device/fido",
"//gpu",
diff --git a/chromium/extensions/browser/api/declarative_net_request/BUILD.gn b/chromium/extensions/browser/api/declarative_net_request/BUILD.gn
index 1fc492f5a0c..13a266e22f1 100644
--- src/3rdparty/chromium/extensions/browser/api/declarative_net_request/BUILD.gn
+++ src/3rdparty/chromium/extensions/browser/api/declarative_net_request/BUILD.gn
@@ -23,6 +23,7 @@ source_set("declarative_net_request") {
"//extensions/common",
"//extensions/common/api",
"//services/preferences/public/cpp",
+ "//components/web_cache/browser",
]
public_deps = [ "//extensions/browser:browser_sources" ]
From d26ab136f38d6bf3311eb04136820b85bc2fc96e Mon Sep 17 00:00:00 2001
From: Dan Sanders <sandersd@chromium.org>
Date: Mon, 26 Feb 2024 20:53:06 +0000
Subject: [PATCH] [Backport] Security bug 326521449
Cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/5323230:
Validate recovery_frame_cnt_ at time of use.
Bug: b/326521449
Change-Id: I778a16553bb3319dddfef6121895a89e0f1938df
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5323230
Commit-Queue: Dan Sanders <sandersd@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
Auto-Submit: Dan Sanders <sandersd@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1265425}
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/556725
Reviewed-by: Michal Klocek <michal.klocek@qt.io>
---
chromium/media/gpu/h264_decoder.cc | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/chromium/media/gpu/h264_decoder.cc b/chromium/media/gpu/h264_decoder.cc
index 2bfdd28d83a..ef9de709045 100644
--- src/3rdparty/chromium/media/gpu/h264_decoder.cc
+++ src/3rdparty/chromium/media/gpu/h264_decoder.cc
@@ -769,13 +769,6 @@ H264Decoder::H264Accelerator::Status H264Decoder::StartNewFrame(
return H264Accelerator::Status::kFail;
}
- if (recovery_frame_cnt_ && *recovery_frame_cnt_ >= max_frame_num_) {
- DVLOG(1) << "Invalid recovery_frame_cnt=" << *recovery_frame_cnt_
- << " (it must be less or equal to max_frame_num-1=" << max_frame_num_ - 1
- << ")";
- return H264Accelerator::Status::kFail;
- }
-
if (!InitCurrPicture(slice_hdr))
return H264Accelerator::Status::kFail;
@@ -999,11 +992,19 @@ bool H264Decoder::FinishPicture(scoped_refptr<H264Picture> pic) {
DVLOG(4) << "Finishing picture frame_num: " << pic->frame_num
<< ", entries in DPB: " << dpb_.size();
if (recovery_frame_cnt_) {
- // This is the first picture after the recovery point SEI message. Computes
- // the frame_num of the frame that should be output from (Spec D.2.8).
+ // This is the first picture after the recovery point SEI message. Validate
+ // `recovery_frame_cnt_` now that we are certain to have max_frame_num_.
+ if (*recovery_frame_cnt_ >= max_frame_num_) {
+ DVLOG(1) << "Invalid recovery_frame_cnt=" << *recovery_frame_cnt_
+ << " (must be less than or equal to max_frame_num-1="
+ << (max_frame_num_ - 1) << ")";
+ return false;
+ }
+
+ // Compute the frame_num of the first frame that should be output (D.2.8).
recovery_frame_num_ =
(*recovery_frame_cnt_ + pic->frame_num) % max_frame_num_;
- DVLOG(3) << "recovery_frame_num_" << *recovery_frame_num_;
+ DVLOG(3) << "recovery_frame_num_=" << *recovery_frame_num_;
recovery_frame_cnt_.reset();
}
From 08e69b86a697e265b3e064b06a05fc5e7a4e079b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dominik=20R=C3=B6ttsches?= <drott@chromium.org>
Date: Thu, 14 Mar 2024 12:48:18 +0000
Subject: [PATCH] [Backport] CVE-2024-3839: Out of bounds read in Fonts
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/5361874:
Disable STAT sanitization/checks through OTS
Due to issues in upstream, OTS STAT sanitization does not provide an
added security benefit. Pass-through the STAT table.
Bug: chromium:41491859
Change-Id: I19dcd87376af553afe242452396b951a74691f3c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5361874
Commit-Queue: Dominik Röttsches <drott@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1272710}
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/556726
Reviewed-by: Michal Klocek <michal.klocek@qt.io>
---
.../blink/renderer/platform/fonts/web_font_decoder.cc | 2 ++
1 file changed, 2 insertions(+)
diff --git a/chromium/third_party/blink/renderer/platform/fonts/web_font_decoder.cc b/chromium/third_party/blink/renderer/platform/fonts/web_font_decoder.cc
index 0953dc528dd..1ac9b8c9623 100644
--- src/3rdparty/chromium/third_party/blink/renderer/platform/fonts/web_font_decoder.cc
+++ src/3rdparty/chromium/third_party/blink/renderer/platform/fonts/web_font_decoder.cc
@@ -104,6 +104,7 @@ ots::TableAction BlinkOTSContext::GetTableAction(uint32_t tag) {
const uint32_t kCpalTag = OTS_TAG('C', 'P', 'A', 'L');
const uint32_t kCff2Tag = OTS_TAG('C', 'F', 'F', '2');
const uint32_t kSbixTag = OTS_TAG('s', 'b', 'i', 'x');
+ const uint32_t kStatTag = OTS_TAG('S', 'T', 'A', 'T');
#if HB_VERSION_ATLEAST(1, 0, 0)
const uint32_t kBaseTag = OTS_TAG('B', 'A', 'S', 'E');
const uint32_t kGdefTag = OTS_TAG('G', 'D', 'E', 'F');
@@ -131,6 +132,7 @@ ots::TableAction BlinkOTSContext::GetTableAction(uint32_t tag) {
case kCpalTag:
case kCff2Tag:
case kSbixTag:
+ case kStatTag:
#if HB_VERSION_ATLEAST(1, 0, 0)
// Let HarfBuzz handle how to deal with broken tables.
case kAvarTag:
From 8755f5a5aafae5a0e74839042cd2f65dc86061e8 Mon Sep 17 00:00:00 2001
From: Liza Burakova <liza@chromium.org>
Date: Wed, 21 Feb 2024 19:02:15 +0000
Subject: [PATCH] [Backport] CVE-2024-3837: Use after free in QUIC
Manual cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/5268864:
Check if session is going away in Handle::RequestStream.
This CL adds an extra check in the QuicChromiumClientSession
handle's RequestSession to make sure the session is not
marked as going away before creating a new StreamRequest.
Bug: 41491379
Change-Id: I687dfc23131871cdba345d3cf78dbbbd2e619ce9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5268864
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Commit-Queue: Liza Burakova <liza@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1263483}
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/556727
Reviewed-by: Michal Klocek <michal.klocek@qt.io>
---
chromium/net/quic/quic_chromium_client_session.cc | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/chromium/net/quic/quic_chromium_client_session.cc b/chromium/net/quic/quic_chromium_client_session.cc
index d039d1ccb94..400006a170f 100644
--- src/3rdparty/chromium/net/quic/quic_chromium_client_session.cc
+++ src/3rdparty/chromium/net/quic/quic_chromium_client_session.cc
@@ -454,7 +454,8 @@ int QuicChromiumClientSession::Handle::RequestStream(
const NetworkTrafficAnnotationTag& traffic_annotation) {
DCHECK(!stream_request_);
- if (!session_)
+ // TODO(crbug.com/41491379): Add a regression test.
+ if (!session_ || session_->going_away_)
return ERR_CONNECTION_CLOSED;
requires_confirmation |= session_->gquic_zero_rtt_disabled();
From 271e21366ac1826df43119eb7b746019947be1f7 Mon Sep 17 00:00:00 2001
From: Brendon Tiszka <tiszka@chromium.org>
Date: Sun, 3 Mar 2024 21:30:59 +0000
Subject: [PATCH] [Backport] Security bug 327698060
Cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/5337387:
PaintOpReader: Harden PaintImage deserialization
Add missing validity check after `Read`
Bug: 327698060
Change-Id: I0aa5120296009998af3235a01304a1f597a82a33
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5337387
Commit-Queue: Khushal Sagar <khushalsagar@chromium.org>
Reviewed-by: Khushal Sagar <khushalsagar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1267636}
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/556748
Reviewed-by: Michal Klocek <michal.klocek@qt.io>
---
chromium/cc/paint/paint_op_reader.cc | 3 +++
1 file changed, 3 insertions(+)
diff --git a/chromium/cc/paint/paint_op_reader.cc b/chromium/cc/paint/paint_op_reader.cc
index 6d920c33562..7d1eb51441f 100644
--- src/3rdparty/chromium/cc/paint/paint_op_reader.cc
+++ src/3rdparty/chromium/cc/paint/paint_op_reader.cc
@@ -351,6 +351,9 @@ void PaintOpReader::Read(PaintImage* image) {
case PaintOp::SerializedImageType::kImageData: {
SkColorType color_type;
Read(&color_type);
+ if (!valid_) {
+ return;
+ }
// Color types requiring alignment larger than kDefaultAlignment is not
// supported.
if (static_cast<size_t>(SkColorTypeBytesPerPixel(color_type)) >
From 502983e6adc3db0d12ee8e9ff35d53df0e149870 Mon Sep 17 00:00:00 2001
From: Pete Williamson <petewil@chromium.org>
Date: Tue, 27 Feb 2024 00:19:05 +0000
Subject: [PATCH] [Backport] Security bug 40940917
Cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/5293726:
Fix misalligned address in hunspell::NodeReader::ReaderForLookupAt
With the Hunspell spell checking library, we are using a custom wrapper
to read the dictionaries from files. In that custom wrapper, we were
reading by using reinterpret_cast to interpret an offset into a pointer,
and then reading the bytes at that pointer for the child_offset.
The spell checking code appears to have been working properly in the
field. However, the current code caused fuzzing test failures, and
those failures are blocking other tests, so we need to fix this to
unblock other tests.
It turns out that we were casting a value to a pointer that did not
have proper alignment (for instance, a pointer to a 32 bit int needs
to be 4 byte allinged, but this pointer was not). While it has often
worked in older compilers, it turns out this is undefined behavior.
Instead of relying on undefined behavior, the right thing to do is to
use std::memcpy to copy the bytes from the misalligned address into
their final destination (either an int32 or an int16 in this case).
Bug: 40940917
Change-Id: I8aeba9ee8000b51e98863813235d8dceb1c41ceb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5293726
Commit-Queue: Peter Williamson <petewil@chromium.org>
Reviewed-by: Trevor Perrier <perrier@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1265552}
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/556750
Reviewed-by: Michal Klocek <michal.klocek@qt.io>
---
.../hunspell/google/bdict_reader.cc | 26 ++++++++++++++-----
1 file changed, 20 insertions(+), 6 deletions(-)
diff --git a/chromium/third_party/hunspell/google/bdict_reader.cc b/chromium/third_party/hunspell/google/bdict_reader.cc
index d51112ea48e7..56abd15dd121 100644
--- src/3rdparty/chromium/third_party/hunspell/google/bdict_reader.cc
+++ src/3rdparty/chromium/third_party/hunspell/google/bdict_reader.cc
@@ -5,6 +5,7 @@
#include "third_party/hunspell/google/bdict_reader.h"
#include <stdint.h>
+#include <cstdint>
#include "base/check.h"
@@ -413,19 +414,32 @@ NodeReader::FindResult NodeReader::ReaderForLookupAt(
if (index >= static_cast<size_t>(lookup_num_chars()) || !is_valid_)
return FIND_DONE;
- size_t child_offset;
+ size_t child_offset = 0;
if (is_lookup_32()) {
// Table contains 32-bit absolute offsets.
- child_offset =
- reinterpret_cast<const unsigned int*>(table_begin)[index];
+
+ // We need to use memcpy here instead of just casting the offset into a
+ // pointer to an int because the cast can cause undefined behavior if
+ // the pointer is not alligned, and in this case it is not.
+ int byte_offset = index * sizeof(uint32_t);
+ std::memcpy(&child_offset,
+ reinterpret_cast<const void*>(table_begin + byte_offset),
+ sizeof(uint32_t));
if (!child_offset)
return FIND_NOTHING; // This entry in the table is empty.
} else {
// Table contains 16-bit offsets relative to the current node.
- child_offset =
- reinterpret_cast<const unsigned short*>(table_begin)[index];
- if (!child_offset)
+
+ // We need to use memcpy here instead of just casting the offset into a
+ // pointer to an int because the cast can cause undefined behavior if
+ // the pointer is not alligned, and in this case it is not.
+ int byte_offset = index * sizeof(uint16_t);
+ std::memcpy(&child_offset,
+ reinterpret_cast<const void*>(table_begin + byte_offset),
+ sizeof(uint16_t));
+ if (!child_offset) {
return FIND_NOTHING; // This entry in the table is empty.
+ }
child_offset += node_offset_;
}
From 6086d28d3a0dbfac1808ba5767ca1eea4a5acaf9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marja=20H=C3=B6ltt=C3=A4?= <marja@google.com>
Date: Tue, 26 Mar 2024 13:53:21 +0000
Subject: [PATCH] [Backport] CVE-2024-3914: Use after free in V8 (1/2)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Manual backport of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/5387887:
[M120-LTS] Fix DOMArrayBuffer::IsDetached()
M120 merge issues:
third_party/blink/renderer/core/typed_arrays/dom_array_buffer.cc:
- Conflicting types for variable worlds
- Conflicting AllWorldsInIsolate() call (M120 doesn't use the last argument)
A DOMArrayBuffer was maintaining its own "is_detached_" state, and
would consider itself non-detached even if the corresponding
JSArrayBuffer (or, all of them, in case there are several) was
detached.
Piping in the v8::Isolate would be a too big change for this fix, so this is using v8::Isolate::GetCurrent() for now.
Bug: 330759272
Change-Id: I1e98ebd2066d2e59658db12f1bb419b6ebc1d706
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5387887
Commit-Queue: Marja Hölttä <marja@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1278283}
(cherry picked from commit 04e7550d7aa3bf4ac4e49d7074972d357de139e6)
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/556751
Reviewed-by: Michal Klocek <michal.klocek@qt.io>
---
.../core/typed_arrays/dom_array_buffer.cc | 50 +++++++++++++++++++
.../core/typed_arrays/dom_array_buffer.h | 13 +++++
.../core/typed_arrays/dom_array_buffer_base.h | 2 +-
3 files changed, 64 insertions(+), 1 deletion(-)
diff --git a/chromium/third_party/blink/renderer/core/typed_arrays/dom_array_buffer.cc b/chromium/third_party/blink/renderer/core/typed_arrays/dom_array_buffer.cc
index c195e28b442..197a4aebf5b 100644
--- src/3rdparty/chromium/third_party/blink/renderer/core/typed_arrays/dom_array_buffer.cc
+++ src/3rdparty/chromium/third_party/blink/renderer/core/typed_arrays/dom_array_buffer.cc
@@ -48,6 +48,15 @@ static void AccumulateArrayBuffersForAllWorlds(
v8::Isolate* isolate,
DOMArrayBuffer* object,
Vector<v8::Local<v8::ArrayBuffer>, 4>& buffers) {
+ if (!object->has_non_main_world_wrappers() && IsMainThread()) {
+ const DOMWrapperWorld& world = DOMWrapperWorld::MainWorld();
+ v8::Local<v8::Object> wrapper = world.DomDataStore().Get(object, isolate);
+ if (!wrapper.IsEmpty()) {
+ buffers.push_back(v8::Local<v8::ArrayBuffer>::Cast(wrapper));
+ }
+ return;
+ }
+
Vector<scoped_refptr<DOMWrapperWorld>> worlds;
DOMWrapperWorld::AllWorldsInCurrentThread(worlds);
for (const auto& world : worlds) {
@@ -256,6 +265,47 @@ v8::MaybeLocal<v8::Value> DOMArrayBuffer::Wrap(ScriptState* script_state) {
wrapper);
}
+bool DOMArrayBuffer::IsDetached() const {
+ if (contents_.BackingStore() == nullptr) {
+ return is_detached_;
+ }
+ if (is_detached_) {
+ return true;
+ }
+
+ v8::Isolate* isolate = v8::Isolate::GetCurrent();
+ v8::HandleScope handle_scope(isolate);
+ Vector<v8::Local<v8::ArrayBuffer>, 4> buffer_handles;
+ AccumulateArrayBuffersForAllWorlds(isolate, const_cast<DOMArrayBuffer*>(this), buffer_handles);
+
+ // There may be several v8::ArrayBuffers corresponding to the DOMArrayBuffer,
+ // but at most one of them may be non-detached.
+ int nondetached_count = 0;
+ int detached_count = 0;
+
+ for (const auto& buffer_handle : buffer_handles) {
+ if (buffer_handle->WasDetached()) {
+ ++detached_count;
+ } else {
+ ++nondetached_count;
+ }
+ }
+ CHECK_LE(nondetached_count, 1);
+
+ return nondetached_count == 0 && detached_count > 0;
+}
+
+v8::Local<v8::Object> DOMArrayBuffer::AssociateWithWrapper(
+ v8::Isolate* isolate,
+ const WrapperTypeInfo* wrapper_type_info,
+ v8::Local<v8::Object> wrapper) {
+ if (!DOMWrapperWorld::Current(isolate).IsMainWorld()) {
+ has_non_main_world_wrappers_ = true;
+ }
+ return ScriptWrappable::AssociateWithWrapper(isolate, wrapper_type_info,
+ wrapper);
+}
+
DOMArrayBuffer* DOMArrayBuffer::Slice(size_t begin, size_t end) const {
begin = std::min(begin, ByteLength());
end = std::min(end, ByteLength());
diff --git a/chromium/third_party/blink/renderer/core/typed_arrays/dom_array_buffer.h b/chromium/third_party/blink/renderer/core/typed_arrays/dom_array_buffer.h
index 87881af8787..84322ff31fc 100644
--- src/3rdparty/chromium/third_party/blink/renderer/core/typed_arrays/dom_array_buffer.h
+++ src/3rdparty/chromium/third_party/blink/renderer/core/typed_arrays/dom_array_buffer.h
@@ -87,6 +87,17 @@ class CORE_EXPORT DOMArrayBuffer : public DOMArrayBufferBase {
void Trace(Visitor*) const override;
+ bool IsDetached() const override;
+
+ v8::Local<v8::Object> AssociateWithWrapper(
+ v8::Isolate* isolate,
+ const WrapperTypeInfo* wrapper_type_info,
+ v8::Local<v8::Object> wrapper) override;
+
+ bool has_non_main_world_wrappers() const {
+ return has_non_main_world_wrappers_;
+ }
+
private:
v8::Maybe<bool> TransferDetachable(v8::Isolate*,
v8::Local<v8::Value> detach_key,
@@ -97,6 +108,8 @@ class CORE_EXPORT DOMArrayBuffer : public DOMArrayBufferBase {
// support only v8::String as the detach key type. It's also convenient that
// we can write `array_buffer->SetDetachKey(isolate, "my key")`.
TraceWrapperV8Reference<v8::String> detach_key_;
+
+ bool has_non_main_world_wrappers_ = false;
};
} // namespace blink
diff --git a/chromium/third_party/blink/renderer/core/typed_arrays/dom_array_buffer_base.h b/chromium/third_party/blink/renderer/core/typed_arrays/dom_array_buffer_base.h
index 95511438595..c83dc489c97 100644
--- src/3rdparty/chromium/third_party/blink/renderer/core/typed_arrays/dom_array_buffer_base.h
+++ src/3rdparty/chromium/third_party/blink/renderer/core/typed_arrays/dom_array_buffer_base.h
@@ -27,7 +27,7 @@ class CORE_EXPORT DOMArrayBufferBase : public ScriptWrappable {
size_t ByteLength() const { return contents_.DataLength(); }
- bool IsDetached() const { return is_detached_; }
+ virtual bool IsDetached() const { return is_detached_; }
void Detach() { is_detached_ = true; }
From 92d5d3c528d5c4b22aa295b6b26303f174c0991b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marja=20H=C3=B6ltt=C3=A4?= <marja@google.com>
Date: Thu, 4 Apr 2024 09:43:42 +0200
Subject: [PATCH] [Backport] CVE-2024-3914: Use after free in V8 (2/2)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Manual cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/5419329:
[M120-LTS] Comment out a CHECK that a DOMAB has maximally one non-detached JSAB
Based on crash reports, this assumption is not true and has to be
investigated.
Removing this newly introduced CHECK to be able to merge fixes in this
area - we still violate this invariant but the fixes are a step into
the right direction.
Fix in question:
https://chromium-review.googlesource.com/5387887
which also introduced this CHECK.
Bug: 330759272
Change-Id: I4ba52fee7ed8f45e352efd347e87df03d896ac3d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5419329
Commit-Queue: Marja Hölttä <marja@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1282379}
(cherry picked from commit 1e1e1bccfb84713fc325025eae43e746abcc115d)
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/556752
Reviewed-by: Michal Klocek <michal.klocek@qt.io>
---
.../blink/renderer/core/typed_arrays/dom_array_buffer.cc | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/chromium/third_party/blink/renderer/core/typed_arrays/dom_array_buffer.cc b/chromium/third_party/blink/renderer/core/typed_arrays/dom_array_buffer.cc
index 197a4aebf5b..796f45d7114 100644
--- src/3rdparty/chromium/third_party/blink/renderer/core/typed_arrays/dom_array_buffer.cc
+++ src/3rdparty/chromium/third_party/blink/renderer/core/typed_arrays/dom_array_buffer.cc
@@ -290,7 +290,11 @@ bool DOMArrayBuffer::IsDetached() const {
++nondetached_count;
}
}
- CHECK_LE(nondetached_count, 1);
+ // This CHECK fires even though it should not. TODO(330759272): Investigate
+ // under which conditions we end up with multiple non-detached JSABs for the
+ // same DOMAB and potentially restore this check.
+
+ // CHECK_LE(nondetached_count, 1);
return nondetached_count == 0 && detached_count > 0;
}
From 7ea70f94879b466b8319a52b786ae2ce3e47b737 Mon Sep 17 00:00:00 2001
From: Dan McArdle <dmcardle@chromium.org>
Date: Tue, 30 Jan 2024 22:20:43 +0000
Subject: [PATCH] [Backport] Dependency for security bug 326498393 (1/2)
Cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/5245970:
Prevent dangling pointer by inlining ScopedBusyTimeout
Bug: 1522873
Change-Id: Icd6262eb23404c273fa31021cddf36bea0bb56db
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5245970
Reviewed-by: Evan Stade <estade@chromium.org>
Reviewed-by: David Benjamin <davidben@chromium.org>
Commit-Queue: Dan McArdle <dmcardle@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1254220}
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/556851
Reviewed-by: Michal Klocek <michal.klocek@qt.io>
---
chromium/sql/database.cc | 48 +++++++++++++++-------------------------
1 file changed, 18 insertions(+), 30 deletions(-)
diff --git a/chromium/sql/database.cc b/chromium/sql/database.cc
index d5d350e85f4..5825127634a 100644
--- src/3rdparty/chromium/sql/database.cc
+++ src/3rdparty/chromium/sql/database.cc
@@ -74,21 +74,6 @@ static constexpr char kSqliteOpenInMemoryPath[] = ":memory:";
// TODO(shess): Better story on this. http://crbug.com/56559
const int kBusyTimeoutSeconds = 1;
-class ScopedBusyTimeout {
- public:
- explicit ScopedBusyTimeout(sqlite3* db) : db_(db) {}
- ~ScopedBusyTimeout() { sqlite3_busy_timeout(db_, 0); }
-
- int SetTimeout(base::TimeDelta timeout) {
- DCHECK_LT(timeout.InMilliseconds(), INT_MAX);
- return sqlite3_busy_timeout(db_,
- static_cast<int>(timeout.InMilliseconds()));
- }
-
- private:
- raw_ptr<sqlite3> db_;
-};
-
// Helper to "safely" enable writable_schema. No error checking
// because it is reasonable to just forge ahead in case of an error.
// If turning it on fails, then most likely nothing will work, whereas
@@ -1405,22 +1390,14 @@ SqliteResultCode Database::ExecuteAndReturnResultCode(const char* sql) {
}
bool Database::Execute(const char* sql) {
- TRACE_EVENT1("sql", "Database::Execute", "query", TRACE_STR_COPY(sql));
-
- if (!db_) {
- DCHECK(poisoned_) << "Illegal use of Database without a db";
- return false;
- }
+ TRACE_EVENT0("sql", "Database::Execute");
- SqliteResultCode sqlite_result_code = ExecuteAndReturnResultCode(sql);
- if (sqlite_result_code != SqliteResultCode::kOk)
- OnSqliteError(ToSqliteErrorCode(sqlite_result_code), nullptr, sql);
-
- return sqlite_result_code == SqliteResultCode::kOk;
+ return ExecuteWithTimeout(sql, base::TimeDelta());
}
bool Database::ExecuteWithTimeout(const char* sql, base::TimeDelta timeout) {
- TRACE_EVENT0("sql", "Database::ExecuteWithTimeout");
+ TRACE_EVENT1("sql", "Database::ExecuteWithTimeout", "query",
+ TRACE_STR_COPY(sql));
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
if (!db_) {
@@ -1428,9 +1405,20 @@ bool Database::ExecuteWithTimeout(const char* sql, base::TimeDelta timeout) {
return false;
}
- ScopedBusyTimeout busy_timeout(db_);
- busy_timeout.SetTimeout(timeout);
- return Execute(sql);
+ // Passing zero or a negative value to sqlite3_busy_timeout() would clear any
+ // busy handlers defined prior to this point.
+ if (timeout.is_positive()) {
+ DCHECK_LT(timeout.InMilliseconds(), INT_MAX);
+ sqlite3_busy_timeout(db_, static_cast<int>(timeout.InMilliseconds()));
+ }
+ SqliteResultCode sqlite_result_code = ExecuteAndReturnResultCode(sql);
+ sqlite3_busy_timeout(db_, 0);
+ if (sqlite_result_code != SqliteResultCode::kOk) {
+ OnSqliteError(ToSqliteErrorCode(sqlite_result_code), nullptr, sql);
+ // At this point, `this` may have been modified or even deleted as a result
+ // of the caller-provided error callback.
+ }
+ return sqlite_result_code == SqliteResultCode::kOk;
}
bool Database::ExecuteScriptForTesting(const char* sql_script) {
From 1e584e2b322c5bc8ff49a7a64375d969662242e1 Mon Sep 17 00:00:00 2001
From: Dan McArdle <dmcardle@chromium.org>
Date: Tue, 6 Feb 2024 23:15:33 +0000
Subject: [PATCH] [Backport] Dependency for security bug 326498393 (2/2)
Manual cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/5245191:
Prevent dangling pointer in ScopedWritableSchema
Bug: 1522873
Change-Id: I71d7d178c98f2f2fc880762942fbca608d8cb81c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5245191
Reviewed-by: Evan Stade <estade@chromium.org>
Commit-Queue: Dan McArdle <dmcardle@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1257060}
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/556852
Reviewed-by: Michal Klocek <michal.klocek@qt.io>
---
chromium/sql/database.cc | 29 +++++++++++++++++++----------
chromium/sql/database.h | 4 ++++
2 files changed, 23 insertions(+), 10 deletions(-)
diff --git a/chromium/sql/database.cc b/chromium/sql/database.cc
index 5825127634a..54c7c44c7e2 100644
--- src/3rdparty/chromium/sql/database.cc
+++ src/3rdparty/chromium/sql/database.cc
@@ -74,23 +74,29 @@ static constexpr char kSqliteOpenInMemoryPath[] = ":memory:";
// TODO(shess): Better story on this. http://crbug.com/56559
const int kBusyTimeoutSeconds = 1;
-// Helper to "safely" enable writable_schema. No error checking
-// because it is reasonable to just forge ahead in case of an error.
-// If turning it on fails, then most likely nothing will work, whereas
-// if turning it off fails, it only matters if some code attempts to
-// continue working with the database and tries to modify the
+// RAII-style wrapper that enables `writable_schema` until it goes out of scope.
+// No error checking on the PRAGMA statements because it is reasonable to just
+// forge ahead in case of an error. If turning it on fails, then most likely
+// nothing will work, whereas if turning it off fails, it only matters if some
+// code attempts to continue working with the database and tries to modify the
// sqlite_schema table (none of our code does this).
class ScopedWritableSchema {
public:
- explicit ScopedWritableSchema(sqlite3* db) : db_(db) {
- sqlite3_exec(db_, "PRAGMA writable_schema=1", nullptr, nullptr, nullptr);
+ explicit ScopedWritableSchema(base::WeakPtr<Database> db)
+ : db_(std::move(db)) {
+ CHECK(db_->is_open());
+ std::ignore = db_->Execute("PRAGMA writable_schema=1");
}
~ScopedWritableSchema() {
- sqlite3_exec(db_, "PRAGMA writable_schema=0", nullptr, nullptr, nullptr);
+ // Database invalidates its WeakPtrs before closing the SQLite connection.
+ if (db_) {
+ CHECK(db_->is_open());
+ std::ignore = db_->Execute("PRAGMA writable_schema=0");
+ }
}
private:
- raw_ptr<sqlite3> db_;
+ const base::WeakPtr<Database> db_;
};
// Raze() helper that uses SQLite's online backup API.
@@ -386,6 +392,9 @@ void Database::CloseInternal(bool forced) {
std::move(memory_dump_provider_));
}
+ // Invalidate any `WeakPtr`s held by scoping helpers.
+ weak_factory_.InvalidateWeakPtrs();
+
auto sqlite_result_code = ToSqliteResultCode(sqlite3_close(db_));
DCHECK_NE(sqlite_result_code, SqliteResultCode::kBusy)
@@ -1014,7 +1023,7 @@ bool Database::Raze() {
// sqlite3.c lockBtree().]
// TODO(shess): With this, "PRAGMA auto_vacuum" and "PRAGMA
// page_size" can be used to query such a database.
- ScopedWritableSchema writable_schema(db_);
+ ScopedWritableSchema writable_schema(weak_factory_.GetWeakPtr());
#if BUILDFLAG(IS_WIN)
// On Windows, truncate silently fails when applied to memory-mapped files.
diff --git a/chromium/sql/database.h b/chromium/sql/database.h
index 79172875fda..71a2f672221 100644
--- src/3rdparty/chromium/sql/database.h
+++ src/3rdparty/chromium/sql/database.h
@@ -24,6 +24,7 @@
#include "base/memory/raw_ptr.h"
#include "base/memory/raw_ptr_exclusion.h"
#include "base/memory/ref_counted.h"
+#include "base/memory/weak_ptr.h"
#include "base/sequence_checker.h"
#include "base/strings/string_piece.h"
#include "base/threading/scoped_blocking_call.h"
@@ -1021,6 +1022,9 @@ class COMPONENT_EXPORT(SQL) Database {
// Stores the dump provider object when db is open.
std::unique_ptr<DatabaseMemoryDumpProvider> memory_dump_provider_;
+
+ // Vends WeakPtr<Database> for internal scoping helpers.
+ base::WeakPtrFactory<Database> weak_factory_{this};
};
} // namespace sql
From 1ea3a7dc0b8e1f779f0ff047799cce884bb9986a Mon Sep 17 00:00:00 2001
From: Dan McArdle <dmcardle@chromium.org>
Date: Thu, 29 Feb 2024 17:08:14 +0000
Subject: [PATCH] [Backport] Security bug 326498393
Manual backport of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/5320373:
SQL: Make sql::Transaction hold WeakPtr<Database>
Now, Transaction checks whether the Database has been destroyed before
using it.
This CL also adds stricter sequence checking to Database, which found a
few places that Database was passed between sequences:
(1) storage::DatabaseTracker was using and destroying sql::Database on
different sequences. Now, it simply destroys the Database instance
in storage::DatabaseTracker::Shutdown(), which already runs on the
correct sequence.
(1) history::InMemoryDatabase was "slurping" data from disk on one
sequence, then querying the data on a different sequence. To support
this use case, we added added sql:Database::DetachFromSequence().
Now, InMemoryDatabase effectively annotates the cut point where the
Database is handed off to another sequence.
Bug: 326498393
Change-Id: I866061feaf08d48607d97731a512bc02ce497f71
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5320373
Reviewed-by: manuk hovanesian <manukh@chromium.org>
Commit-Queue: Dan McArdle <dmcardle@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Auto-Submit: Dan McArdle <dmcardle@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1267047}
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/556853
Reviewed-by: Michal Klocek <michal.klocek@qt.io>
---
.../history/core/browser/in_memory_database.cc | 3 +++
chromium/sql/database.cc | 16 ++++++++++++++++
chromium/sql/database.h | 12 ++++++++++--
chromium/sql/internal_api_token.h | 1 +
chromium/sql/transaction.cc | 17 ++++++++++++++---
chromium/sql/transaction.h | 4 ++--
.../browser/database/database_tracker.cc | 8 ++++++++
.../storage/browser/database/database_tracker.h | 4 +++-
8 files changed, 57 insertions(+), 8 deletions(-)
diff --git a/chromium/components/history/core/browser/in_memory_database.cc b/chromium/components/history/core/browser/in_memory_database.cc
index ad9c02696e3c..d139ddc45112 100644
--- src/3rdparty/chromium/components/history/core/browser/in_memory_database.cc
+++ src/3rdparty/chromium/components/history/core/browser/in_memory_database.cc
@@ -122,6 +122,9 @@ bool InMemoryDatabase::InitFromDisk(const base::FilePath& history_name) {
// inserting into it.
CreateMainURLIndex();
+ // After this point, the database may be accessed from another sequence.
+ db_.DetachFromSequence();
+
return true;
}
diff --git a/chromium/sql/database.cc b/chromium/sql/database.cc
index 54c7c44c7e26..b69e4c968c56 100644
--- src/3rdparty/chromium/sql/database.cc
+++ src/3rdparty/chromium/sql/database.cc
@@ -45,6 +45,7 @@
#include "build/build_config.h"
#include "sql/database_memory_dump_provider.h"
#include "sql/initialization.h"
+#include "sql/internal_api_token.h"
#include "sql/meta_table.h"
#include "sql/sql_features.h"
#include "sql/sqlite_result_code.h"
@@ -221,6 +222,11 @@ base::FilePath Database::SharedMemoryFilePath(const base::FilePath& db_path) {
return base::FilePath(db_path.value() + FILE_PATH_LITERAL("-shm"));
}
+base::WeakPtr<Database> Database::GetWeakPtr(InternalApiToken) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+ return weak_factory_.GetWeakPtr();
+}
+
Database::StatementRef::StatementRef(Database* database,
sqlite3_stmt* stmt,
bool was_valid)
@@ -341,6 +347,11 @@ bool Database::OpenInMemory() {
return OpenInternal(kSqliteOpenInMemoryPath, OpenMode::kInMemory);
}
+void Database::DetachFromSequence() {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+ DETACH_FROM_SEQUENCE(sequence_checker_);
+}
+
bool Database::OpenTemporary(base::PassKey<Recovery>) {
TRACE_EVENT0("sql", "Database::OpenTemporary");
@@ -350,6 +361,9 @@ bool Database::OpenTemporary(base::PassKey<Recovery>) {
void Database::CloseInternal(bool forced) {
TRACE_EVENT0("sql", "Database::CloseInternal");
+
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
// TODO(shess): Calling "PRAGMA journal_mode = DELETE" at this point
// will delete the -journal file. For ChromiumOS or other more
// embedded systems, this is probably not appropriate, whereas on
@@ -966,6 +980,8 @@ void Database::TrimMemory() {
bool Database::Raze() {
TRACE_EVENT0("sql", "Database::Raze");
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
absl::optional<base::ScopedBlockingCall> scoped_blocking_call;
InitScopedBlockingCall(FROM_HERE, &scoped_blocking_call);
diff --git a/chromium/sql/database.h b/chromium/sql/database.h
index 71a2f6722219..2570aade0157 100644
--- src/3rdparty/chromium/sql/database.h
+++ src/3rdparty/chromium/sql/database.h
@@ -27,6 +27,7 @@
#include "base/memory/weak_ptr.h"
#include "base/sequence_checker.h"
#include "base/strings/string_piece.h"
+#include "base/thread_annotations.h"
#include "base/threading/scoped_blocking_call.h"
#include "base/types/pass_key.h"
#include "sql/internal_api_token.h"
@@ -261,8 +262,10 @@ struct COMPONENT_EXPORT(SQL) DatabaseDiagnostics {
// Handle to an open SQLite database.
//
-// Instances of this class are not thread-safe. After construction, a Database
-// instance should only be accessed from one sequence.
+// Instances of this class are not thread-safe. With few exceptions, Database
+// instances should only be accessed from one sequence. Database instances may
+// be constructed on one sequence and safely used/destroyed on another. Callers
+// may explicitly use `DetachFromSequence()` before moving to another sequence.
//
// When a Database instance goes out of scope, any uncommitted transactions are
// rolled back.
@@ -394,6 +397,10 @@ class COMPONENT_EXPORT(SQL) Database {
// Returns true if the database has been successfully opened.
bool is_open() const;
+ // Detach from the currently-attached sequence. If already attached to a
+ // sequence, this method must be called from that sequence.
+ void DetachFromSequence();
+
// Closes the database. This is automatically performed on destruction for
// you, but this allows you to close the database early. You must not call
// any other functions after closing it. It is permissable to call Close on
@@ -700,6 +707,7 @@ class COMPONENT_EXPORT(SQL) Database {
static base::FilePath SharedMemoryFilePath(const base::FilePath& db_path);
// Internal state accessed by other classes in //sql.
+ base::WeakPtr<Database> GetWeakPtr(InternalApiToken);
sqlite3* db(InternalApiToken) const { return db_; }
bool poisoned(InternalApiToken) const { return poisoned_; }
base::FilePath DbPath(InternalApiToken) const { return DbPath(); }
diff --git a/chromium/sql/internal_api_token.h b/chromium/sql/internal_api_token.h
index c71b70309ee9..d9ec0b834f8e 100644
--- src/3rdparty/chromium/sql/internal_api_token.h
+++ src/3rdparty/chromium/sql/internal_api_token.h
@@ -28,6 +28,7 @@ class InternalApiToken {
friend class BuiltInRecovery;
friend class DatabaseTestPeer;
friend class Recovery;
+ friend class Transaction;
friend struct test::ColumnInfo;
friend bool test::CorruptSizeInHeader(const base::FilePath&);
};
diff --git a/chromium/sql/transaction.cc b/chromium/sql/transaction.cc
index a8d85b67fbd7..71bc7381b55f 100644
--- src/3rdparty/chromium/sql/transaction.cc
+++ src/3rdparty/chromium/sql/transaction.cc
@@ -7,11 +7,13 @@
#include "base/check.h"
#include "base/sequence_checker.h"
#include "sql/database.h"
+#include "sql/internal_api_token.h"
namespace sql {
-Transaction::Transaction(Database* database) : database_(*database) {
- DCHECK(database);
+Transaction::Transaction(Database* database) {
+ CHECK(database);
+ database_ = database->GetWeakPtr(InternalApiToken{});
}
Transaction::~Transaction() {
@@ -21,7 +23,7 @@ Transaction::~Transaction() {
<< "Begin() not called immediately after Transaction creation";
#endif // DCHECK_IS_ON()
- if (is_active_)
+ if (is_active_ && database_ && database_->is_open())
database_->RollbackTransaction();
}
@@ -33,6 +35,9 @@ bool Transaction::Begin() {
#endif // DCHECK_IS_ON()
DCHECK(!is_active_);
+ if (!database_) {
+ return false;
+ }
is_active_ = database_->BeginTransaction();
return is_active_;
}
@@ -49,6 +54,9 @@ void Transaction::Rollback() {
DCHECK(is_active_) << __func__ << " called after Begin() failed";
is_active_ = false;
+ if (!database_) {
+ return;
+ }
database_->RollbackTransaction();
}
@@ -63,6 +71,9 @@ bool Transaction::Commit() {
DCHECK(is_active_) << __func__ << " called after Begin() failed";
is_active_ = false;
+ if (!database_) {
+ return false;
+ }
return database_->CommitTransaction();
}
diff --git a/chromium/sql/transaction.h b/chromium/sql/transaction.h
index 54573bcf98d2..8e20d7fe737a 100644
--- src/3rdparty/chromium/sql/transaction.h
+++ src/3rdparty/chromium/sql/transaction.h
@@ -7,7 +7,7 @@
#include "base/check.h"
#include "base/component_export.h"
-#include "base/memory/raw_ref.h"
+#include "base/memory/weak_ptr.h"
#include "base/sequence_checker.h"
#include "base/thread_annotations.h"
@@ -91,7 +91,7 @@ class COMPONENT_EXPORT(SQL) Transaction {
private:
SEQUENCE_CHECKER(sequence_checker_);
- const raw_ref<Database> database_ GUARDED_BY_CONTEXT(sequence_checker_);
+ base::WeakPtr<Database> database_ GUARDED_BY_CONTEXT(sequence_checker_);
#if DCHECK_IS_ON()
bool begin_called_ GUARDED_BY_CONTEXT(sequence_checker_) = false;
diff --git a/chromium/storage/browser/database/database_tracker.cc b/chromium/storage/browser/database/database_tracker.cc
index 2926db89bac4..5dbfcf2d4227 100644
--- src/3rdparty/chromium/storage/browser/database/database_tracker.cc
+++ src/3rdparty/chromium/storage/browser/database/database_tracker.cc
@@ -4,6 +4,7 @@
#include "storage/browser/database/database_tracker.h"
+#include <stddef.h>
#include <stdint.h>
#include <algorithm>
@@ -986,6 +987,13 @@ void DatabaseTracker::Shutdown() {
else if (!force_keep_session_state_)
ClearSessionOnlyOrigins();
CloseTrackerDatabaseAndClearCaches();
+
+ // Explicitly destroy `db_` on the correct sequence rather than waiting for
+ // the destructor, which may run on another sequence. Destroy related fields
+ // first to prevent dangling pointers. Destruction order is important.
+ meta_table_.reset();
+ databases_table_.reset();
+ db_.reset();
}
void DatabaseTracker::SetForceKeepSessionState() {
diff --git a/chromium/storage/browser/database/database_tracker.h b/chromium/storage/browser/database/database_tracker.h
index 17b6aea42766..57d189e338d3 100644
--- src/3rdparty/chromium/storage/browser/database/database_tracker.h
+++ src/3rdparty/chromium/storage/browser/database/database_tracker.h
@@ -77,7 +77,9 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) OriginInfo {
// The data in this class is not thread-safe, so all methods of this class
// should be called on the task runner returned by task_runner(). The only
// exceptions are the constructor, the destructor, and the getters explicitly
-// marked as thread-safe.
+// marked as thread-safe. Although the destructor itself may run on any thread,
+// destruction effectively occurs in Shutdown(), which expects to be called on
+// task_runner().
class COMPONENT_EXPORT(STORAGE_BROWSER) DatabaseTracker
: public base::RefCountedThreadSafe<DatabaseTracker> {
public:
From aea30bc1019793100c4586d499fa7f82ff18613b Mon Sep 17 00:00:00 2001
From: Liam Brady <lbrady@google.com>
Date: Thu, 7 Mar 2024 22:11:26 +0000
Subject: [PATCH] [Backport] CVE-2024-3840: Insufficient policy enforcement in
Site Isolation
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Manual cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/5269464:
Clear user activation on cross-site navigations.
When full site isolation is disabled, renderer processes and
RenderFrameHosts are re-used when performing cross-site navigations.
This includes user activation state, and, more specifically, the sticky
`has_been_active_` bit in `UserActivationState`.
Currently, the `UserActivationState` on the renderer-side is reset only
if the navigation's associated frame is a main frame. That means that if
an iframe navigates to a cross-site page, its sticky user activation
state will be the leftover state from the previous page. So, if a user
interacted with the previous page in any capacity, the newly loaded page
will think it has received a user gesture, essentially using an
unintentional cache of the user activation state.
This becomes an issue when dealing with our framebusting interventions.
We only allow an iframe to do a top-level navigation if it received a
user gesture. However, if an iframe's previous document received a user
activation, or worse, if the iframe was not navigated to anything and
got a user activation because its embedder was interacted with, this
allows the current document to circumvent our framebusting
interventions. The latter happens because of same-origin descendant
activation behavior. See:
https://source.chromium.org/chromium/chromium/src/+/main:content/browser/renderer_host/frame_tree_node.cc;l=766-778;drc=30753b1135fa271a3b45bbdbfef6567e46733a7f;bpv=1;bpt=1
Note that this problem does not exist if site isolation is enabled
(which is by default on desktop platforms), since a cross-site
navigation will create a whole new process with a fresh
`UserActivationState`.
To fix this, this CL clears the user activation state on cross-site
subframe navigations in the renderer (user activation is already cleared
for main frames). To ensure that same-site navigations persist user
state even if a cross-origin or same-origin navigation results in a new
process or RenderFrameHost being created, this CL also explicitly
transfers sticky user activation state for all same-site
cross-RenderFrameHost navigations. This takes place in the browser, and
the resulting bit to determine if a frame should have sticky user
activation is passed to the renderer.
The ultimate end goal is to unconditionally clear the user activation
state for all cross-document navigations. That unfortunately is not
possible today as there are entrenched use cases that rely on sticky
user activation state being cached for same-site navigations. See:
https://crbug.com/40228985.
This CL also fixes the aforementioned regression when enabling the
RenderDocument feature, since this CL will now preserve the sticky user
activation state regardless of what
process/RenderFrameHost/RenderDocument state the navigation results in.
This CL adds some tests to the no-auto-wpt-origin-isolation test suite, which requires some additional description:
* These tests are running on all platforms because site isolation behavior may differ per platform
* All of the tests in the-iframe-element are being added because it would be useful to understand their behavior in all expected process
configurations
* The total time taken for this test suite on linux-rel showed a total time percentage of <0.3%
Bug: 41493458
Change-Id: Ibec11437fcd03470571e04a4e0dfaadffddf6c03
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5269464
Reviewed-by: Mustaq Ahmed <mustaq@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Reviewed-by: Andrew Verge <averge@chromium.org>
Commit-Queue: Liam Brady <lbrady@google.com>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1269856}
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/556966
Reviewed-by: Michael Brüning <michael.bruning@qt.io>
---
.../browser/renderer_host/frame_tree_node.cc | 30 ++++++++---------
.../browser/renderer_host/frame_tree_node.h | 15 ++++-----
.../navigation_controller_impl.cc | 4 ++-
.../renderer_host/navigation_entry_impl.cc | 5 +--
.../renderer_host/navigation_request.cc | 29 ++++++++++++----
.../browser/renderer_host/navigator.cc | 33 +++++++++++++++----
.../renderer_host/render_frame_host_impl.cc | 11 +++++--
.../renderer_host/render_frame_host_impl.h | 3 +-
.../content/renderer/render_frame_impl.cc | 3 ++
.../common/frame/user_activation_state.cc | 4 +++
.../common/frame/user_activation_state.h | 4 +++
.../frame/user_activation_update_types.mojom | 8 +++++
.../mojom/navigation/navigation_params.mojom | 4 +++
.../blink/public/web/web_navigation_params.h | 4 +++
.../blink/renderer/core/frame/frame.cc | 21 ++++++++++--
.../blink/renderer/core/frame/frame.h | 10 +++++-
.../blink/renderer/core/frame/remote_frame.cc | 3 ++
.../renderer/core/loader/document_loader.cc | 21 ++++++++++--
.../renderer/core/loader/document_loader.h | 4 +++
19 files changed, 164 insertions(+), 52 deletions(-)
diff --git a/chromium/content/browser/renderer_host/frame_tree_node.cc b/chromium/content/browser/renderer_host/frame_tree_node.cc
index dd618cfa09e5..1ed5a5e0f004 100644
--- src/3rdparty/chromium/content/browser/renderer_host/frame_tree_node.cc
+++ src/3rdparty/chromium/content/browser/renderer_host/frame_tree_node.cc
@@ -338,19 +338,6 @@ bool FrameTreeNode::IsOutermostMainFrame() const {
return !GetParentOrOuterDocument();
}
-void FrameTreeNode::ResetForNavigation() {
- // This frame has had its user activation bits cleared in the renderer before
- // arriving here. We just need to clear them here and in the other renderer
- // processes that may have a reference to this frame.
- //
- // We do not take user activation into account when calculating
- // |ResetForNavigationResult|, as we are using it to determine bfcache
- // eligibility and the page can get another user gesture after restore.
- UpdateUserActivationState(
- blink::mojom::UserActivationUpdateType::kClearActivation,
- blink::mojom::UserActivationNotificationType::kNone);
-}
-
RenderFrameHostImpl* FrameTreeNode::GetParentOrOuterDocument() const {
return GetParentOrOuterDocumentHelper(/*escape_guest_view=*/false,
/*include_prospective=*/true);
@@ -769,15 +756,22 @@ void FrameTreeNode::BeforeUnloadCanceled() {
}
}
+bool FrameTreeNode::NotifyUserActivationStickyOnly() {
+ return NotifyUserActivation(
+ blink::mojom::UserActivationNotificationType::kNone,
+ /*sticky_only=*/true);
+}
+
bool FrameTreeNode::NotifyUserActivation(
- blink::mojom::UserActivationNotificationType notification_type) {
+ blink::mojom::UserActivationNotificationType notification_type,
+ bool sticky_only) {
// User Activation V2 requires activating all ancestor frames in addition to
// the current frame. See
// https://html.spec.whatwg.org/multipage/interaction.html#tracking-user-activation.
for (RenderFrameHostImpl* rfh = current_frame_host(); rfh;
rfh = rfh->GetParent()) {
rfh->DidReceiveUserActivation();
- rfh->ActivateUserActivation(notification_type);
+ rfh->ActivateUserActivation(notification_type, sticky_only);
}
current_frame_host()->browsing_context_state()->set_has_active_user_gesture(
@@ -792,7 +786,8 @@ bool FrameTreeNode::NotifyUserActivation(
for (FrameTreeNode* node : frame_tree().Nodes()) {
if (node->current_frame_host()->GetLastCommittedOrigin().IsSameOriginWith(
current_origin)) {
- node->current_frame_host()->ActivateUserActivation(notification_type);
+ node->current_frame_host()->ActivateUserActivation(notification_type,
+ sticky_only);
}
}
}
@@ -860,6 +855,9 @@ bool FrameTreeNode::UpdateUserActivationState(
return false;
}
} break;
+ case blink::mojom::UserActivationUpdateType::kNotifyActivationStickyOnly:
+ update_result = NotifyUserActivationStickyOnly();
+ break;
case blink::mojom::UserActivationUpdateType::kClearActivation:
update_result = ClearUserActivation();
break;
diff --git a/chromium/content/browser/renderer_host/frame_tree_node.h b/chromium/content/browser/renderer_host/frame_tree_node.h
index 0edd7b389cc3..284a81e467dd 100644
--- src/3rdparty/chromium/content/browser/renderer_host/frame_tree_node.h
+++ src/3rdparty/chromium/content/browser/renderer_host/frame_tree_node.h
@@ -122,14 +122,6 @@ class CONTENT_EXPORT FrameTreeNode : public RenderFrameHostOwner {
bool IsMainFrame() const;
bool IsOutermostMainFrame() const;
- // Clears any state in this node which was set by the document itself (CSP &
- // UserActivationState) and notifies proxies as appropriate. Invoked after
- // committing navigation to a new document (since the new document comes with
- // a fresh set of CSP).
- // TODO(arthursonzogni): Remove this function. The frame/document must not be
- // left temporarily with lax state.
- void ResetForNavigation();
-
FrameTree& frame_tree() const { return frame_tree_.get(); }
Navigator& navigator();
@@ -772,8 +764,13 @@ class CONTENT_EXPORT FrameTreeNode : public RenderFrameHostOwner {
class OpenerDestroyedObserver;
// The |notification_type| parameter is used for histograms only.
+ // |sticky_only| is set to true when propagating sticky user activation during
+ // cross-document navigations. The transient state remains unchanged.
bool NotifyUserActivation(
- blink::mojom::UserActivationNotificationType notification_type);
+ blink::mojom::UserActivationNotificationType notification_type,
+ bool sticky_only = false);
+
+ bool NotifyUserActivationStickyOnly();
bool ConsumeTransientUserActivation();
diff --git a/chromium/content/browser/renderer_host/navigation_controller_impl.cc b/chromium/content/browser/renderer_host/navigation_controller_impl.cc
index 24203784c631..54e064642bdc 100644
--- src/3rdparty/chromium/content/browser/renderer_host/navigation_controller_impl.cc
+++ src/3rdparty/chromium/content/browser/renderer_host/navigation_controller_impl.cc
@@ -3979,7 +3979,9 @@ NavigationControllerImpl::CreateNavigationRequestFromLoadParams(
/*origin_agent_cluster_left_as_default=*/true,
/*enabled_client_hints=*/
std::vector<network::mojom::WebClientHintsType>(),
- /*is_cross_browsing_instance=*/false, /*old_page_info=*/nullptr,
+ /*is_cross_site_cross_browsing_context_group=*/false,
+ /*should_have_sticky_user_activation=*/false,
+ /*old_page_info=*/nullptr,
/*http_response_code=*/-1,
blink::mojom::NavigationApiHistoryEntryArrays::New(),
/*early_hints_preloaded_resources=*/std::vector<GURL>(),
diff --git a/chromium/content/browser/renderer_host/navigation_entry_impl.cc b/chromium/content/browser/renderer_host/navigation_entry_impl.cc
index 1c318ac954ed..897fb61b887d 100644
--- src/3rdparty/chromium/content/browser/renderer_host/navigation_entry_impl.cc
+++ src/3rdparty/chromium/content/browser/renderer_host/navigation_entry_impl.cc
@@ -942,8 +942,9 @@ NavigationEntryImpl::ConstructCommitNavigationParams(
true /* origin_agent_cluster_left_as_default */,
std::vector<
network::mojom::WebClientHintsType>() /* enabled_client_hints */,
- false /* is_cross_browsing_instance */, nullptr /* old_page_info */,
- -1 /* http_response_code */,
+ false /* is_cross_site_cross_browsing_context_group */,
+ false /* should_have_sticky_user_activation */,
+ nullptr /* old_page_info */, -1 /* http_response_code */,
blink::mojom::NavigationApiHistoryEntryArrays::New(),
std::vector<GURL>() /* early_hints_preloaded_resources */,
// This timestamp will be populated when the commit IPC is sent.
diff --git a/chromium/content/browser/renderer_host/navigation_request.cc b/chromium/content/browser/renderer_host/navigation_request.cc
index da6c714c0c04..923354bdafbf 100644
--- src/3rdparty/chromium/content/browser/renderer_host/navigation_request.cc
+++ src/3rdparty/chromium/content/browser/renderer_host/navigation_request.cc
@@ -1318,7 +1318,8 @@ std::unique_ptr<NavigationRequest> NavigationRequest::CreateRendererInitiated(
/*origin_agent_cluster_left_as_default=*/true,
/*enabled_client_hints=*/
std::vector<network::mojom::WebClientHintsType>(),
- /*is_cross_browsing_instance=*/false,
+ /*is_cross_site_cross_browsing_context_group=*/false,
+ /*should_have_sticky_user_activation=*/false,
/*old_page_info=*/nullptr, /*http_response_code=*/-1,
blink::mojom::NavigationApiHistoryEntryArrays::New(),
/*early_hints_preloaded_resources=*/std::vector<GURL>(),
@@ -1462,7 +1463,8 @@ NavigationRequest::CreateForSynchronousRendererCommit(
/*origin_agent_cluster_left_as_default=*/true,
/*enabled_client_hints=*/
std::vector<network::mojom::WebClientHintsType>(),
- /*is_cross_browsing_instance=*/false,
+ /*is_cross_site_cross_browsing_context_group=*/false,
+ /*should_have_sticky_user_activation=*/false,
/*old_page_info=*/nullptr, http_response_code,
blink::mojom::NavigationApiHistoryEntryArrays::New(),
/*early_hints_preloaded_resources=*/std::vector<GURL>(),
@@ -5599,11 +5601,11 @@ void NavigationRequest::CommitNavigation() {
}
}
+ RenderFrameHostImpl* old_frame_host =
+ frame_tree_node_->render_manager()->current_frame_host();
if (!NavigationTypeUtils::IsSameDocument(common_params_->navigation_type)) {
// We want to record this for the frame that we are navigating away from.
- frame_tree_node_->render_manager()
- ->current_frame_host()
- ->RecordNavigationSuddenTerminationHandlers();
+ old_frame_host->RecordNavigationSuddenTerminationHandlers();
}
if (IsServedFromBackForwardCache() || IsPrerenderedPageActivation()) {
CommitPageActivation();
@@ -5623,8 +5625,7 @@ void NavigationRequest::CommitNavigation() {
if (!weak_self)
return;
- DCHECK(GetRenderFrameHost() ==
- frame_tree_node_->render_manager()->current_frame_host() ||
+ DCHECK(GetRenderFrameHost() == old_frame_host ||
GetRenderFrameHost() ==
frame_tree_node_->render_manager()->speculative_frame_host());
@@ -5699,6 +5700,20 @@ void NavigationRequest::CommitNavigation() {
response(), frame_tree_node_);
}
+ // Sticky user activation should only be preserved for same-site subframe
+ // navigations. This is done to prevent newly navigated documents from
+ // re-using the sticky user activation state from the previously navigated
+ // document in the frame. We persist user activation across same-site
+ // navigations for compatibility reasons, and this does not need to match the
+ // same-site checks used in the process model. See: crbug.com/736415.
+ // TODO(crbug.com/40228985): Remove this once we find a way to reset
+ // activation unconditionally without breaking sites in practice.
+ commit_params_->should_have_sticky_user_activation =
+ !frame_tree_node_->IsMainFrame() &&
+ old_frame_host->HasStickyUserActivation() &&
+ net::SchemefulSite(old_frame_host->GetLastCommittedOrigin()) ==
+ net::SchemefulSite(origin);
+
// Generate a UKM source and track it on NavigationRequest. This will be
// passed down to the blink::Document to be created, if any, and used for UKM
// source creation when navigation has successfully committed.
diff --git a/chromium/content/browser/renderer_host/navigator.cc b/chromium/content/browser/renderer_host/navigator.cc
index 1063f29af344..e4cdb82d5598 100644
--- src/3rdparty/chromium/content/browser/renderer_host/navigator.cc
+++ src/3rdparty/chromium/content/browser/renderer_host/navigator.cc
@@ -47,6 +47,7 @@
#include "content/public/common/content_constants.h"
#include "content/public/common/url_utils.h"
#include "net/base/net_errors.h"
+#include "net/base/schemeful_site.h"
#include "services/metrics/public/cpp/ukm_builders.h"
#include "services/metrics/public/cpp/ukm_recorder.h"
#include "services/metrics/public/cpp/ukm_source_id.h"
@@ -512,6 +513,9 @@ void Navigator::DidNavigate(
was_within_same_document);
}
+ // Store this information before DidNavigateFrame() potentially swaps RFHs.
+ url::Origin old_frame_origin = old_frame_host->GetLastCommittedOrigin();
+
// DidNavigateFrame() must be called before replicating the new origin and
// other properties to proxies. This is because it destroys the subframes of
// the frame we're navigating from, which might trigger those subframes to
@@ -524,6 +528,29 @@ void Navigator::DidNavigate(
.ShouldClearProxiesOnCommit(),
navigation_request->commit_params().frame_policy);
+ // The main frame, same site, and cross-site navigation checks for user
+ // activation mirror the checks in DocumentLoader::CommitNavigation() (note:
+ // CommitNavigation() is not called for same-document navigations, which is
+ // why we have the !was_within_same_document check). This is done to prevent
+ // newly navigated pages from re-using the sticky user activation state from
+ // the previously navigated page in the frame. We persist user activation
+ // across same-site navigations for compatibility reasons with user
+ // activation, and does not need to match the same-site checks used in the
+ // process model. See: crbug.com/736415, and crbug.com/40228985 for the
+ // specific regression that resulted in this requirement.
+ if (!was_within_same_document) {
+ if (!navigation_request->commit_params()
+ .should_have_sticky_user_activation) {
+ frame_tree_node->UpdateUserActivationState(
+ blink::mojom::UserActivationUpdateType::kClearActivation,
+ blink::mojom::UserActivationNotificationType::kNone);
+ } else {
+ frame_tree_node->UpdateUserActivationState(
+ blink::mojom::UserActivationUpdateType::kNotifyActivationStickyOnly,
+ blink::mojom::UserActivationNotificationType::kNone);
+ }
+ }
+
// Save the new page's origin and other properties, and replicate them to
// proxies, including the proxy created in DidNavigateFrame() to replace the
// old frame in cross-process navigation cases.
@@ -534,12 +561,6 @@ void Navigator::DidNavigate(
render_frame_host->browsing_context_state()->SetInsecureNavigationsSet(
params.insecure_navigations_set);
- if (!was_within_same_document) {
- // Navigating to a new location means a new, fresh set of http headers
- // and/or <meta> elements - we need to reset Permissions Policy.
- frame_tree_node->ResetForNavigation();
- }
-
// If the committing URL requires the SiteInstance's site to be assigned,
// that site assignment should've already happened at ReadyToCommit time. We
// should never get here with a SiteInstance that doesn't have a site
diff --git a/chromium/content/browser/renderer_host/render_frame_host_impl.cc b/chromium/content/browser/renderer_host/render_frame_host_impl.cc
index c5f90d4965f9..d1d0efb398ba 100644
--- src/3rdparty/chromium/content/browser/renderer_host/render_frame_host_impl.cc
+++ src/3rdparty/chromium/content/browser/renderer_host/render_frame_host_impl.cc
@@ -6002,9 +6002,14 @@ void RenderFrameHostImpl::ConsumeTransientUserActivation() {
}
void RenderFrameHostImpl::ActivateUserActivation(
- blink::mojom::UserActivationNotificationType notification_type) {
- user_activation_state_.Activate(notification_type);
- history_user_activation_state_.Activate();
+ blink::mojom::UserActivationNotificationType notification_type,
+ bool sticky_only) {
+ if (sticky_only) {
+ user_activation_state_.SetHasBeenActive();
+ } else {
+ user_activation_state_.Activate(notification_type);
+ history_user_activation_state_.Activate();
+ }
}
void RenderFrameHostImpl::ActivateFocusSourceUserActivation() {
diff --git a/chromium/content/browser/renderer_host/render_frame_host_impl.h b/chromium/content/browser/renderer_host/render_frame_host_impl.h
index 378657c0a55d..0a001fb828e1 100644
--- src/3rdparty/chromium/content/browser/renderer_host/render_frame_host_impl.h
+++ src/3rdparty/chromium/content/browser/renderer_host/render_frame_host_impl.h
@@ -2931,7 +2931,8 @@ class CONTENT_EXPORT RenderFrameHostImpl
void ClearUserActivation();
void ConsumeTransientUserActivation();
void ActivateUserActivation(
- blink::mojom::UserActivationNotificationType notification_type);
+ blink::mojom::UserActivationNotificationType notification_type,
+ bool sticky_only = false);
// These are called only when RenderFrameHostOwner is iterating over all
// frames, not directly from the renderer.
diff --git a/chromium/content/renderer/render_frame_impl.cc b/chromium/content/renderer/render_frame_impl.cc
index 5da8ffc96764..13afea50ecba 100644
--- src/3rdparty/chromium/content/renderer/render_frame_impl.cc
+++ src/3rdparty/chromium/content/renderer/render_frame_impl.cc
@@ -981,6 +981,9 @@ void FillMiscNavigationParams(
navigation_params->is_cross_site_cross_browsing_context_group =
commit_params.is_cross_site_cross_browsing_context_group;
+ navigation_params->should_have_sticky_user_activation =
+ commit_params.should_have_sticky_user_activation;
+
#if BUILDFLAG(IS_ANDROID) || defined(TOOLKIT_QT)
// Only android webview uses this.
navigation_params->grant_load_local_resources =
diff --git a/chromium/third_party/blink/common/frame/user_activation_state.cc b/chromium/third_party/blink/common/frame/user_activation_state.cc
index 5804fa8049be..ad85a074d10b 100644
--- src/3rdparty/chromium/third_party/blink/common/frame/user_activation_state.cc
+++ src/3rdparty/chromium/third_party/blink/common/frame/user_activation_state.cc
@@ -47,6 +47,10 @@ void UserActivationState::Activate(
transient_state_expiry_time_for_interaction_ = transient_state_expiry_time_;
}
+void UserActivationState::SetHasBeenActive() {
+ has_been_active_ = true;
+}
+
void UserActivationState::Clear() {
has_been_active_ = false;
last_activation_was_restricted_ = false;
diff --git a/chromium/third_party/blink/public/common/frame/user_activation_state.h b/chromium/third_party/blink/public/common/frame/user_activation_state.h
index fe530372caa0..6e143d2e9795 100644
--- src/3rdparty/chromium/third_party/blink/public/common/frame/user_activation_state.h
+++ src/3rdparty/chromium/third_party/blink/public/common/frame/user_activation_state.h
@@ -111,6 +111,10 @@ class BLINK_COMMON_EXPORT UserActivationState {
// |is_restricted| as a parameter here.
void Activate(mojom::UserActivationNotificationType notification_type);
+ // Used when propagating user activation state across cross-process
+ // navigations.
+ void SetHasBeenActive();
+
void Clear();
// Returns the sticky activation state, which is |true| if the frame has ever
diff --git a/chromium/third_party/blink/public/mojom/frame/user_activation_update_types.mojom b/chromium/third_party/blink/public/mojom/frame/user_activation_update_types.mojom
index 6db97b4a695c..5acd66856df0 100644
--- src/3rdparty/chromium/third_party/blink/public/mojom/frame/user_activation_update_types.mojom
+++ src/3rdparty/chromium/third_party/blink/public/mojom/frame/user_activation_update_types.mojom
@@ -7,8 +7,16 @@ module blink.mojom;
// Types of UserActivationV2 state updates sent between the browser and the
// renderer processes.
enum UserActivationUpdateType {
+ // Used to give a document sticky and transient user activation as a result of
+ // a user gesture.
kNotifyActivation,
kNotifyActivationPendingBrowserVerification,
+ // Used to propagate the sticky user activation state during cross-document
+ // navigations.
+ kNotifyActivationStickyOnly,
+ // Used after a sensitive API is called to prevent abuse of the API.
kConsumeTransientActivation,
+ // Used during cross-document navigations when the user activation state
+ // shouldn't be propagated.
kClearActivation
};
diff --git a/chromium/third_party/blink/public/mojom/navigation/navigation_params.mojom b/chromium/third_party/blink/public/mojom/navigation/navigation_params.mojom
index b568f7442142..3947e8dcaeb3 100644
--- src/3rdparty/chromium/third_party/blink/public/mojom/navigation/navigation_params.mojom
+++ src/3rdparty/chromium/third_party/blink/public/mojom/navigation/navigation_params.mojom
@@ -510,6 +510,10 @@ struct CommitNavigationParams {
// Whether this is a cross-site navigation that swaps BrowsingContextGroups.
bool is_cross_site_cross_browsing_context_group = false;
+ // Whether the new document should start with sticky user activation, because
+ // the previously committed document did, and the navigation was same-site.
+ bool should_have_sticky_user_activation = false;
+
// Should only be set to a valid value for main-frame same-site navigations
// where we did a proactive BrowsingInstance swap and we're reusing the old
// page's process.
diff --git a/chromium/third_party/blink/public/web/web_navigation_params.h b/chromium/third_party/blink/public/web/web_navigation_params.h
index 560c4e985497..206bff0b0042 100644
--- src/3rdparty/chromium/third_party/blink/public/web/web_navigation_params.h
+++ src/3rdparty/chromium/third_party/blink/public/web/web_navigation_params.h
@@ -466,6 +466,10 @@ struct BLINK_EXPORT WebNavigationParams {
// (BrowsingInstances).
bool is_cross_site_cross_browsing_context_group = false;
+ // Whether the new document should start with sticky user activation, because
+ // the previously committed document did, and the navigation was same-site.
+ bool should_have_sticky_user_activation = false;
+
// Blink's copy of the policy container containing security policies to be
// enforced on the document created by this navigation.
std::unique_ptr<WebPolicyContainer> policy_container;
diff --git a/chromium/third_party/blink/renderer/core/frame/frame.cc b/chromium/third_party/blink/renderer/core/frame/frame.cc
index e0ce2a1bcbef..ce18145c67a6 100644
--- src/3rdparty/chromium/third_party/blink/renderer/core/frame/frame.cc
+++ src/3rdparty/chromium/third_party/blink/renderer/core/frame/frame.cc
@@ -286,10 +286,21 @@ void Frame::DidChangeVisibilityState() {
child_frames[i]->DidChangeVisibilityState();
}
+void Frame::NotifyUserActivationInFrameTreeStickyOnly() {
+ NotifyUserActivationInFrameTree(
+ mojom::blink::UserActivationNotificationType::kNone,
+ /*sticky_only=*/true);
+}
+
void Frame::NotifyUserActivationInFrameTree(
- mojom::blink::UserActivationNotificationType notification_type) {
+ mojom::blink::UserActivationNotificationType notification_type,
+ bool sticky_only) {
for (Frame* node = this; node; node = node->Tree().Parent()) {
- node->user_activation_state_.Activate(notification_type);
+ if (sticky_only) {
+ node->user_activation_state_.SetHasBeenActive();
+ } else {
+ node->user_activation_state_.Activate(notification_type);
+ }
node->ActivateHistoryUserActivationState();
}
@@ -307,7 +318,11 @@ void Frame::NotifyUserActivationInFrameTree(
if (local_frame_node &&
security_origin->CanAccess(
local_frame_node->GetSecurityContext()->GetSecurityOrigin())) {
- node->user_activation_state_.Activate(notification_type);
+ if (sticky_only) {
+ node->user_activation_state_.SetHasBeenActive();
+ } else {
+ node->user_activation_state_.Activate(notification_type);
+ }
node->ActivateHistoryUserActivationState();
}
}
diff --git a/chromium/third_party/blink/renderer/core/frame/frame.h b/chromium/third_party/blink/renderer/core/frame/frame.h
index 2487e9a9a98a..3707cff0fa64 100644
--- src/3rdparty/chromium/third_party/blink/renderer/core/frame/frame.h
+++ src/3rdparty/chromium/third_party/blink/renderer/core/frame/frame.h
@@ -256,6 +256,12 @@ class CORE_EXPORT Frame : public GarbageCollected<Frame> {
return user_activation_state_.LastActivationWasRestricted();
}
+ // Sets the sticky user activation state of this frame. This does not change
+ // the transient user activation state.
+ void SetStickyUserActivationState() {
+ user_activation_state_.SetHasBeenActive();
+ }
+
// Resets the user activation state of this frame.
void ClearUserActivation() { user_activation_state_.Clear(); }
@@ -482,8 +488,10 @@ class CORE_EXPORT Frame : public GarbageCollected<Frame> {
void ApplyFrameOwnerProperties(
mojom::blink::FrameOwnerPropertiesPtr properties);
+ void NotifyUserActivationInFrameTreeStickyOnly();
void NotifyUserActivationInFrameTree(
- mojom::blink::UserActivationNotificationType notification_type);
+ mojom::blink::UserActivationNotificationType notification_type,
+ bool sticky_only = false);
bool ConsumeTransientUserActivationInFrameTree();
void ClearUserActivationInFrameTree();
diff --git a/chromium/third_party/blink/renderer/core/frame/remote_frame.cc b/chromium/third_party/blink/renderer/core/frame/remote_frame.cc
index e5d0e24fa1cb..7f3f457261da 100644
--- src/3rdparty/chromium/third_party/blink/renderer/core/frame/remote_frame.cc
+++ src/3rdparty/chromium/third_party/blink/renderer/core/frame/remote_frame.cc
@@ -665,6 +665,9 @@ void RemoteFrame::UpdateUserActivationState(
case mojom::blink::UserActivationUpdateType::kNotifyActivation:
NotifyUserActivationInFrameTree(notification_type);
break;
+ case mojom::blink::UserActivationUpdateType::kNotifyActivationStickyOnly:
+ NotifyUserActivationInFrameTreeStickyOnly();
+ break;
case mojom::blink::UserActivationUpdateType::kConsumeTransientActivation:
ConsumeTransientUserActivationInFrameTree();
break;
diff --git a/chromium/third_party/blink/renderer/core/loader/document_loader.cc b/chromium/third_party/blink/renderer/core/loader/document_loader.cc
index b7b130e869fe..b29ab01b9f84 100644
--- src/3rdparty/chromium/third_party/blink/renderer/core/loader/document_loader.cc
+++ src/3rdparty/chromium/third_party/blink/renderer/core/loader/document_loader.cc
@@ -305,6 +305,7 @@ struct SameSizeAsDocumentLoader
bool origin_agent_cluster;
bool origin_agent_cluster_left_as_default;
bool is_cross_site_cross_browsing_context_group;
+ bool should_have_sticky_user_activation;
WebVector<WebHistoryItem> navigation_api_back_entries;
WebVector<WebHistoryItem> navigation_api_forward_entries;
std::unique_ptr<CodeCacheHost> code_cache_host;
@@ -512,6 +513,8 @@ DocumentLoader::DocumentLoader(
params_->origin_agent_cluster_left_as_default),
is_cross_site_cross_browsing_context_group_(
params_->is_cross_site_cross_browsing_context_group),
+ should_have_sticky_user_activation_(
+ params_->should_have_sticky_user_activation),
navigation_api_back_entries_(params_->navigation_api_back_entries),
navigation_api_forward_entries_(params_->navigation_api_forward_entries),
extra_data_(std::move(extra_data)),
@@ -651,6 +654,9 @@ DocumentLoader::CreateWebNavigationParamsToCloneDocument() {
params->document_ukm_source_id = ukm_source_id_;
params->is_cross_site_cross_browsing_context_group =
is_cross_site_cross_browsing_context_group_;
+ // Required for javascript: URL commits to propagate sticky user activation.
+ params->should_have_sticky_user_activation =
+ frame_->HasStickyUserActivation() && !frame_->IsMainFrame();
params->has_text_fragment_token = has_text_fragment_token_;
// Origin trials must still work on the cloned document.
params->initiator_origin_trial_features =
@@ -2554,10 +2560,19 @@ void DocumentLoader::CommitNavigation() {
frame_->ClearScrollSnapshotClients();
- // Clear the user activation state.
+ // Determine whether to give the frame sticky user activation. These checks
+ // mirror the check in Navigator::DidNavigate(). Main frame navigations and
+ // cross-site navigations should not hold on to the sticky user activation
+ // state of the previously navigated page. Same-site navigations should retain
+ // the previous document's sticky user activation state, regardless of whether
+ // the navigation resulted in a new process being created.
+ // See: crbug.com/41493458
// TODO(crbug.com/736415): Clear this bit unconditionally for all frames.
- if (frame_->IsMainFrame())
- frame_->ClearUserActivation();
+ if (!should_have_sticky_user_activation_) {
+ frame_->ClearUserActivation();
+ } else {
+ frame_->SetStickyUserActivationState();
+ }
// The DocumentLoader was flagged as activated if it needs to notify the frame
// that it was activated before navigation. Update the frame state based on
diff --git a/chromium/third_party/blink/renderer/core/loader/document_loader.h b/chromium/third_party/blink/renderer/core/loader/document_loader.h
index 1548bbee1845..ca1758eef3f4 100644
--- src/3rdparty/chromium/third_party/blink/renderer/core/loader/document_loader.h
+++ src/3rdparty/chromium/third_party/blink/renderer/core/loader/document_loader.h
@@ -783,6 +783,10 @@ class CORE_EXPORT DocumentLoader : public GarbageCollected<DocumentLoader>,
// BrowsingContextGroup.
bool is_cross_site_cross_browsing_context_group_ = false;
+ // Whether the new document should start with sticky user activation, because
+ // the previously committed document did, and the navigation was same-site.
+ bool should_have_sticky_user_activation_ = false;
+
WebVector<WebHistoryItem> navigation_api_back_entries_;
WebVector<WebHistoryItem> navigation_api_forward_entries_;
From 6fd09d7a49f15e12264f25a54db9fe189b235d16 Mon Sep 17 00:00:00 2001
From: Ayu Ishii <ayui@chromium.org>
Date: Thu, 21 Mar 2024 20:06:29 +0000
Subject: [PATCH] [Backport] Security bug 323898565
Cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/5288855:
WebSQL: Remove session data logic
On Profile destruction, currently there is a race with
the DatabaseTracker shutdown. DatabaseTracker shutdown
will post task[1] on the database task runner because
it needs to do file I/O. However it also has some logic
to retrieve info for special storage policies from the
Profile which may already be destroyed by that time.
This change removes logic for special storage policy from
DatabaseTracker. DatabaseTracker(WebSQL) is removed
from all platforms as of M119 except for WebView, which does
not utilize the special storage policy. All policy and
deprecation support have ended with M123. Therefore cleaning
up the code to avoid the race.
[1]https://source.chromium.org/chromium/chromium/src/+/main:content/browser/storage_partition_impl.cc;l=1200;drc=be92f4cc2f137460213d52a926c9477275a456c5
Bug: 323898565, 325476286
Change-Id: Ie2ef898c558308439a8b0d3fdf67f7157440b20a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5288855
Commit-Queue: Ayu Ishii <ayui@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1276451}
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/557063
Reviewed-by: Michal Klocek <michal.klocek@qt.io>
---
chromium/content/browser/browser_context.cc | 8 ---
.../content/browser/storage_partition_impl.cc | 3 +-
.../browser/database/database_tracker.cc | 65 +------------------
.../browser/database/database_tracker.h | 15 -----
4 files changed, 3 insertions(+), 88 deletions(-)
diff --git a/chromium/content/browser/browser_context.cc b/chromium/content/browser/browser_context.cc
index c07c16f62e3..6b952d4572f 100644
--- src/3rdparty/chromium/content/browser/browser_context.cc
+++ src/3rdparty/chromium/content/browser/browser_context.cc
@@ -56,7 +56,6 @@
#include "media/mojo/services/video_decode_perf_history.h"
#include "media/mojo/services/webrtc_video_perf_history.h"
#include "storage/browser/blob/blob_storage_context.h"
-#include "storage/browser/database/database_tracker.h"
#include "storage/browser/file_system/external_mount_points.h"
#include "third_party/blink/public/mojom/push_messaging/push_messaging.mojom.h"
#include "third_party/perfetto/include/perfetto/tracing/traced_proto.h"
@@ -252,13 +251,6 @@ void BrowserContext::EnsureResourceContextInitialized() {
void BrowserContext::SaveSessionState() {
StoragePartition* storage_partition = GetDefaultStoragePartition();
- storage::DatabaseTracker* database_tracker =
- storage_partition->GetDatabaseTracker();
- database_tracker->task_runner()->PostTask(
- FROM_HERE,
- base::BindOnce(&storage::DatabaseTracker::SetForceKeepSessionState,
- base::WrapRefCounted(database_tracker)));
-
storage_partition->GetCookieManagerForBrowserProcess()
->SetForceKeepSessionState();
diff --git a/chromium/content/browser/storage_partition_impl.cc b/chromium/content/browser/storage_partition_impl.cc
index 3147e6d0371..12ef04e2d8f 100644
--- src/3rdparty/chromium/content/browser/storage_partition_impl.cc
+++ src/3rdparty/chromium/content/browser/storage_partition_impl.cc
@@ -1371,8 +1371,7 @@ void StoragePartitionImpl::Initialize(
browser_context_, partition_path_, is_in_memory(), quota_manager_proxy);
database_tracker_ = storage::DatabaseTracker::Create(
- partition_path_, is_in_memory(),
- browser_context_->GetSpecialStoragePolicy(), quota_manager_proxy);
+ partition_path_, is_in_memory(), quota_manager_proxy);
dom_storage_context_ = DOMStorageContextWrapper::Create(
this, browser_context_->GetSpecialStoragePolicy());
diff --git a/chromium/storage/browser/database/database_tracker.cc b/chromium/storage/browser/database/database_tracker.cc
index 5dbfcf2d422..865661ea56e 100644
--- src/3rdparty/chromium/storage/browser/database/database_tracker.cc
+++ src/3rdparty/chromium/storage/browser/database/database_tracker.cc
@@ -42,7 +42,6 @@
#include "storage/browser/database/databases_table.h"
#include "storage/browser/quota/quota_client_type.h"
#include "storage/browser/quota/quota_manager_proxy.h"
-#include "storage/browser/quota/special_storage_policy.h"
#include "storage/common/database/database_identifier.h"
#include "third_party/blink/public/common/storage_key/storage_key.h"
#include "third_party/blink/public/mojom/quota/quota_types.mojom.h"
@@ -73,12 +72,6 @@ OriginInfo::OriginInfo(const OriginInfo& origin_info) = default;
OriginInfo::~OriginInfo() = default;
-void OriginInfo::GetAllDatabaseNames(
- std::vector<std::u16string>* databases) const {
- for (const auto& name_and_size : database_sizes_)
- databases->push_back(name_and_size.first);
-}
-
int64_t OriginInfo::GetDatabaseSize(const std::u16string& database_name) const {
auto it = database_sizes_.find(database_name);
if (it != database_sizes_.end())
@@ -92,11 +85,10 @@ OriginInfo::OriginInfo(const std::string& origin_identifier, int64_t total_size)
scoped_refptr<DatabaseTracker> DatabaseTracker::Create(
const base::FilePath& profile_path,
bool is_incognito,
- scoped_refptr<SpecialStoragePolicy> special_storage_policy,
scoped_refptr<QuotaManagerProxy> quota_manager_proxy) {
auto database_tracker = base::MakeRefCounted<DatabaseTracker>(
- profile_path, is_incognito, std::move(special_storage_policy),
- std::move(quota_manager_proxy), base::PassKey<DatabaseTracker>());
+ profile_path, is_incognito, std::move(quota_manager_proxy),
+ base::PassKey<DatabaseTracker>());
database_tracker->RegisterQuotaClient();
return database_tracker;
}
@@ -104,7 +96,6 @@ scoped_refptr<DatabaseTracker> DatabaseTracker::Create(
DatabaseTracker::DatabaseTracker(
const base::FilePath& profile_path,
bool is_incognito,
- scoped_refptr<SpecialStoragePolicy> special_storage_policy,
scoped_refptr<QuotaManagerProxy> quota_manager_proxy,
base::PassKey<DatabaseTracker>)
: is_incognito_(is_incognito),
@@ -117,7 +108,6 @@ DatabaseTracker::DatabaseTracker(
.page_size = 4096,
.cache_size = 500,
})),
- special_storage_policy_(std::move(special_storage_policy)),
quota_manager_proxy_(std::move(quota_manager_proxy)),
task_runner_(base::ThreadPool::CreateSequencedTaskRunner(
{base::MayBlock(), base::TaskPriority::USER_VISIBLE,
@@ -791,12 +781,6 @@ void DatabaseTracker::DeleteDataModifiedSince(
DatabaseSet to_be_deleted;
int rv = net::OK;
for (const auto& origin : origins_identifiers) {
- if (special_storage_policy_.get() &&
- special_storage_policy_->IsStorageProtected(
- GetOriginURLFromIdentifier(origin))) {
- continue;
- }
-
std::vector<DatabaseDetails> details;
if (!databases_table_->GetAllDatabaseDetailsForOriginIdentifier(origin,
&details)) {
@@ -930,44 +914,6 @@ void DatabaseTracker::DeleteIncognitoDBDirectory() {
base::DeletePathRecursively(incognito_db_dir);
}
-void DatabaseTracker::ClearSessionOnlyOrigins() {
- DCHECK(task_runner_->RunsTasksInCurrentSequence());
- bool has_session_only_databases =
- special_storage_policy_.get() &&
- special_storage_policy_->HasSessionOnlyOrigins();
-
- // Clearing only session-only databases, and there are none.
- if (!has_session_only_databases)
- return;
-
- if (!LazyInit())
- return;
-
- std::vector<std::string> origin_identifiers;
- GetAllOriginIdentifiers(&origin_identifiers);
-
- for (const auto& origin : origin_identifiers) {
- GURL origin_url = GetOriginURLFromIdentifier(origin);
- if (!special_storage_policy_->IsStorageSessionOnly(origin_url))
- continue;
- if (special_storage_policy_->IsStorageProtected(origin_url))
- continue;
- OriginInfo origin_info;
- std::vector<std::u16string> databases;
- GetOriginInfo(origin, &origin_info);
- origin_info.GetAllDatabaseNames(&databases);
-
- for (const auto& database : databases) {
- base::File file(
- GetFullDBFilePath(origin, database),
- base::File::FLAG_OPEN_ALWAYS | base::File::FLAG_WIN_SHARE_DELETE |
- base::File::FLAG_DELETE_ON_CLOSE | base::File::FLAG_READ);
- }
- DeleteOrigin(origin, true);
- }
-}
-
-
void DatabaseTracker::Shutdown() {
DCHECK(task_runner_->RunsTasksInCurrentSequence());
if (shutting_down_) {
@@ -984,8 +930,6 @@ void DatabaseTracker::Shutdown() {
if (is_incognito_)
DeleteIncognitoDBDirectory();
- else if (!force_keep_session_state_)
- ClearSessionOnlyOrigins();
CloseTrackerDatabaseAndClearCaches();
// Explicitly destroy `db_` on the correct sequence rather than waiting for
@@ -996,9 +940,4 @@ void DatabaseTracker::Shutdown() {
db_.reset();
}
-void DatabaseTracker::SetForceKeepSessionState() {
- DCHECK(task_runner_->RunsTasksInCurrentSequence());
- force_keep_session_state_ = true;
-}
-
} // namespace storage
diff --git a/chromium/storage/browser/database/database_tracker.h b/chromium/storage/browser/database/database_tracker.h
index 57d189e338d..bc55a0b020a 100644
--- src/3rdparty/chromium/storage/browser/database/database_tracker.h
+++ src/3rdparty/chromium/storage/browser/database/database_tracker.h
@@ -41,7 +41,6 @@ namespace storage {
class DatabaseQuotaClient;
class QuotaClientCallbackWrapper;
class QuotaManagerProxy;
-class SpecialStoragePolicy;
COMPONENT_EXPORT(STORAGE_BROWSER)
extern const base::FilePath::CharType kDatabaseDirectoryName[];
@@ -101,13 +100,11 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) DatabaseTracker
static scoped_refptr<DatabaseTracker> Create(
const base::FilePath& profile_path,
bool is_incognito,
- scoped_refptr<SpecialStoragePolicy> special_storage_policy,
scoped_refptr<QuotaManagerProxy> quota_manager_proxy);
// Exposed for base::MakeRefCounted. Users should call Create().
DatabaseTracker(const base::FilePath& profile_path,
bool is_incognito,
- scoped_refptr<SpecialStoragePolicy> special_storage_policy,
scoped_refptr<QuotaManagerProxy> quota_manager_proxy,
base::PassKey<DatabaseTracker>);
@@ -168,9 +165,6 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) DatabaseTracker
// Deletes databases touched since `cutoff`.
//
- // Does not delete databases belonging to origins designated as protected by
- // the SpecialStoragePolicy passed to the DatabaseTracker constructor.
- //
// `callback` must must be non-null, and is invoked upon completion with a
// net::Error. The status will be net::OK on success, or net::FAILED if not
// all databases could be deleted. `callback` may be called before this method
@@ -198,8 +192,6 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) DatabaseTracker
// Shutdown the database tracker, deleting database files if the tracker is
// used for an Incognito profile.
void Shutdown();
- // Disables the exit-time deletion of session-only data.
- void SetForceKeepSessionState();
protected:
// Subclasses need PassKeys to call the constructor.
@@ -248,9 +240,6 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) DatabaseTracker
// exists.
void DeleteIncognitoDBDirectory();
- // Deletes session-only databases. Blocks databases from being created/opened.
- void ClearSessionOnlyOrigins();
-
bool DeleteClosedDatabase(const std::string& origin_identifier,
const std::u16string& database_name);
@@ -303,7 +292,6 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) DatabaseTracker
bool is_initialized_ = false;
const bool is_incognito_;
- bool force_keep_session_state_ = false;
bool shutting_down_ = false;
const base::FilePath profile_path_;
@@ -324,9 +312,6 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) DatabaseTracker
std::vector<std::pair<net::CompletionOnceCallback, DatabaseSet>>
deletion_callbacks_;
- // Apps and Extensions can have special rights.
- const scoped_refptr<SpecialStoragePolicy> special_storage_policy_;
-
// Can be accessed from any thread via quota_manager_proxy().
//
// Thread-safety argument: The reference is immutable.
From 3ef0cdbadfc403513047464d18e63fc952a43f7b Mon Sep 17 00:00:00 2001
From: Shahbaz Youssefi <syoussefi@chromium.org>
Date: Mon, 8 Apr 2024 10:14:45 -0400
Subject: [PATCH] [Backport] CVE-2024-4058: Type Confusion in ANGLE
Cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/angle/angle/+/5462388:
SPIR-V: Fix const constructors with single scalar
These constructors may be generated because of
RemoveArrayLengthTraverser.
Bug: chromium:332546345
Change-Id: I2b2bf3728ef5bae148abc2a8518f8f3f42850025
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5462388
(cherry picked from commit 0b776d32f69a932acb61963d9daad9e13f610944)
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5466390
Commit-Queue: Zakhar Voit <voit@google.com>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Reviewed-by: Geoff Lang <geofflang@chromium.org>
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/557315
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
---
.../third_party/angle/src/compiler/translator/Compiler.cpp | 5 +++++
.../angle/src/compiler/translator/spirv/OutputSPIRV.cpp | 2 ++
2 files changed, 7 insertions(+)
diff --git a/chromium/third_party/angle/src/compiler/translator/Compiler.cpp b/chromium/third_party/angle/src/compiler/translator/Compiler.cpp
index c70c419631a..c846c9b33fa 100644
--- src/3rdparty/chromium/third_party/angle/src/compiler/translator/Compiler.cpp
+++ src/3rdparty/chromium/third_party/angle/src/compiler/translator/Compiler.cpp
@@ -1034,6 +1034,11 @@ bool TCompiler::checkAndSimplifyAST(TIntermBlock *root,
{
return false;
}
+ // Fold the expressions again, because |RemoveArrayLengthMethod| can introduce new constants.
+ if (!FoldExpressions(this, root, &mDiagnostics))
+ {
+ return false;
+ }
if (!RemoveUnreferencedVariables(this, root, &mSymbolTable))
{
diff --git a/chromium/third_party/angle/src/compiler/translator/spirv/OutputSPIRV.cpp b/chromium/third_party/angle/src/compiler/translator/spirv/OutputSPIRV.cpp
index ed3aac4e432..4c3d1d000c6 100644
--- src/3rdparty/chromium/third_party/angle/src/compiler/translator/spirv/OutputSPIRV.cpp
+++ src/3rdparty/chromium/third_party/angle/src/compiler/translator/spirv/OutputSPIRV.cpp
@@ -1335,6 +1335,8 @@ spirv::IdRef OutputSPIRVTraverser::createComplexConstant(const TType &type,
if (type.isMatrix() && !type.isArray())
{
+ ASSERT(parameters.size() == type.getRows() * type.getCols());
+
// Matrices are constructed from their columns.
spirv::IdRefList columnIds;
From b0ff4b16e877e3787a7200b71440c9f2be7fe1d7 Mon Sep 17 00:00:00 2001
From: Antonio Maiorano <amaiorano@google.com>
Date: Thu, 18 Apr 2024 13:07:04 -0400
Subject: [PATCH] [Backport] CVE-2024-4060: Use after free in Dawn
Cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/external/github.com/microsoft/DirectXShaderCompiler/+/5464347:
Replace dynamic_cast with virtual call (#6515)
Make TextDiagnosticPrinter::setPrefix a virtual function in base class
DiagnosticConsumer. This allows us to avoid using dynamic_cast in
BackendConsumer::DxilDiagHandler, required for codebases that do not
enable RTTI. This is also the only place in the codebase that uses RTTI
(AFAICT).
Bug: chromium:333420620
Change-Id: Ida73077f24fdb4b705b5d868b04ac6cfecb30327
Reviewed-on: https://chromium-review.googlesource.com/c/external/github.com/microsoft/DirectXShaderCompiler/+/5464347
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Reviewed-by: David Neto <dneto@google.com>
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/557316
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
---
.../dxc/tools/clang/include/clang/Basic/Diagnostic.h | 2 ++
.../tools/clang/include/clang/Frontend/TextDiagnosticPrinter.h | 3 ++-
.../third_party/dxc/tools/clang/lib/CodeGen/CodeGenAction.cpp | 2 +-
3 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/chromium/third_party/dawn/third_party/dxc/tools/clang/include/clang/Basic/Diagnostic.h b/chromium/third_party/dawn/third_party/dxc/tools/clang/include/clang/Basic/Diagnostic.h
index cf56e3a7208..b6f804012c5 100644
--- src/3rdparty/chromium/third_party/dawn/third_party/dxc/tools/clang/include/clang/Basic/Diagnostic.h
+++ src/3rdparty/chromium/third_party/dawn/third_party/dxc/tools/clang/include/clang/Basic/Diagnostic.h
@@ -1395,6 +1395,8 @@ class DiagnosticConsumer {
/// warnings and errors.
virtual void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
const Diagnostic &Info);
+
+ virtual void setPrefix(std::string Value) {} // HLSL Change
};
/// \brief A diagnostic client that ignores all diagnostics.
diff --git a/chromium/third_party/dawn/third_party/dxc/tools/clang/include/clang/Frontend/TextDiagnosticPrinter.h b/chromium/third_party/dawn/third_party/dxc/tools/clang/include/clang/Frontend/TextDiagnosticPrinter.h
index 04a570559fe..936031e0967 100644
--- src/3rdparty/chromium/third_party/dawn/third_party/dxc/tools/clang/include/clang/Frontend/TextDiagnosticPrinter.h
+++ src/3rdparty/chromium/third_party/dawn/third_party/dxc/tools/clang/include/clang/Frontend/TextDiagnosticPrinter.h
@@ -45,7 +45,8 @@ class TextDiagnosticPrinter : public DiagnosticConsumer {
/// setPrefix - Set the diagnostic printer prefix string, which will be
/// printed at the start of any diagnostics. If empty, no prefix string is
/// used.
- void setPrefix(std::string Value) { Prefix = Value; }
+ // HLSL Change: add override
+ void setPrefix(std::string Value) override { Prefix = Value; }
void BeginSourceFile(const LangOptions &LO, const Preprocessor *PP) override;
void EndSourceFile() override;
diff --git a/chromium/third_party/dawn/third_party/dxc/tools/clang/lib/CodeGen/CodeGenAction.cpp b/chromium/third_party/dawn/third_party/dxc/tools/clang/lib/CodeGen/CodeGenAction.cpp
index 4fa721e8122..68ebaadf5a8 100644
--- src/3rdparty/chromium/third_party/dawn/third_party/dxc/tools/clang/lib/CodeGen/CodeGenAction.cpp
+++ src/3rdparty/chromium/third_party/dawn/third_party/dxc/tools/clang/lib/CodeGen/CodeGenAction.cpp
@@ -557,7 +557,7 @@ BackendConsumer::DxilDiagHandler(const llvm::DiagnosticInfoDxil &D) {
// If no location information is available, add function name
if (Loc.isInvalid()) {
- auto *DiagClient = dynamic_cast<TextDiagnosticPrinter*>(Diags.getClient());
+ auto *DiagClient = Diags.getClient();
auto *func = D.getFunction();
if (DiagClient && func)
DiagClient->setPrefix("Function: " + func->getName().str());
From b916ca00cc5222253ac5860c3e612ccf00d899c3 Mon Sep 17 00:00:00 2001
From: Nina Satragno <nsatragno@chromium.org>
Date: Mon, 8 Apr 2024 20:07:43 +0000
Subject: [PATCH] [Backport] Security bug 332724843
Partial manual backport of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/5426374:
[webauthn] Don't keep network contexts around
Instead of having fido code pass network contexts around, pass a factory
and get a fresh one every time one is needed.
Keep cable transact* methods that take a network context around until
we clean up the android cable client module in a follow-up.
Bug: 332724843
Change-Id: I33ea5c741706041c75c10cf881452fcf77fce445
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5426374
Reviewed-by: Adam Langley <agl@chromium.org>
Commit-Queue: Nina Satragno <nsatragno@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1284063}
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/557317
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
---
chromium/device/fido/BUILD.gn | 1 +
.../device/fido/cable/fido_tunnel_device.cc | 9 +--
.../device/fido/cable/fido_tunnel_device.h | 5 +-
.../device/fido/cable/v2_authenticator.cc | 66 +++++++++++++++----
chromium/device/fido/cable/v2_authenticator.h | 28 +++++++-
chromium/device/fido/cable/v2_discovery.cc | 10 +--
chromium/device/fido/cable/v2_discovery.h | 5 +-
.../device/fido/fido_discovery_factory.cc | 7 +-
chromium/device/fido/fido_discovery_factory.h | 8 ++-
.../device/fido/network_context_factory.h | 20 ++++++
10 files changed, 122 insertions(+), 37 deletions(-)
create mode 100644 chromium/device/fido/network_context_factory.h
diff --git a/chromium/device/fido/BUILD.gn b/chromium/device/fido/BUILD.gn
index 8e168df02ef..be11e40a4ce 100644
--- src/3rdparty/chromium/device/fido/BUILD.gn
+++ src/3rdparty/chromium/device/fido/BUILD.gn
@@ -42,6 +42,7 @@ component("fido") {
"fido_transport_protocol.h",
"json_request.cc",
"json_request.h",
+ "network_context_factory.h",
"opaque_attestation_statement.cc",
"opaque_attestation_statement.h",
"p256_public_key.cc",
diff --git a/chromium/device/fido/cable/fido_tunnel_device.cc b/chromium/device/fido/cable/fido_tunnel_device.cc
index ad35e6ee176..bd10a4793bb 100644
--- src/3rdparty/chromium/device/fido/cable/fido_tunnel_device.cc
+++ src/3rdparty/chromium/device/fido/cable/fido_tunnel_device.cc
@@ -17,6 +17,7 @@
#include "device/fido/features.h"
#include "device/fido/fido_constants.h"
#include "device/fido/fido_parsing_utils.h"
+#include "device/fido/network_context_factory.h"
#include "net/traffic_annotation/network_traffic_annotation.h"
#include "third_party/boringssl/src/include/openssl/aes.h"
#include "third_party/boringssl/src/include/openssl/digest.h"
@@ -96,7 +97,7 @@ constexpr net::NetworkTrafficAnnotationTag kTrafficAnnotation =
})");
FidoTunnelDevice::FidoTunnelDevice(
- network::mojom::NetworkContext* network_context,
+ NetworkContextFactory network_context_factory,
absl::optional<base::RepeatingCallback<void(std::unique_ptr<Pairing>)>>
pairing_callback,
absl::optional<base::RepeatingCallback<void(Event)>> event_callback,
@@ -130,7 +131,7 @@ FidoTunnelDevice::FidoTunnelDevice(
base::BindOnce(&FidoTunnelDevice::OnTunnelReady, base::Unretained(this)),
base::BindRepeating(&FidoTunnelDevice::OnTunnelData,
base::Unretained(this)));
- network_context->CreateWebSocket(
+ network_context_factory.Run()->CreateWebSocket(
url, {kCableWebSocketProtocol}, net::SiteForCookies(),
net::IsolationInfo(), /*additional_headers=*/{},
network::mojom::kBrowserProcessId, url::Origin::Create(url),
@@ -145,7 +146,7 @@ FidoTunnelDevice::FidoTunnelDevice(
FidoTunnelDevice::FidoTunnelDevice(
FidoRequestType request_type,
- network::mojom::NetworkContext* network_context,
+ NetworkContextFactory network_context_factory,
std::unique_ptr<Pairing> pairing,
base::OnceClosure pairing_is_invalid,
absl::optional<base::RepeatingCallback<void(Event)>> event_callback)
@@ -187,7 +188,7 @@ FidoTunnelDevice::FidoTunnelDevice(
headers.emplace_back(
network::mojom::HttpHeader::New(kCableSignalConnectionHeader, "true"));
}
- network_context->CreateWebSocket(
+ network_context_factory.Run()->CreateWebSocket(
url, {kCableWebSocketProtocol}, net::SiteForCookies(),
net::IsolationInfo(), std::move(headers),
network::mojom::kBrowserProcessId, url::Origin::Create(url),
diff --git a/chromium/device/fido/cable/fido_tunnel_device.h b/chromium/device/fido/cable/fido_tunnel_device.h
index 1d0a7cb3372..840ecc34232 100644
--- src/3rdparty/chromium/device/fido/cable/fido_tunnel_device.h
+++ src/3rdparty/chromium/device/fido/cable/fido_tunnel_device.h
@@ -17,6 +17,7 @@
#include "device/fido/cable/websocket_adapter.h"
#include "device/fido/fido_constants.h"
#include "device/fido/fido_device.h"
+#include "device/fido/network_context_factory.h"
#include "third_party/abseil-cpp/absl/types/variant.h"
namespace network::mojom {
@@ -33,7 +34,7 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoTunnelDevice : public FidoDevice {
public:
// This constructor is used for QR-initiated connections.
FidoTunnelDevice(
- network::mojom::NetworkContext* network_context,
+ NetworkContextFactory network_context_factory,
absl::optional<base::RepeatingCallback<void(std::unique_ptr<Pairing>)>>
pairing_callback,
absl::optional<base::RepeatingCallback<void(Event)>> event_callback,
@@ -47,7 +48,7 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoTunnelDevice : public FidoDevice {
// run.
FidoTunnelDevice(
FidoRequestType request_type,
- network::mojom::NetworkContext* network_context,
+ NetworkContextFactory network_context_factory,
std::unique_ptr<Pairing> pairing,
base::OnceClosure pairing_is_invalid,
absl::optional<base::RepeatingCallback<void(Event)>> event_callback);
diff --git a/chromium/device/fido/cable/v2_authenticator.cc b/chromium/device/fido/cable/v2_authenticator.cc
index e4bd3244cb3..faf8915ec10 100644
--- src/3rdparty/chromium/device/fido/cable/v2_authenticator.cc
+++ src/3rdparty/chromium/device/fido/cable/v2_authenticator.cc
@@ -23,6 +23,7 @@
#include "device/fido/features.h"
#include "device/fido/fido_constants.h"
#include "device/fido/fido_parsing_utils.h"
+#include "device/fido/network_context_factory.h"
#include "device/fido/public_key_credential_descriptor.h"
#include "device/fido/public_key_credential_params.h"
#include "device/fido/public_key_credential_rp_entity.h"
@@ -289,7 +290,7 @@ class TunnelTransport : public Transport {
public:
TunnelTransport(
Platform* platform,
- network::mojom::NetworkContext* network_context,
+ NetworkContextFactory network_context_factory,
base::span<const uint8_t> secret,
base::span<const uint8_t, device::kP256X962Length> peer_identity,
GeneratePairingDataCallback generate_pairing_data)
@@ -302,7 +303,7 @@ class TunnelTransport : public Transport {
secret,
base::span<const uint8_t>(),
device::cablev2::DerivedValueType::kEIDKey)),
- network_context_(network_context),
+ network_context_factory_(std::move(network_context_factory)),
peer_identity_(device::fido_parsing_utils::Materialize(peer_identity)),
generate_pairing_data_(std::move(generate_pairing_data)),
secret_(fido_parsing_utils::Materialize(secret)) {
@@ -319,7 +320,7 @@ class TunnelTransport : public Transport {
TunnelTransport(
Platform* platform,
- network::mojom::NetworkContext* network_context,
+ NetworkContextFactory network_context_factory,
base::span<const uint8_t> secret,
base::span<const uint8_t, device::cablev2::kClientNonceSize> client_nonce,
std::array<uint8_t, device::cablev2::kRoutingIdSize> routing_id,
@@ -331,7 +332,7 @@ class TunnelTransport : public Transport {
secret,
client_nonce,
device::cablev2::DerivedValueType::kEIDKey)),
- network_context_(network_context),
+ network_context_factory_(network_context_factory),
secret_(fido_parsing_utils::Materialize(secret)),
local_identity_(std::move(local_identity)) {
DCHECK_EQ(state_, State::kNone);
@@ -390,7 +391,7 @@ class TunnelTransport : public Transport {
void StartWebSocket() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
- network_context_->CreateWebSocket(
+ network_context_factory_.Run()->CreateWebSocket(
target_, {device::kCableWebSocketProtocol}, net::SiteForCookies(),
net::IsolationInfo(), /*additional_headers=*/{},
network::mojom::kBrowserProcessId, url::Origin::Create(target_),
@@ -623,7 +624,7 @@ class TunnelTransport : public Transport {
const std::array<uint8_t, kEIDKeySize> eid_key_;
std::unique_ptr<WebSocketAdapter> websocket_client_;
std::unique_ptr<Crypter> crypter_;
- const raw_ptr<network::mojom::NetworkContext> network_context_;
+ NetworkContextFactory network_context_factory_;
const absl::optional<std::array<uint8_t, kP256X962Length>> peer_identity_;
std::array<uint8_t, kPSKSize> psk_;
GeneratePairingDataCallback generate_pairing_data_;
@@ -1238,7 +1239,7 @@ std::unique_ptr<Transaction> TransactWithPlaintextTransport(
std::unique_ptr<Transaction> TransactFromQRCode(
std::unique_ptr<Platform> platform,
- network::mojom::NetworkContext* network_context,
+ NetworkContextFactory network_context_factory,
base::span<const uint8_t, kRootSecretSize> root_secret,
const std::string& authenticator_name,
base::span<const uint8_t, 16> qr_secret,
@@ -1249,16 +1250,34 @@ std::unique_ptr<Transaction> TransactFromQRCode(
Platform* const platform_ptr = platform.get();
return std::make_unique<CTAP2Processor>(
- std::make_unique<TunnelTransport>(platform_ptr, network_context,
- qr_secret, peer_identity,
- std::move(generate_pairing_data)),
+ std::make_unique<TunnelTransport>(
+ platform_ptr, std::move(network_context_factory), qr_secret,
+ peer_identity, std::move(generate_pairing_data)),
std::move(platform));
}
-std::unique_ptr<Transaction> TransactFromFCM(
+std::unique_ptr<Transaction> TransactFromQRCodeDeprecated(
std::unique_ptr<Platform> platform,
network::mojom::NetworkContext* network_context,
base::span<const uint8_t, kRootSecretSize> root_secret,
+ const std::string& authenticator_name,
+ base::span<const uint8_t, 16> qr_secret,
+ base::span<const uint8_t, kP256X962Length> peer_identity,
+ std::optional<std::vector<uint8_t>> contact_id) {
+ NetworkContextFactory factory = base::BindRepeating(
+ [](network::mojom::NetworkContext* network_context) {
+ return network_context;
+ },
+ network_context);
+ return TransactFromQRCode(std::move(platform), std::move(factory),
+ root_secret, authenticator_name, qr_secret,
+ peer_identity, std::move(contact_id));
+}
+
+std::unique_ptr<Transaction> TransactFromFCM(
+ std::unique_ptr<Platform> platform,
+ NetworkContextFactory network_context_factory,
+ base::span<const uint8_t, kRootSecretSize> root_secret,
std::array<uint8_t, kRoutingIdSize> routing_id,
base::span<const uint8_t, kTunnelIdSize> tunnel_id,
base::span<const uint8_t, kPairingIDSize> pairing_id,
@@ -1269,12 +1288,31 @@ std::unique_ptr<Transaction> TransactFromFCM(
Platform* const platform_ptr = platform.get();
return std::make_unique<CTAP2Processor>(
- std::make_unique<TunnelTransport>(platform_ptr, network_context,
- paired_secret, client_nonce, routing_id,
- tunnel_id, IdentityKey(root_secret)),
+ std::make_unique<TunnelTransport>(
+ platform_ptr, std::move(network_context_factory), paired_secret,
+ client_nonce, routing_id, tunnel_id, IdentityKey(root_secret)),
std::move(platform));
}
+std::unique_ptr<Transaction> TransactFromFCMDeprecated(
+ std::unique_ptr<Platform> platform,
+ network::mojom::NetworkContext* network_context,
+ base::span<const uint8_t, kRootSecretSize> root_secret,
+ std::array<uint8_t, kRoutingIdSize> routing_id,
+ base::span<const uint8_t, kTunnelIdSize> tunnel_id,
+ base::span<const uint8_t, kPairingIDSize> pairing_id,
+ base::span<const uint8_t, kClientNonceSize> client_nonce,
+ std::optional<base::span<const uint8_t>> contact_id) {
+ NetworkContextFactory factory = base::BindRepeating(
+ [](network::mojom::NetworkContext* network_context) {
+ return network_context;
+ },
+ network_context);
+ return TransactFromFCM(std::move(platform), std::move(factory), root_secret,
+ std::move(routing_id), tunnel_id, pairing_id,
+ client_nonce, std::move(contact_id));
+}
+
} // namespace authenticator
} // namespace cablev2
} // namespace device
diff --git a/chromium/device/fido/cable/v2_authenticator.h b/chromium/device/fido/cable/v2_authenticator.h
index 0f55d7758ed..811e632e915 100644
--- src/3rdparty/chromium/device/fido/cable/v2_authenticator.h
+++ src/3rdparty/chromium/device/fido/cable/v2_authenticator.h
@@ -14,6 +14,7 @@
#include "base/functional/callback.h"
#include "device/fido/cable/v2_constants.h"
#include "device/fido/fido_constants.h"
+#include "device/fido/network_context_factory.h"
#include "services/network/public/mojom/network_context.mojom-forward.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
#include "third_party/abseil-cpp/absl/types/variant.h"
@@ -139,7 +140,7 @@ std::unique_ptr<Transaction> TransactWithPlaintextTransport(
// contents of a QR code.
std::unique_ptr<Transaction> TransactFromQRCode(
std::unique_ptr<Platform> platform,
- network::mojom::NetworkContext* network_context,
+ NetworkContextFactory network_context_factory,
base::span<const uint8_t, kRootSecretSize> root_secret,
const std::string& authenticator_name,
// TODO: name this constant.
@@ -147,11 +148,22 @@ std::unique_ptr<Transaction> TransactFromQRCode(
base::span<const uint8_t, kP256X962Length> peer_identity,
absl::optional<std::vector<uint8_t>> contact_id);
+// Deprecated, kept around while Android cable code is cleaned up. Use
+// TransactFromQRCode instead.
+std::unique_ptr<Transaction> TransactFromQRCodeDeprecated(
+ std::unique_ptr<Platform> platform,
+ network::mojom::NetworkContext* network_context,
+ base::span<const uint8_t, kRootSecretSize> root_secret,
+ const std::string& authenticator_name,
+ base::span<const uint8_t, 16> qr_secret,
+ base::span<const uint8_t, kP256X962Length> peer_identity,
+ std::optional<std::vector<uint8_t>> contact_id);
+
// TransactFromFCM starts a network-based transaction based on the decoded
// contents of a cloud message.
std::unique_ptr<Transaction> TransactFromFCM(
std::unique_ptr<Platform> platform,
- network::mojom::NetworkContext* network_context,
+ NetworkContextFactory network_context_factory,
base::span<const uint8_t, kRootSecretSize> root_secret,
std::array<uint8_t, kRoutingIdSize> routing_id,
base::span<const uint8_t, kTunnelIdSize> tunnel_id,
@@ -159,6 +171,18 @@ std::unique_ptr<Transaction> TransactFromFCM(
base::span<const uint8_t, kClientNonceSize> client_nonce,
absl::optional<base::span<const uint8_t>> contact_id);
+// Deprecated, kept around while Android cable code is cleaned up. Use
+// TransactFromFCM instead.
+std::unique_ptr<Transaction> TransactFromFCMDeprecated(
+ std::unique_ptr<Platform> platform,
+ network::mojom::NetworkContext* network_context,
+ base::span<const uint8_t, kRootSecretSize> root_secret,
+ std::array<uint8_t, kRoutingIdSize> routing_id,
+ base::span<const uint8_t, kTunnelIdSize> tunnel_id,
+ base::span<const uint8_t, kPairingIDSize> pairing_id,
+ base::span<const uint8_t, kClientNonceSize> client_nonce,
+ std::optional<base::span<const uint8_t>> contact_id);
+
} // namespace authenticator
} // namespace cablev2
} // namespace device
diff --git a/chromium/device/fido/cable/v2_discovery.cc b/chromium/device/fido/cable/v2_discovery.cc
index e6a8bb03557..c19ed148ee1 100644
--- src/3rdparty/chromium/device/fido/cable/v2_discovery.cc
+++ src/3rdparty/chromium/device/fido/cable/v2_discovery.cc
@@ -49,7 +49,7 @@ void RecordEvent(CableV2DiscoveryEvent event) {
Discovery::Discovery(
FidoRequestType request_type,
- network::mojom::NetworkContext* network_context,
+ NetworkContextFactory network_context_factory,
absl::optional<base::span<const uint8_t, kQRKeySize>> qr_generator_key,
std::unique_ptr<AdvertEventStream> advert_stream,
std::unique_ptr<EventStream<std::unique_ptr<Pairing>>>
@@ -62,7 +62,7 @@ Discovery::Discovery(
absl::optional<base::RepeatingCallback<void(Event)>> event_callback)
: FidoDeviceDiscovery(FidoTransportProtocol::kHybrid),
request_type_(request_type),
- network_context_(network_context),
+ network_context_factory_(std::move(network_context_factory)),
qr_keys_(KeysFromQRGeneratorKey(qr_generator_key)),
extension_keys_(KeysFromExtension(extension_contents)),
advert_stream_(std::move(advert_stream)),
@@ -161,7 +161,7 @@ void Discovery::OnBLEAdvertSeen(base::span<const uint8_t, kAdvertSize> advert) {
event_callback_->Run(Event::kBLEAdvertReceived);
}
AddDevice(std::make_unique<cablev2::FidoTunnelDevice>(
- network_context_, pairing_callback_, event_callback_,
+ network_context_factory_, pairing_callback_, event_callback_,
qr_keys_->qr_secret, qr_keys_->local_identity_seed, *plaintext));
return;
}
@@ -177,7 +177,7 @@ void Discovery::OnBLEAdvertSeen(base::span<const uint8_t, kAdvertSize> advert) {
RecordEvent(CableV2DiscoveryEvent::kExtensionMatch);
device_committed_ = true;
AddDevice(std::make_unique<cablev2::FidoTunnelDevice>(
- network_context_, base::DoNothing(), event_callback_,
+ network_context_factory_, base::DoNothing(), event_callback_,
extension.qr_secret, extension.local_identity_seed, *plaintext));
return;
}
@@ -190,7 +190,7 @@ void Discovery::OnBLEAdvertSeen(base::span<const uint8_t, kAdvertSize> advert) {
void Discovery::OnContactDevice(std::unique_ptr<Pairing> pairing) {
auto pairing_copy = std::make_unique<Pairing>(*pairing);
tunnels_pending_advert_.emplace_back(std::make_unique<FidoTunnelDevice>(
- request_type_, network_context_, std::move(pairing),
+ request_type_, network_context_factory_, std::move(pairing),
base::BindOnce(&Discovery::PairingIsInvalid, weak_factory_.GetWeakPtr(),
std::move(pairing_copy)),
event_callback_));
diff --git a/chromium/device/fido/cable/v2_discovery.h b/chromium/device/fido/cable/v2_discovery.h
index c5aa2179d7b..44fc0180c58 100644
--- src/3rdparty/chromium/device/fido/cable/v2_discovery.h
+++ src/3rdparty/chromium/device/fido/cable/v2_discovery.h
@@ -19,6 +19,7 @@
#include "device/fido/cable/v2_constants.h"
#include "device/fido/fido_constants.h"
#include "device/fido/fido_device_discovery.h"
+#include "device/fido/network_context_factory.h"
#include "services/network/public/mojom/network_context.mojom-forward.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
@@ -36,7 +37,7 @@ class COMPONENT_EXPORT(DEVICE_FIDO) Discovery : public FidoDeviceDiscovery {
Discovery(
FidoRequestType request_type,
- network::mojom::NetworkContext* network_context,
+ NetworkContextFactory network_context_factory,
absl::optional<base::span<const uint8_t, kQRKeySize>> qr_generator_key,
std::unique_ptr<AdvertEventStream> advert_stream,
// contact_device_stream contains a series of pairings indicating that the
@@ -80,7 +81,7 @@ class COMPONENT_EXPORT(DEVICE_FIDO) Discovery : public FidoDeviceDiscovery {
const std::vector<CableDiscoveryData>& extension_contents);
const FidoRequestType request_type_;
- const raw_ptr<network::mojom::NetworkContext> network_context_;
+ NetworkContextFactory network_context_factory_;
const absl::optional<UnpairedKeys> qr_keys_;
const std::vector<UnpairedKeys> extension_keys_;
std::unique_ptr<AdvertEventStream> advert_stream_;
diff --git a/chromium/device/fido/fido_discovery_factory.cc b/chromium/device/fido/fido_discovery_factory.cc
index 040aa19fbb4..2d6c85c7721 100644
--- src/3rdparty/chromium/device/fido/fido_discovery_factory.cc
+++ src/3rdparty/chromium/device/fido/fido_discovery_factory.cc
@@ -87,7 +87,7 @@ std::vector<std::unique_ptr<FidoDiscoveryBase>> FidoDiscoveryFactory::Create(
&CableDiscoveryData::version);
if (qr_generator_key_.has_value() || have_v2_discovery_data) {
ret.emplace_back(std::make_unique<cablev2::Discovery>(
- request_type_.value(), network_context_, qr_generator_key_,
+ request_type_.value(), network_context_factory_, qr_generator_key_,
v1_discovery->GetV2AdvertStream(),
std::move(contact_device_stream_),
cable_data_.value_or(std::vector<CableDiscoveryData>()),
@@ -148,11 +148,6 @@ void FidoDiscoveryFactory::set_android_accessory_params(
aoa_request_description_ = std::move(aoa_request_description);
}
-void FidoDiscoveryFactory::set_network_context(
- network::mojom::NetworkContext* network_context) {
- network_context_ = network_context;
-}
-
void FidoDiscoveryFactory::set_cable_pairing_callback(
base::RepeatingCallback<void(std::unique_ptr<cablev2::Pairing>)> callback) {
cable_pairing_callback_ = std::move(callback);
diff --git a/chromium/device/fido/fido_discovery_factory.h b/chromium/device/fido/fido_discovery_factory.h
index ed2b51d86cc..9705d548c24 100644
--- src/3rdparty/chromium/device/fido/fido_discovery_factory.h
+++ src/3rdparty/chromium/device/fido/fido_discovery_factory.h
@@ -22,6 +22,7 @@
#include "device/fido/fido_request_handler_base.h"
#include "device/fido/fido_transport_protocol.h"
#include "device/fido/hid/fido_hid_discovery.h"
+#include "device/fido/network_context_factory.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "services/device/public/mojom/usb_manager.mojom.h"
#include "services/network/public/mojom/network_context.mojom-forward.h"
@@ -69,7 +70,10 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoDiscoveryFactory {
mojo::Remote<device::mojom::UsbDeviceManager>,
std::string aoa_request_description);
- void set_network_context(network::mojom::NetworkContext*);
+ void set_network_context_factory(
+ NetworkContextFactory network_context_factory) {
+ network_context_factory_ = std::move(network_context_factory);
+ }
// set_cable_pairing_callback installs a repeating callback that will be
// called when a QR handshake results in a phone wishing to pair with this
@@ -167,7 +171,7 @@ class COMPONENT_EXPORT(DEVICE_FIDO) FidoDiscoveryFactory {
absl::optional<mojo::Remote<device::mojom::UsbDeviceManager>>
usb_device_manager_;
std::string aoa_request_description_;
- raw_ptr<network::mojom::NetworkContext> network_context_ = nullptr;
+ NetworkContextFactory network_context_factory_;
absl::optional<std::vector<CableDiscoveryData>> cable_data_;
absl::optional<std::array<uint8_t, cablev2::kQRKeySize>> qr_generator_key_;
absl::optional<FidoRequestType> request_type_;
diff --git a/chromium/device/fido/network_context_factory.h b/chromium/device/fido/network_context_factory.h
new file mode 100644
index 00000000000..802f45c4c45
--- /dev/null
+++ src/3rdparty/chromium/device/fido/network_context_factory.h
@@ -0,0 +1,20 @@
+// Copyright 2024 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef DEVICE_FIDO_NETWORK_CONTEXT_FACTORY_H_
+#define DEVICE_FIDO_NETWORK_CONTEXT_FACTORY_H_
+
+#include "base/functional/callback_forward.h"
+
+namespace network::mojom {
+class NetworkContext;
+} // namespace network::mojom
+
+namespace device {
+using NetworkContextFactory =
+ base::RepeatingCallback<network::mojom::NetworkContext*()>;
+} // namespace device
+
+#endif // DEVICE_FIDO_NETWORK_CONTEXT_FACTORY_H_
+
From 533465116ce3b87ebd30d1b4de055daa413c1050 Mon Sep 17 00:00:00 2001
From: Tommy Steimel <steimel@chromium.org>
Date: Wed, 24 Apr 2024 09:18:49 +0000
Subject: [PATCH] [Backport] CVE-2024-4331: Use after free in Picture In
Picture
Cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/5477908:
Don't assume the enter event window is a LocalDOMWindow
This CL changes DocumentPictureInPictureEvent to store a DOMWindow
instead of a LocalDOMWindow to prevent crashes when the window it gets
is actually a RemoteDOMWindow.
(cherry picked from commit 2314741cdf2c4a6e11234dda7006ec0dd9005bbb)
Bug: 335003891
Change-Id: I86a0ec5a89b51a26d5dd89559f86e6e4d6c3e8fe
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5467978
Commit-Queue: Tommy Steimel <steimel@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1290122}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5477908
Auto-Submit: Tommy Steimel <steimel@chromium.org>
Commit-Queue: Frank Liberato <liberato@chromium.org>
Cr-Commit-Position: refs/branch-heads/6367@{#974}
Cr-Branched-From: d158c6dc6e3604e6f899041972edf26087a49740-refs/heads/main@{#1274542}
(cherry picked from commit 98bcf9ef5cdd3a19f8b739f2cc6b2afdb01b7694)
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/560756
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
---
.../document_picture_in_picture_event.cc | 9 ++++-----
.../document_picture_in_picture_event.h | 11 +++++------
2 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/chromium/third_party/blink/renderer/modules/document_picture_in_picture/document_picture_in_picture_event.cc b/chromium/third_party/blink/renderer/modules/document_picture_in_picture/document_picture_in_picture_event.cc
index 037813c62c2..572d0803c25 100644
--- src/3rdparty/chromium/third_party/blink/renderer/modules/document_picture_in_picture/document_picture_in_picture_event.cc
+++ src/3rdparty/chromium/third_party/blink/renderer/modules/document_picture_in_picture/document_picture_in_picture_event.cc
@@ -8,7 +8,7 @@ namespace blink {
DocumentPictureInPictureEvent* DocumentPictureInPictureEvent::Create(
const AtomicString& type,
- LocalDOMWindow* document_picture_in_picture_window) {
+ DOMWindow* document_picture_in_picture_window) {
return MakeGarbageCollected<DocumentPictureInPictureEvent>(
type, document_picture_in_picture_window);
}
@@ -19,13 +19,13 @@ DocumentPictureInPictureEvent* DocumentPictureInPictureEvent::Create(
return MakeGarbageCollected<DocumentPictureInPictureEvent>(type, initializer);
}
-LocalDOMWindow* DocumentPictureInPictureEvent::window() const {
+DOMWindow* DocumentPictureInPictureEvent::window() const {
return document_picture_in_picture_window_.Get();
}
DocumentPictureInPictureEvent::DocumentPictureInPictureEvent(
AtomicString const& type,
- LocalDOMWindow* document_picture_in_picture_window)
+ DOMWindow* document_picture_in_picture_window)
: Event(type, Bubbles::kYes, Cancelable::kNo),
document_picture_in_picture_window_(document_picture_in_picture_window) {}
@@ -33,8 +33,7 @@ DocumentPictureInPictureEvent::DocumentPictureInPictureEvent(
AtomicString const& type,
const DocumentPictureInPictureEventInit* initializer)
: Event(type, initializer),
- document_picture_in_picture_window_(
- static_cast<LocalDOMWindow*>(initializer->window())) {}
+ document_picture_in_picture_window_(initializer->window()) {}
void DocumentPictureInPictureEvent::Trace(Visitor* visitor) const {
visitor->Trace(document_picture_in_picture_window_);
diff --git a/chromium/third_party/blink/renderer/modules/document_picture_in_picture/document_picture_in_picture_event.h b/chromium/third_party/blink/renderer/modules/document_picture_in_picture/document_picture_in_picture_event.h
index 7af20221469..59cd8cb7a2e 100644
--- src/3rdparty/chromium/third_party/blink/renderer/modules/document_picture_in_picture/document_picture_in_picture_event.h
+++ src/3rdparty/chromium/third_party/blink/renderer/modules/document_picture_in_picture/document_picture_in_picture_event.h
@@ -6,7 +6,7 @@
#define THIRD_PARTY_BLINK_RENDERER_MODULES_DOCUMENT_PICTURE_IN_PICTURE_DOCUMENT_PICTURE_IN_PICTURE_EVENT_H_
#include "third_party/blink/renderer/bindings/modules/v8/v8_document_picture_in_picture_event_init.h"
-#include "third_party/blink/renderer/core/frame/local_dom_window.h"
+#include "third_party/blink/renderer/core/frame/dom_window.h"
#include "third_party/blink/renderer/modules/event_modules.h"
#include "third_party/blink/renderer/platform/wtf/text/atomic_string.h"
@@ -18,22 +18,21 @@ class MODULES_EXPORT DocumentPictureInPictureEvent final : public Event {
DEFINE_WRAPPERTYPEINFO();
public:
- static DocumentPictureInPictureEvent* Create(const AtomicString&,
- LocalDOMWindow*);
+ static DocumentPictureInPictureEvent* Create(const AtomicString&, DOMWindow*);
static DocumentPictureInPictureEvent* Create(
const AtomicString&,
const DocumentPictureInPictureEventInit*);
- DocumentPictureInPictureEvent(AtomicString const&, LocalDOMWindow*);
+ DocumentPictureInPictureEvent(AtomicString const&, DOMWindow*);
DocumentPictureInPictureEvent(AtomicString const&,
const DocumentPictureInPictureEventInit*);
- LocalDOMWindow* window() const;
+ DOMWindow* window() const;
void Trace(Visitor*) const override;
private:
- Member<LocalDOMWindow> document_picture_in_picture_window_;
+ Member<DOMWindow> document_picture_in_picture_window_;
};
} // namespace blink
From 8f952f1e9d4fbfcfa23f8970985214f473ebffed Mon Sep 17 00:00:00 2001
From: Antonio Maiorano <amaiorano@google.com>
Date: Thu, 25 Apr 2024 16:49:11 -0400
Subject: [PATCH] [Backport] CVE-2024-4368: Use after free in Dawn
Manual cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/external/github.com/microsoft/DirectXShaderCompiler/+/5490380:
Fixed crash in loop unroll caused by bug in structurize loop exits (#6548)
Fixed a bug in `hlsl::RemoveUnstructuredLoopExits` where when a new
exiting block is created from splitting, it was added to the current
loop being processed, when it could also part of an inner loop. Not
adding the new block to inner loops that it's part of makes the inner
loops malformed, and causes crash.
This fix adds the new block to the inner most loop that it should be
part of. Also adds the `StructurizeLoopExits` option to `loop-unroll`
pass, which was missing before.
Bug: chromium:333508731
Change-Id: I7efc21bc61aeb81b4906a600c35272af232710ea
Reviewed-on: https://chromium-review.googlesource.com/c/external/github.com/microsoft/DirectXShaderCompiler/+/5490380
Reviewed-by: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@chromium.org>
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/560757
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
---
.../Scalar/DxilRemoveUnstructuredLoopExits.cpp | 7 ++++++-
.../dxc/lib/Transforms/Scalar/LoopUnrollPass.cpp | 12 ++++++++++++
.../dawn/third_party/dxc/utils/hct/hctdb.py | 3 ++-
3 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/chromium/third_party/dawn/third_party/dxc/lib/Transforms/Scalar/DxilRemoveUnstructuredLoopExits.cpp b/chromium/third_party/dawn/third_party/dxc/lib/Transforms/Scalar/DxilRemoveUnstructuredLoopExits.cpp
index 1278b1cac0a..a68a4dd7c56 100644
--- src/3rdparty/chromium/third_party/dawn/third_party/dxc/lib/Transforms/Scalar/DxilRemoveUnstructuredLoopExits.cpp
+++ src/3rdparty/chromium/third_party/dawn/third_party/dxc/lib/Transforms/Scalar/DxilRemoveUnstructuredLoopExits.cpp
@@ -430,7 +430,12 @@ static bool RemoveUnstructuredLoopExitsIteration(BasicBlock *exiting_block, Loop
BasicBlock *new_not_exiting_block = new_exiting_block->splitBasicBlock(new_exiting_block->getFirstNonPHI());
new_exiting_block->setName("dx.struct_exit.new_exiting");
new_not_exiting_block->setName(old_name);
- L->addBasicBlockToLoop(new_not_exiting_block, *LI);
+ // Query for new_exiting_block's own loop to add new_not_exiting_block to.
+ // It's possible that new_exiting_block is part of another inner loop
+ // separate from L. If added directly to L, the inner loop(s) will not
+ // contain new_not_exiting_block, making them malformed.
+ Loop *inner_loop_of_exiting_block = LI->getLoopFor(new_exiting_block);
+ inner_loop_of_exiting_block->addBasicBlockToLoop(new_not_exiting_block, *LI);
// Branch to latch_exit
new_exiting_block->getTerminator()->eraseFromParent();
diff --git a/chromium/third_party/dawn/third_party/dxc/lib/Transforms/Scalar/LoopUnrollPass.cpp b/chromium/third_party/dawn/third_party/dxc/lib/Transforms/Scalar/LoopUnrollPass.cpp
index dd520f7e57d..b17a5a4a0bc 100644
--- src/3rdparty/chromium/third_party/dawn/third_party/dxc/lib/Transforms/Scalar/LoopUnrollPass.cpp
+++ src/3rdparty/chromium/third_party/dawn/third_party/dxc/lib/Transforms/Scalar/LoopUnrollPass.cpp
@@ -155,6 +155,18 @@ namespace {
bool UserAllowPartial;
bool UserRuntime;
+ // HLSL Change - begin
+ // Function overrides that resolve options when used for DxOpt
+ void applyOptions(PassOptions O) override {
+ GetPassOptionBool(O, "StructurizeLoopExits", &StructurizeLoopExits,
+ false);
+ }
+ void dumpConfig(raw_ostream &OS) override {
+ LoopPass::dumpConfig(OS);
+ OS << ",StructurizeLoopExits=" << StructurizeLoopExits;
+ }
+ // HLSL Change - end
+
bool runOnLoop(Loop *L, LPPassManager &LPM) override;
/// This transformation requires natural loop information & requires that
diff --git a/chromium/third_party/dawn/third_party/dxc/utils/hct/hctdb.py b/chromium/third_party/dawn/third_party/dxc/utils/hct/hctdb.py
index dcb1923fafe..96da518d1ac 100644
--- src/3rdparty/chromium/third_party/dawn/third_party/dxc/utils/hct/hctdb.py
+++ src/3rdparty/chromium/third_party/dawn/third_party/dxc/utils/hct/hctdb.py
@@ -2270,7 +2270,8 @@ def add_pass(name, type_name, doc, opts):
{'n':'unroll-count', 'i':'UnrollCount', 't':'unsigned', 'd':'Use this unroll count for all loops including those with unroll_count pragma values, for testing purposes'},
{'n':'unroll-allow-partial', 'i':'UnrollAllowPartial', 't':'bool', 'd':'Allows loops to be partially unrolled until -unroll-threshold loop size is reached.'},
{'n':'unroll-runtime', 'i':'UnrollRuntime', 't':'bool', 'd':'Unroll loops with run-time trip counts'},
- {'n':'pragma-unroll-threshold', 'i':'PragmaUnrollThreshold', 't':'unsigned', 'd':'Unrolled size limit for loops with an unroll(full) or unroll_count pragma.'}])
+ {'n':'pragma-unroll-threshold', 'i':'PragmaUnrollThreshold', 't':'unsigned', 'd':'Unrolled size limit for loops with an unroll(full) or unroll_count pragma.'},
+ {'n': 'StructurizeLoopExits', 't': 'bool', 'c': 1, 'd': 'Whether the unroller should try to structurize loop exits first.'}])
add_pass('mldst-motion', 'MergedLoadStoreMotion', 'MergedLoadStoreMotion', [])
add_pass('gvn', 'GVN', 'Global Value Numbering', [
{'n':'noloads', 't':'bool', 'c':1},
From 453dc7630d1e8457a97a7cbc70b8990afd5bf5e4 Mon Sep 17 00:00:00 2001
From: Ken Rockot <rockot@google.com>
Date: Wed, 8 May 2024 15:32:48 +0000
Subject: [PATCH] [Backport] CVE-2024-4671: Use after free in Visuals
Cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/5523748:
Viz: Tolerate SinkGroup destruction during submit
Fixed: 339266700
Change-Id: I8c0ea8c540948016346b00db64fe33260d2446f0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5523748
Reviewed-by: Kyle Charbonneau <kylechar@chromium.org>
Reviewed-by: Jonathan Ross <jonross@chromium.org>
Commit-Queue: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/main@{#1298119}
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/560758
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
---
.../frame_sinks/frame_sink_bundle_impl.cc | 30 ++++++++++++++-----
1 file changed, 22 insertions(+), 8 deletions(-)
diff --git a/chromium/components/viz/service/frame_sinks/frame_sink_bundle_impl.cc b/chromium/components/viz/service/frame_sinks/frame_sink_bundle_impl.cc
index e339c23f9cd..2ae2dfe1a8b 100644
--- src/3rdparty/chromium/components/viz/service/frame_sinks/frame_sink_bundle_impl.cc
+++ src/3rdparty/chromium/components/viz/service/frame_sinks/frame_sink_bundle_impl.cc
@@ -4,12 +4,15 @@
#include "components/viz/service/frame_sinks/frame_sink_bundle_impl.h"
+#include <map>
#include <utility>
#include <vector>
#include "base/check.h"
#include "base/functional/bind.h"
+#include "base/memory/raw_ptr.h"
#include "base/memory/raw_ref.h"
+#include "base/memory/weak_ptr.h"
#include "build/build_config.h"
#include "components/viz/service/frame_sinks/compositor_frame_sink_impl.h"
#include "components/viz/service/frame_sinks/frame_sink_manager_impl.h"
@@ -45,6 +48,10 @@ class FrameSinkBundleImpl::SinkGroup : public BeginFrameObserver {
bool IsEmpty() const { return frame_sinks_.empty(); }
+ base::WeakPtr<SinkGroup> GetWeakPtr() {
+ return weak_ptr_factory_.GetWeakPtr();
+ }
+
void AddFrameSink(uint32_t sink_id) {
frame_sinks_.insert(sink_id);
@@ -206,6 +213,8 @@ class FrameSinkBundleImpl::SinkGroup : public BeginFrameObserver {
std::set<uint32_t> unacked_submissions_;
BeginFrameArgs last_used_begin_frame_args_;
+
+ base::WeakPtrFactory<SinkGroup> weak_ptr_factory_{this};
};
FrameSinkBundleImpl::FrameSinkBundleImpl(
@@ -276,8 +285,9 @@ void FrameSinkBundleImpl::SetNeedsBeginFrame(uint32_t sink_id,
void FrameSinkBundleImpl::Submit(
std::vector<mojom::BundledFrameSubmissionPtr> submissions) {
- std::set<SinkGroup*> groups;
- std::set<SinkGroup*> affected_groups;
+ std::map<raw_ptr<SinkGroup>, base::WeakPtr<SinkGroup>> groups;
+ std::map<raw_ptr<SinkGroup>, base::WeakPtr<SinkGroup>> affected_groups;
+
// Count the frame submissions before processing anything. This ensures that
// any frames submitted here will be acked together in a batch, and not acked
// individually in case they happen to ack synchronously within
@@ -288,10 +298,10 @@ void FrameSinkBundleImpl::Submit(
// through to the client without batching.
for (auto& submission : submissions) {
if (auto* group = GetSinkGroup(submission->sink_id)) {
- groups.insert(group);
+ groups.emplace(group, group->GetWeakPtr());
if (submission->data->is_frame()) {
group->WillSubmitFrame(submission->sink_id);
- affected_groups.insert(group);
+ affected_groups.emplace(group, group->GetWeakPtr());
}
}
}
@@ -321,12 +331,16 @@ void FrameSinkBundleImpl::Submit(
}
}
- for (auto* group : groups) {
- group->DidFinishFrame();
+ for (const auto& [unsafe_group, weak_group] : groups) {
+ if (weak_group) {
+ weak_group->DidFinishFrame();
+ }
}
- for (auto* group : affected_groups) {
- group->FlushMessages();
+ for (const auto& [unsafe_group, weak_group] : affected_groups) {
+ if (weak_group) {
+ weak_group->FlushMessages();
+ }
}
}
From 09a04a46a47e57bc423cc37fb95bc66041936940 Mon Sep 17 00:00:00 2001
From: Shu-yu Guo <syg@chromium.org>
Date: Thu, 9 May 2024 12:03:28 -0700
Subject: [PATCH] [Backport] Security bug 339458194
Cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/v8/v8/+/5527397
Only normalize JSObject targets in SetOrCopyDataProperties
Bug: b/339458194
Change-Id: I4d6eebdd921971fa28d7c474535d978900ba633f
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5527397
Commit-Queue: Shu-yu Guo <syg@chromium.org>
Cr-Commit-Position: refs/heads/main@{#93811}
(cherry picked from commit f320600cd1f48ba6bb57c0395823fe0c5e5ec52e)
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/560760
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
---
chromium/v8/src/objects/js-objects.cc | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/chromium/v8/src/objects/js-objects.cc b/chromium/v8/src/objects/js-objects.cc
index 6094323ee09..9d104492603 100644
--- src/3rdparty/chromium/v8/src/objects/js-objects.cc
+++ src/3rdparty/chromium/v8/src/objects/js-objects.cc
@@ -429,9 +429,7 @@ Maybe<bool> JSReceiver::SetOrCopyDataProperties(
Nothing<bool>());
if (!from->HasFastProperties() && target->HasFastProperties() &&
- !IsJSGlobalProxy(*target)) {
- // JSProxy is always in slow-mode.
- DCHECK(!IsJSProxy(*target));
+ IsJSObject(*target) && !IsJSGlobalProxy(*target)) {
// Convert to slow properties if we're guaranteed to overflow the number of
// descriptors.
int source_length;
Reference in New Issue View Git Blame Copy Permalink