extra/chromium: fixes

This commit is contained in:
Kevin Mihelich 2018-03-16 00:03:25 +00:00
parent 6fb5836ea9
commit 012458fcd3
13 changed files with 1714 additions and 98 deletions

View file

@ -1,14 +1,14 @@
From 99b6cda245256c0ccb5c50e4bf7a4c6256ec5c2c Mon Sep 17 00:00:00 2001
From f058ce64e97da474d30dd91064bf3ec42e739b81 Mon Sep 17 00:00:00 2001
From: Kevin Mihelich <kevin@archlinuxarm.org>
Date: Sun, 23 Oct 2016 10:57:25 -0600
Subject: [PATCH] ARM toolchain fixes
Subject: [PATCH 01/10] ARM toolchain fixes
---
build/toolchain/linux/BUILD.gn | 24 ++++++++++--------------
1 file changed, 10 insertions(+), 14 deletions(-)
diff --git a/build/toolchain/linux/BUILD.gn b/build/toolchain/linux/BUILD.gn
index 86cd7dabc6e8..4b1e73b1aca3 100644
index 4e9ff440d18c..7f528b08c98a 100644
--- a/build/toolchain/linux/BUILD.gn
+++ b/build/toolchain/linux/BUILD.gn
@@ -22,15 +22,13 @@ clang_toolchain("clang_arm64") {
@ -54,5 +54,5 @@ index 86cd7dabc6e8..4b1e73b1aca3 100644
toolchain_args = {
current_cpu = "arm"
--
2.12.0
2.16.2

View file

@ -0,0 +1,76 @@
From 8678e8d54637f291cf10e68c472d6255c3582682 Mon Sep 17 00:00:00 2001
From: Jose Dapena Paz <jose.dapena@lge.com>
Date: Wed, 7 Mar 2018 18:50:50 +0000
Subject: [PATCH 02/10] GCC build fix: mark is_trivially_copy_constructible for
WTF::Vector as false.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Compilation in GCC fails because of https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80654
if T in WTF::Optional<WTF::Vector<T>> is not trivially copy constructible.
The problem already happened in std::vector and was workarounded. This
change implements a similar fix for WTF::Vector.
Bug: 816952
Change-Id: If87f01beb952e03eb49dcaf0c5db6efd745bf05e
Reviewed-on: https://chromium-review.googlesource.com/944404
Commit-Queue: José Dapena Paz <jose.dapena@lge.com>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541516}
---
third_party/WebKit/Source/platform/wtf/DEPS | 1 +
third_party/WebKit/Source/platform/wtf/Vector.h | 17 +++++++++++++++++
2 files changed, 18 insertions(+)
diff --git a/third_party/WebKit/Source/platform/wtf/DEPS b/third_party/WebKit/Source/platform/wtf/DEPS
index 7e80997a5fc9..c780a4de7aba 100644
--- a/third_party/WebKit/Source/platform/wtf/DEPS
+++ b/third_party/WebKit/Source/platform/wtf/DEPS
@@ -16,6 +16,7 @@ include_rules = [
"+base/process/process_metrics.h",
"+base/rand_util.h",
"+base/strings",
+ "+base/template_util.h",
"+base/threading/thread_checker.h",
"+base/time/time.h",
"+base/tuple.h",
diff --git a/third_party/WebKit/Source/platform/wtf/Vector.h b/third_party/WebKit/Source/platform/wtf/Vector.h
index 8b451bddecef..8955c81712c1 100644
--- a/third_party/WebKit/Source/platform/wtf/Vector.h
+++ b/third_party/WebKit/Source/platform/wtf/Vector.h
@@ -28,6 +28,7 @@
#include <utility>
#include "base/macros.h"
+#include "base/template_util.h"
#include "build/build_config.h"
#include "platform/wtf/Alignment.h"
#include "platform/wtf/ConditionalDestructor.h"
@@ -1992,6 +1993,22 @@ Vector<T, inlineCapacity, Allocator>::Trace(VisitorDispatcher visitor) {
} // namespace WTF
+namespace base {
+
+#if defined(__GNUC__) && !defined(__clang__) && __GNUC__ <= 7
+// Workaround for g++7 and earlier family.
+// Due to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80654, without this
+// Optional<WTF::Vector<T>> where T is non-copyable causes a compile error.
+// As we know it is not trivially copy constructible, explicitly declare so.
+//
+// It completes the declaration in base/template_util.h that was provided
+// for std::vector
+template <typename T>
+struct is_trivially_copy_constructible<WTF::Vector<T>> : std::false_type {};
+#endif
+
+} // namespace base
+
using WTF::Vector;
#endif // WTF_Vector_h
--
2.16.2

View file

@ -0,0 +1,103 @@
From e5fc0341fcb0e1514daa2d347d832d1525142b33 Mon Sep 17 00:00:00 2001
From: Hidehiko Abe <hidehiko@chromium.org>
Date: Fri, 23 Feb 2018 09:50:41 +0000
Subject: [PATCH 03/10] Workaround for g++7 is_trivially_copy_constructible
failure.
cf) https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80654
Please see also crbug.com/784732#27 for details.
BUG=784732
TEST=Some with GCC.
Change-Id: I0a6d28d9c26ac9ed026d137e17fddbe86586f1e1
Reviewed-on: https://chromium-review.googlesource.com/927942
Commit-Queue: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538740}
---
base/optional.h | 3 ++-
base/template_util.h | 18 ++++++++++++++++++
base/template_util_unittest.cc | 9 +++++++++
3 files changed, 29 insertions(+), 1 deletion(-)
diff --git a/base/optional.h b/base/optional.h
index f6619a575815..c763acf824ee 100644
--- a/base/optional.h
+++ b/base/optional.h
@@ -9,6 +9,7 @@
#include <utility>
#include "base/logging.h"
+#include "base/template_util.h"
namespace base {
@@ -106,7 +107,7 @@ struct OptionalStorageBase<T, true /* trivially destructible */> {
// compiler generated constexpr {copy,move} constructors). Note that
// placement-new is prohibited in constexpr.
template <typename T,
- bool = std::is_trivially_copy_constructible<T>::value,
+ bool = is_trivially_copy_constructible<T>::value,
bool = std::is_trivially_move_constructible<T>::value>
struct OptionalStorage : OptionalStorageBase<T> {
// This is no trivially {copy,move} constructible case. Other cases are
diff --git a/base/template_util.h b/base/template_util.h
index f76003d8237b..8544aa294597 100644
--- a/base/template_util.h
+++ b/base/template_util.h
@@ -10,6 +10,7 @@
#include <iterator>
#include <type_traits>
#include <utility>
+#include <vector>
#include "build/build_config.h"
@@ -127,6 +128,23 @@ template <class T>
using is_trivially_copyable = std::is_trivially_copyable<T>;
#endif
+#if defined(__GNUC__) && !defined(__clang__) && __GNUC__ <= 7
+// Workaround for g++7 and earlier family.
+// Due to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80654, without this
+// Optional<std::vector<T>> where T is non-copyable causes a compile error.
+// As we know it is not trivially copy constructible, explicitly declare so.
+template <typename T>
+struct is_trivially_copy_constructible
+ : std::is_trivially_copy_constructible<T> {};
+
+template <typename... T>
+struct is_trivially_copy_constructible<std::vector<T...>> : std::false_type {};
+#else
+// Otherwise use std::is_trivially_copy_constructible as is.
+template <typename T>
+using is_trivially_copy_constructible = std::is_trivially_copy_constructible<T>;
+#endif
+
} // namespace base
#undef CR_USE_FALLBACKS_FOR_GCC_WITH_LIBCXX
diff --git a/base/template_util_unittest.cc b/base/template_util_unittest.cc
index 12e5d362dd25..2c42445f785d 100644
--- a/base/template_util_unittest.cc
+++ b/base/template_util_unittest.cc
@@ -92,6 +92,15 @@ static_assert(!base::is_trivially_copyable<TrivialCopyButWithDestructor>::value,
"TrivialCopyButWithDestructor should not be detected as "
"trivially copyable");
+class NoCopy {
+ public:
+ NoCopy(const NoCopy&) = delete;
+};
+
+static_assert(
+ !base::is_trivially_copy_constructible<std::vector<NoCopy>>::value,
+ "is_trivially_copy_constructible<std::vector<T>> must be compiled.");
+
} // namespace
} // namespace base
--
2.16.2

View file

@ -0,0 +1,41 @@
From 1e9b3f1552dc88531eaaf7c785807a1941ca1269 Mon Sep 17 00:00:00 2001
From: Jose Dapena Paz <jose.dapena@lge.com>
Date: Mon, 5 Mar 2018 14:19:54 +0000
Subject: [PATCH 04/10] GCC build fix: base::Optional<T> requires the full
declaration of T
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
In GCC 7.2/7.3, base::Optional<T> fails to compile, if T has still
only been forward declared, as it cannot properly resolve the
is_trivially_* declarations. In this case it is needed to include the
full declaration of the type, and not only the forward declaration.
Change-Id: I63e5c6307394c6c2eda6af108c80395152bfc04f
Reviewed-on: https://chromium-review.googlesource.com/944401
Commit-Queue: José Dapena Paz <jose.dapena@lge.com>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540815}
---
services/preferences/tracked/pref_hash_filter.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/services/preferences/tracked/pref_hash_filter.h b/services/preferences/tracked/pref_hash_filter.h
index bdc9b621c9b7..526c53f614db 100644
--- a/services/preferences/tracked/pref_hash_filter.h
+++ b/services/preferences/tracked/pref_hash_filter.h
@@ -21,9 +21,9 @@
#include "services/preferences/public/interfaces/preferences.mojom.h"
#include "services/preferences/tracked/hash_store_contents.h"
#include "services/preferences/tracked/interceptable_pref_filter.h"
+#include "services/preferences/tracked/pref_hash_store.h"
#include "services/preferences/tracked/tracked_preference.h"
-class PrefHashStore;
class PrefService;
namespace base {
--
2.16.2

View file

@ -0,0 +1,163 @@
From 8bd2b87e3a914cbf266fe22cf861274cc2996220 Mon Sep 17 00:00:00 2001
From: Jose Dapena Paz <jose.dapena@lge.com>
Date: Mon, 5 Mar 2018 23:43:08 +0000
Subject: [PATCH 05/10] GCC: IDB methods String() renamed to GetString()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
GCC 7.2/7.3 complains for redefining String as a method, in IDB code,
because the string accessor is named String(). So rename the accessor
to be GetString.
It seems basic.scope.hiding should imply the original code is valid,
so reported as a bug to GCC:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84709
Change-Id: Ide66104bd0953f5bb03a1bc62d7f238833ea62e7
Reviewed-on: https://chromium-review.googlesource.com/944406
Commit-Queue: José Dapena Paz <jose.dapena@lge.com>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540986}
---
.../WebKit/Source/bindings/modules/v8/V8BindingForModules.cpp | 10 +++++-----
.../Source/bindings/modules/v8/V8BindingForModulesTest.cpp | 2 +-
third_party/WebKit/Source/modules/exported/WebIDBKey.cpp | 2 +-
third_party/WebKit/Source/modules/indexeddb/IDBDatabase.cpp | 2 +-
third_party/WebKit/Source/modules/indexeddb/IDBKey.h | 2 +-
third_party/WebKit/Source/modules/indexeddb/IDBKeyPath.h | 2 +-
.../Source/modules/indexeddb/InspectorIndexedDBAgent.cpp | 2 +-
7 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/third_party/WebKit/Source/bindings/modules/v8/V8BindingForModules.cpp b/third_party/WebKit/Source/bindings/modules/v8/V8BindingForModules.cpp
index 6b3d0e73a3b4..4d2a5befcad9 100644
--- a/third_party/WebKit/Source/bindings/modules/v8/V8BindingForModules.cpp
+++ b/third_party/WebKit/Source/bindings/modules/v8/V8BindingForModules.cpp
@@ -68,7 +68,7 @@ v8::Local<v8::Value> ToV8(const IDBKeyPath& value,
case IDBKeyPath::kNullType:
return v8::Null(isolate);
case IDBKeyPath::kStringType:
- return V8String(isolate, value.String());
+ return V8String(isolate, value.GetString());
case IDBKeyPath::kArrayType:
return ToV8(value.Array(), creation_context, isolate);
}
@@ -97,7 +97,7 @@ v8::Local<v8::Value> ToV8(const IDBKey* key,
case IDBKey::kNumberType:
return v8::Number::New(isolate, key->Number());
case IDBKey::kStringType:
- return V8String(isolate, key->String());
+ return V8String(isolate, key->GetString());
case IDBKey::kBinaryType:
// https://w3c.github.io/IndexedDB/#convert-a-value-to-a-key
return ToV8(DOMArrayBuffer::Create(key->Binary()), creation_context,
@@ -379,7 +379,7 @@ static std::unique_ptr<IDBKey> CreateIDBKeyFromValueAndKeyPath(
}
DCHECK_EQ(key_path.GetType(), IDBKeyPath::kStringType);
- return CreateIDBKeyFromValueAndKeyPath(isolate, value, key_path.String(),
+ return CreateIDBKeyFromValueAndKeyPath(isolate, value, key_path.GetString(),
exception_state);
}
@@ -483,7 +483,7 @@ bool InjectV8KeyIntoV8Value(v8::Isolate* isolate,
DCHECK(isolate->InContext());
DCHECK_EQ(key_path.GetType(), IDBKeyPath::kStringType);
- Vector<String> key_path_elements = ParseKeyPath(key_path.String());
+ Vector<String> key_path_elements = ParseKeyPath(key_path.GetString());
// The conbination of a key generator and an empty key path is forbidden by
// spec.
@@ -569,7 +569,7 @@ bool CanInjectIDBKeyIntoScriptValue(v8::Isolate* isolate,
const IDBKeyPath& key_path) {
IDB_TRACE("canInjectIDBKeyIntoScriptValue");
DCHECK_EQ(key_path.GetType(), IDBKeyPath::kStringType);
- Vector<String> key_path_elements = ParseKeyPath(key_path.String());
+ Vector<String> key_path_elements = ParseKeyPath(key_path.GetString());
if (!key_path_elements.size())
return false;
diff --git a/third_party/WebKit/Source/bindings/modules/v8/V8BindingForModulesTest.cpp b/third_party/WebKit/Source/bindings/modules/v8/V8BindingForModulesTest.cpp
index 8f79cd7d1560..9add0d0e971d 100644
--- a/third_party/WebKit/Source/bindings/modules/v8/V8BindingForModulesTest.cpp
+++ b/third_party/WebKit/Source/bindings/modules/v8/V8BindingForModulesTest.cpp
@@ -118,7 +118,7 @@ void CheckKeyPathStringValue(v8::Isolate* isolate,
CheckKeyFromValueAndKeyPathInternal(isolate, value, key_path);
ASSERT_TRUE(idb_key);
ASSERT_EQ(IDBKey::kStringType, idb_key->GetType());
- ASSERT_TRUE(expected == idb_key->String());
+ ASSERT_TRUE(expected == idb_key->GetString());
}
void CheckKeyPathNumberValue(v8::Isolate* isolate,
diff --git a/third_party/WebKit/Source/modules/exported/WebIDBKey.cpp b/third_party/WebKit/Source/modules/exported/WebIDBKey.cpp
index 86015cdab573..4f6657af8fc9 100644
--- a/third_party/WebKit/Source/modules/exported/WebIDBKey.cpp
+++ b/third_party/WebKit/Source/modules/exported/WebIDBKey.cpp
@@ -56,7 +56,7 @@ WebData WebIDBKeyView::Binary() const {
}
WebString WebIDBKeyView::String() const {
- return private_->String();
+ return private_->GetString();
}
double WebIDBKeyView::Date() const {
diff --git a/third_party/WebKit/Source/modules/indexeddb/IDBDatabase.cpp b/third_party/WebKit/Source/modules/indexeddb/IDBDatabase.cpp
index d79d8e215151..9a5e23fdc8ed 100644
--- a/third_party/WebKit/Source/modules/indexeddb/IDBDatabase.cpp
+++ b/third_party/WebKit/Source/modules/indexeddb/IDBDatabase.cpp
@@ -297,7 +297,7 @@ IDBObjectStore* IDBDatabase::createObjectStore(
}
if (auto_increment && ((key_path.GetType() == IDBKeyPath::kStringType &&
- key_path.String().IsEmpty()) ||
+ key_path.GetString().IsEmpty()) ||
key_path.GetType() == IDBKeyPath::kArrayType)) {
exception_state.ThrowDOMException(
kInvalidAccessError,
diff --git a/third_party/WebKit/Source/modules/indexeddb/IDBKey.h b/third_party/WebKit/Source/modules/indexeddb/IDBKey.h
index d5f062867441..19d5c0222b64 100644
--- a/third_party/WebKit/Source/modules/indexeddb/IDBKey.h
+++ b/third_party/WebKit/Source/modules/indexeddb/IDBKey.h
@@ -106,7 +106,7 @@ class MODULES_EXPORT IDBKey {
return binary_;
}
- const String& String() const {
+ const String& GetString() const {
DCHECK_EQ(type_, kStringType);
return string_;
}
diff --git a/third_party/WebKit/Source/modules/indexeddb/IDBKeyPath.h b/third_party/WebKit/Source/modules/indexeddb/IDBKeyPath.h
index 953c58a40a67..9f4a0532e211 100644
--- a/third_party/WebKit/Source/modules/indexeddb/IDBKeyPath.h
+++ b/third_party/WebKit/Source/modules/indexeddb/IDBKeyPath.h
@@ -65,7 +65,7 @@ class MODULES_EXPORT IDBKeyPath {
return array_;
}
- const String& String() const {
+ const String& GetString() const {
DCHECK_EQ(type_, kStringType);
return string_;
}
diff --git a/third_party/WebKit/Source/modules/indexeddb/InspectorIndexedDBAgent.cpp b/third_party/WebKit/Source/modules/indexeddb/InspectorIndexedDBAgent.cpp
index 47d3a98bf19c..06080a30718d 100644
--- a/third_party/WebKit/Source/modules/indexeddb/InspectorIndexedDBAgent.cpp
+++ b/third_party/WebKit/Source/modules/indexeddb/InspectorIndexedDBAgent.cpp
@@ -399,7 +399,7 @@ static std::unique_ptr<KeyPath> KeyPathFromIDBKeyPath(
case IDBKeyPath::kStringType:
key_path = KeyPath::create()
.setType(KeyPath::TypeEnum::String)
- .setString(idb_key_path.String())
+ .setString(idb_key_path.GetString())
.build();
break;
case IDBKeyPath::kArrayType: {
--
2.16.2

View file

@ -0,0 +1,122 @@
From 99675941be53b86ed0a3a1027afe9072869cce48 Mon Sep 17 00:00:00 2001
From: Jose Dapena Paz <jose.dapena@lge.com>
Date: Thu, 8 Mar 2018 17:46:02 +0000
Subject: [PATCH 06/10] GCC: PlaybackImageProvider::Settings: explicitely set
copy constructor.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
GCC fails to resolve the default copy constructor of the flat_map, so
we add an explicit reference to use default copy constructor.
Bug: 819294
Change-Id: Ie2d69bdbe60742e9253251c965cbf0a936037871
Reviewed-on: https://chromium-review.googlesource.com/944403
Reviewed-by: David Reveman <reveman@chromium.org>
Commit-Queue: José Dapena Paz <jose.dapena@lge.com>
Cr-Commit-Position: refs/heads/master@{#541827}
---
cc/raster/playback_image_provider.cc | 7 +++++--
cc/raster/playback_image_provider.h | 6 ++++--
cc/raster/playback_image_provider_unittest.cc | 12 ++++++++----
3 files changed, 17 insertions(+), 8 deletions(-)
diff --git a/cc/raster/playback_image_provider.cc b/cc/raster/playback_image_provider.cc
index f2a6682ae183..dbf8530b50e9 100644
--- a/cc/raster/playback_image_provider.cc
+++ b/cc/raster/playback_image_provider.cc
@@ -20,7 +20,7 @@ void UnrefImageFromCache(DrawImage draw_image,
PlaybackImageProvider::PlaybackImageProvider(
ImageDecodeCache* cache,
const gfx::ColorSpace& target_color_space,
- base::Optional<Settings> settings)
+ base::Optional<Settings>&& settings)
: cache_(cache),
target_color_space_(target_color_space),
settings_(std::move(settings)) {
@@ -92,7 +92,10 @@ PlaybackImageProvider::GetDecodedDrawImage(const DrawImage& draw_image) {
}
PlaybackImageProvider::Settings::Settings() = default;
-PlaybackImageProvider::Settings::Settings(const Settings& other) = default;
+PlaybackImageProvider::Settings::Settings(PlaybackImageProvider::Settings&&) =
+ default;
PlaybackImageProvider::Settings::~Settings() = default;
+PlaybackImageProvider::Settings& PlaybackImageProvider::Settings::operator=(
+ PlaybackImageProvider::Settings&&) = default;
} // namespace cc
diff --git a/cc/raster/playback_image_provider.h b/cc/raster/playback_image_provider.h
index bae8c25bfd3e..8c1038e7ff45 100644
--- a/cc/raster/playback_image_provider.h
+++ b/cc/raster/playback_image_provider.h
@@ -20,8 +20,10 @@ class CC_EXPORT PlaybackImageProvider : public ImageProvider {
public:
struct CC_EXPORT Settings {
Settings();
- Settings(const Settings& other);
+ Settings(const Settings&) = delete;
+ Settings(Settings&&);
~Settings();
+ Settings& operator=(Settings&&);
// The set of image ids to skip during raster.
PaintImageIdFlatSet images_to_skip;
@@ -39,7 +41,7 @@ class CC_EXPORT PlaybackImageProvider : public ImageProvider {
// If no settings are provided, all images are skipped during rasterization.
PlaybackImageProvider(ImageDecodeCache* cache,
const gfx::ColorSpace& target_color_space,
- base::Optional<Settings> settings);
+ base::Optional<Settings>&& settings);
~PlaybackImageProvider() override;
void BeginRaster() override;
diff --git a/cc/raster/playback_image_provider_unittest.cc b/cc/raster/playback_image_provider_unittest.cc
index dde64a7b5b50..96a0fdc54a03 100644
--- a/cc/raster/playback_image_provider_unittest.cc
+++ b/cc/raster/playback_image_provider_unittest.cc
@@ -84,7 +84,8 @@ TEST(PlaybackImageProviderTest, SkipsSomeImages) {
settings.emplace();
settings->images_to_skip = {skip_image.stable_id()};
- PlaybackImageProvider provider(&cache, gfx::ColorSpace(), settings);
+ PlaybackImageProvider provider(&cache, gfx::ColorSpace(),
+ std::move(settings));
provider.BeginRaster();
SkIRect rect = SkIRect::MakeWH(10, 10);
@@ -100,7 +101,8 @@ TEST(PlaybackImageProviderTest, RefAndUnrefDecode) {
base::Optional<PlaybackImageProvider::Settings> settings;
settings.emplace();
- PlaybackImageProvider provider(&cache, gfx::ColorSpace(), settings);
+ PlaybackImageProvider provider(&cache, gfx::ColorSpace(),
+ std::move(settings));
provider.BeginRaster();
{
@@ -133,7 +135,8 @@ TEST(PlaybackImageProviderTest, AtRasterImages) {
settings.emplace();
settings->at_raster_images = {draw_image1, draw_image2};
- PlaybackImageProvider provider(&cache, gfx::ColorSpace(), settings);
+ PlaybackImageProvider provider(&cache, gfx::ColorSpace(),
+ std::move(settings));
EXPECT_EQ(cache.refed_image_count(), 0);
provider.BeginRaster();
@@ -158,7 +161,8 @@ TEST(PlaybackImageProviderTest, SwapsGivenFrames) {
settings.emplace();
settings->image_to_current_frame_index = image_to_frame;
- PlaybackImageProvider provider(&cache, gfx::ColorSpace(), settings);
+ PlaybackImageProvider provider(&cache, gfx::ColorSpace(),
+ std::move(settings));
provider.BeginRaster();
SkIRect rect = SkIRect::MakeWH(10, 10);
--
2.16.2

View file

@ -0,0 +1,183 @@
From 8fa647baa1e466bfbb5a8295bd6fc4a9fb5eb43c Mon Sep 17 00:00:00 2001
From: Hidehiko Abe <hidehiko@chromium.org>
Date: Fri, 19 Jan 2018 23:50:24 +0000
Subject: [PATCH 07/10] Fix non-copyable class's optional move.
BUG=784732
TEST=Ran base_unittests -gtest_filter=*Optional*
Change-Id: Ibb5d7cc5d62deacdba7f811f5a7b83c1c58c3907
Reviewed-on: https://chromium-review.googlesource.com/855976
Reviewed-by: danakj <danakj@chromium.org>
Commit-Queue: Hidehiko Abe <hidehiko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530663}
---
base/optional.h | 24 +++++++++++++--
base/optional_unittest.cc | 74 +++++++++++++++++++++++++++++++++++++++++++----
2 files changed, 89 insertions(+), 9 deletions(-)
diff --git a/base/optional.h b/base/optional.h
index c763acf824ee..5a50eb455be6 100644
--- a/base/optional.h
+++ b/base/optional.h
@@ -45,6 +45,15 @@ struct OptionalStorageBase {
// When T is not trivially destructible we must call its
// destructor before deallocating its memory.
+ // Note that this hides the (implicitly declared) move constructor, which
+ // would be used for constexpr move constructor in OptionalStorage<T>.
+ // It is needed iff T is trivially move constructible. However, the current
+ // is_trivially_{copy,move}_constructible implementation requires
+ // is_trivially_destructible (which looks a bug, cf:
+ // https://gcc.gnu.org/bugzilla/show_bug.cgi?id=51452 and
+ // http://cplusplus.github.io/LWG/lwg-active.html#2116), so it is not
+ // necessary for this case at the moment. Please see also the destructor
+ // comment in "is_trivially_destructible = true" specialization below.
~OptionalStorageBase() {
if (!is_null_)
value_.~T();
@@ -78,9 +87,18 @@ struct OptionalStorageBase<T, true /* trivially destructible */> {
: is_null_(false), value_(std::forward<Args>(args)...) {}
// When T is trivially destructible (i.e. its destructor does nothing) there
- // is no need to call it. Explicitly defaulting the destructor means it's not
- // user-provided. Those two together make this destructor trivial.
- ~OptionalStorageBase() = default;
+ // is no need to call it. Implicitly defined destructor is trivial, because
+ // both members (bool and union containing only variants which are trivially
+ // destructible) are trivially destructible.
+ // Explicitly-defaulted destructor is also trivial, but do not use it here,
+ // because it hides the implicit move constructor. It is needed to implement
+ // constexpr move constructor in OptionalStorage iff T is trivially move
+ // constructible. Note that, if T is trivially move constructible, the move
+ // constructor of OptionalStorageBase<T> is also implicitly defined and it is
+ // trivially move constructor. If T is not trivially move constructible,
+ // "not declaring move constructor without destructor declaration" here means
+ // "delete move constructor", which works because any move constructor of
+ // OptionalStorage will not refer to it in that case.
template <class... Args>
void Init(Args&&... args) {
diff --git a/base/optional_unittest.cc b/base/optional_unittest.cc
index 91e63e75d0db..7cc05ef2987d 100644
--- a/base/optional_unittest.cc
+++ b/base/optional_unittest.cc
@@ -115,6 +115,35 @@ class DeletedDefaultConstructor {
int foo_;
};
+class DeletedCopyConstructor {
+ public:
+ explicit DeletedCopyConstructor(int foo) : foo_(foo) {}
+ DeletedCopyConstructor(const DeletedCopyConstructor&) = delete;
+ DeletedCopyConstructor(DeletedCopyConstructor&&) = default;
+
+ int foo() const { return foo_; }
+
+ private:
+ int foo_;
+};
+
+class NonTriviallyDestructibleDeletedCopyConstructor {
+ public:
+ explicit NonTriviallyDestructibleDeletedCopyConstructor(int foo)
+ : foo_(foo) {}
+ NonTriviallyDestructibleDeletedCopyConstructor(
+ const NonTriviallyDestructibleDeletedCopyConstructor&) = delete;
+ NonTriviallyDestructibleDeletedCopyConstructor(
+ NonTriviallyDestructibleDeletedCopyConstructor&&) = default;
+
+ ~NonTriviallyDestructibleDeletedCopyConstructor() {}
+
+ int foo() const { return foo_; }
+
+ private:
+ int foo_;
+};
+
class DeleteNewOperators {
public:
void* operator new(size_t) = delete;
@@ -168,6 +197,15 @@ TEST(OptionalTest, CopyConstructor) {
EXPECT_EQ(first, other);
}
+ {
+ const Optional<std::string> first("foo");
+ Optional<std::string> other(first);
+
+ EXPECT_TRUE(other);
+ EXPECT_EQ(other.value(), "foo");
+ EXPECT_EQ(first, other);
+ }
+
{
Optional<TestObject> first(TestObject(3, 0.1));
Optional<TestObject> other(first);
@@ -210,33 +248,57 @@ TEST(OptionalTest, MoveConstructor) {
constexpr Optional<float> first(0.1f);
constexpr Optional<float> second(std::move(first));
- EXPECT_TRUE(second);
+ EXPECT_TRUE(second.has_value());
EXPECT_EQ(second.value(), 0.1f);
- EXPECT_TRUE(first);
+ EXPECT_TRUE(first.has_value());
}
{
Optional<std::string> first("foo");
Optional<std::string> second(std::move(first));
- EXPECT_TRUE(second);
+ EXPECT_TRUE(second.has_value());
EXPECT_EQ("foo", second.value());
- EXPECT_TRUE(first);
+ EXPECT_TRUE(first.has_value());
}
{
Optional<TestObject> first(TestObject(3, 0.1));
Optional<TestObject> second(std::move(first));
- EXPECT_TRUE(!!second);
+ EXPECT_TRUE(second.has_value());
EXPECT_EQ(TestObject::State::MOVE_CONSTRUCTED, second->state());
EXPECT_TRUE(TestObject(3, 0.1) == second.value());
- EXPECT_TRUE(!!first);
+ EXPECT_TRUE(first.has_value());
EXPECT_EQ(TestObject::State::MOVED_FROM, first->state());
}
+
+ // Even if copy constructor is deleted, move constructor needs to work.
+ // Note that it couldn't be constexpr.
+ {
+ Optional<DeletedCopyConstructor> first(in_place, 42);
+ Optional<DeletedCopyConstructor> second(std::move(first));
+
+ EXPECT_TRUE(second.has_value());
+ EXPECT_EQ(42, second->foo());
+
+ EXPECT_TRUE(first.has_value());
+ }
+
+ {
+ Optional<NonTriviallyDestructibleDeletedCopyConstructor> first(in_place,
+ 42);
+ Optional<NonTriviallyDestructibleDeletedCopyConstructor> second(
+ std::move(first));
+
+ EXPECT_TRUE(second.has_value());
+ EXPECT_EQ(42, second->foo());
+
+ EXPECT_TRUE(first.has_value());
+ }
}
TEST(OptionalTest, MoveValueConstructor) {
--
2.16.2

View file

@ -0,0 +1,226 @@
From 9d9ecb3ba21a6dfe59e6740527dedb09b6caea68 Mon Sep 17 00:00:00 2001
From: Hidehiko Abe <hidehiko@chromium.org>
Date: Fri, 26 Jan 2018 18:01:11 +0000
Subject: [PATCH 08/10] Implement conditional copy/move ctors/assign-operators.
BUG=784732
TEST=Ran trybot.
Change-Id: Iec5f9eaa7482d4e23f5bf2eea4b34c9cd867f89d
Reviewed-on: https://chromium-review.googlesource.com/856021
Commit-Queue: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532004}
---
base/optional.h | 63 +++++++++++++++++++++++++++++++++++--
base/optional_unittest.cc | 80 +++++++++++++++++++++++++++++++++++++++++++----
2 files changed, 134 insertions(+), 9 deletions(-)
diff --git a/base/optional.h b/base/optional.h
index 5a50eb455be6..bb25118ca199 100644
--- a/base/optional.h
+++ b/base/optional.h
@@ -266,6 +266,58 @@ class OptionalBase {
OptionalStorage<T> storage_;
};
+// The following {Copy,Move}{Constructible,Assignable} structs are helpers to
+// implement constructor/assign-operator overloading. Specifically, if T is
+// is not movable but copyable, Optional<T>'s move constructor should not
+// participate in overload resolution. This inheritance trick implements that.
+template <bool is_copy_constructible>
+struct CopyConstructible {};
+
+template <>
+struct CopyConstructible<false> {
+ constexpr CopyConstructible() = default;
+ constexpr CopyConstructible(const CopyConstructible&) = delete;
+ constexpr CopyConstructible(CopyConstructible&&) = default;
+ CopyConstructible& operator=(const CopyConstructible&) = default;
+ CopyConstructible& operator=(CopyConstructible&&) = default;
+};
+
+template <bool is_move_constructible>
+struct MoveConstructible {};
+
+template <>
+struct MoveConstructible<false> {
+ constexpr MoveConstructible() = default;
+ constexpr MoveConstructible(const MoveConstructible&) = default;
+ constexpr MoveConstructible(MoveConstructible&&) = delete;
+ MoveConstructible& operator=(const MoveConstructible&) = default;
+ MoveConstructible& operator=(MoveConstructible&&) = default;
+};
+
+template <bool is_copy_assignable>
+struct CopyAssignable {};
+
+template <>
+struct CopyAssignable<false> {
+ constexpr CopyAssignable() = default;
+ constexpr CopyAssignable(const CopyAssignable&) = default;
+ constexpr CopyAssignable(CopyAssignable&&) = default;
+ CopyAssignable& operator=(const CopyAssignable&) = delete;
+ CopyAssignable& operator=(CopyAssignable&&) = default;
+};
+
+template <bool is_move_assignable>
+struct MoveAssignable {};
+
+template <>
+struct MoveAssignable<false> {
+ constexpr MoveAssignable() = default;
+ constexpr MoveAssignable(const MoveAssignable&) = default;
+ constexpr MoveAssignable(MoveAssignable&&) = default;
+ MoveAssignable& operator=(const MoveAssignable&) = default;
+ MoveAssignable& operator=(MoveAssignable&&) = delete;
+};
+
} // namespace internal
// base::Optional is a Chromium version of the C++17 optional class:
@@ -280,12 +332,18 @@ class OptionalBase {
// - No exceptions are thrown, because they are banned from Chromium.
// - All the non-members are in the 'base' namespace instead of 'std'.
template <typename T>
-class Optional : public internal::OptionalBase<T> {
+class Optional
+ : public internal::OptionalBase<T>,
+ public internal::CopyConstructible<std::is_copy_constructible<T>::value>,
+ public internal::MoveConstructible<std::is_move_constructible<T>::value>,
+ public internal::CopyAssignable<std::is_copy_constructible<T>::value &&
+ std::is_copy_assignable<T>::value>,
+ public internal::MoveAssignable<std::is_move_constructible<T>::value &&
+ std::is_move_assignable<T>::value> {
public:
using value_type = T;
// Defer default/copy/move constructor implementation to OptionalBase.
- // TODO(hidehiko): Implement conditional enabling.
constexpr Optional() = default;
constexpr Optional(const Optional& other) = default;
constexpr Optional(Optional&& other) = default;
@@ -316,7 +374,6 @@ class Optional : public internal::OptionalBase<T> {
~Optional() = default;
// Defer copy-/move- assign operator implementation to OptionalBase.
- // TOOD(hidehiko): Implement conditional enabling.
Optional& operator=(const Optional& other) = default;
Optional& operator=(Optional&& other) = default;
diff --git a/base/optional_unittest.cc b/base/optional_unittest.cc
index 7cc05ef2987d..09f3106bfa7f 100644
--- a/base/optional_unittest.cc
+++ b/base/optional_unittest.cc
@@ -115,11 +115,29 @@ class DeletedDefaultConstructor {
int foo_;
};
-class DeletedCopyConstructor {
+class DeletedCopy {
public:
- explicit DeletedCopyConstructor(int foo) : foo_(foo) {}
- DeletedCopyConstructor(const DeletedCopyConstructor&) = delete;
- DeletedCopyConstructor(DeletedCopyConstructor&&) = default;
+ explicit DeletedCopy(int foo) : foo_(foo) {}
+ DeletedCopy(const DeletedCopy&) = delete;
+ DeletedCopy(DeletedCopy&&) = default;
+
+ DeletedCopy& operator=(const DeletedCopy&) = delete;
+ DeletedCopy& operator=(DeletedCopy&&) = default;
+
+ int foo() const { return foo_; }
+
+ private:
+ int foo_;
+};
+
+class DeletedMove {
+ public:
+ explicit DeletedMove(int foo) : foo_(foo) {}
+ DeletedMove(const DeletedMove&) = default;
+ DeletedMove(DeletedMove&&) = delete;
+
+ DeletedMove& operator=(const DeletedMove&) = default;
+ DeletedMove& operator=(DeletedMove&&) = delete;
int foo() const { return foo_; }
@@ -279,8 +297,18 @@ TEST(OptionalTest, MoveConstructor) {
// Even if copy constructor is deleted, move constructor needs to work.
// Note that it couldn't be constexpr.
{
- Optional<DeletedCopyConstructor> first(in_place, 42);
- Optional<DeletedCopyConstructor> second(std::move(first));
+ Optional<DeletedCopy> first(in_place, 42);
+ Optional<DeletedCopy> second(std::move(first));
+
+ EXPECT_TRUE(second.has_value());
+ EXPECT_EQ(42, second->foo());
+
+ EXPECT_TRUE(first.has_value());
+ }
+
+ {
+ Optional<DeletedMove> first(in_place, 42);
+ Optional<DeletedMove> second(std::move(first));
EXPECT_TRUE(second.has_value());
EXPECT_EQ(42, second->foo());
@@ -465,6 +493,26 @@ TEST(OptionalTest, AssignObject) {
EXPECT_TRUE(a.value() == TestObject(3, 0.1));
EXPECT_TRUE(a == b);
}
+
+ {
+ Optional<DeletedMove> a(in_place, 42);
+ Optional<DeletedMove> b;
+ b = a;
+
+ EXPECT_TRUE(!!a);
+ EXPECT_TRUE(!!b);
+ EXPECT_EQ(a->foo(), b->foo());
+ }
+
+ {
+ Optional<DeletedMove> a(in_place, 42);
+ Optional<DeletedMove> b(in_place, 1);
+ b = a;
+
+ EXPECT_TRUE(!!a);
+ EXPECT_TRUE(!!b);
+ EXPECT_EQ(a->foo(), b->foo());
+ }
}
TEST(OptionalTest, AssignObject_rvalue) {
@@ -513,6 +561,26 @@ TEST(OptionalTest, AssignObject_rvalue) {
EXPECT_EQ(TestObject::State::MOVE_ASSIGNED, a->state());
EXPECT_EQ(TestObject::State::MOVED_FROM, b->state());
}
+
+ {
+ Optional<DeletedMove> a(in_place, 42);
+ Optional<DeletedMove> b;
+ b = std::move(a);
+
+ EXPECT_TRUE(!!a);
+ EXPECT_TRUE(!!b);
+ EXPECT_EQ(42, b->foo());
+ }
+
+ {
+ Optional<DeletedMove> a(in_place, 42);
+ Optional<DeletedMove> b(in_place, 1);
+ b = std::move(a);
+
+ EXPECT_TRUE(!!a);
+ EXPECT_TRUE(!!b);
+ EXPECT_EQ(42, b->foo());
+ }
}
TEST(OptionalTest, AssignNull) {
--
2.16.2

View file

@ -0,0 +1,36 @@
From 45e550e306307b21d697bae00ea4e8dd7e22f0ed Mon Sep 17 00:00:00 2001
From: Andrew Wilson <atwilson@chromium.org>
Date: Tue, 30 Jan 2018 14:47:47 +0000
Subject: [PATCH 09/10] Clear the testing provider on shutdown.
Restore code to clean up the test instance of the PolicyProvider, so unit tests
that set the PolicyProvider no longer crash when run in succession.
Bug: None
Test: Run unit_tests --gtest_filter=ProfilePolicyConnectorTest*
Change-Id: I3a90fb04f3736c7e1351791b52886d726913aeed
Reviewed-on: https://chromium-review.googlesource.com/893098
Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org>
Commit-Queue: Drew Wilson <atwilson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532865}
---
components/policy/core/browser/browser_policy_connector_base.cc | 3 +++
1 file changed, 3 insertions(+)
diff --git a/components/policy/core/browser/browser_policy_connector_base.cc b/components/policy/core/browser/browser_policy_connector_base.cc
index 35ce7605063d..cc9625f4f8af 100644
--- a/components/policy/core/browser/browser_policy_connector_base.cc
+++ b/components/policy/core/browser/browser_policy_connector_base.cc
@@ -60,6 +60,9 @@ void BrowserPolicyConnectorBase::Shutdown() {
for (const auto& provider : *policy_providers_)
provider->Shutdown();
}
+ // Drop g_testing_provider so that tests executed with --single_process can
+ // call SetPolicyProviderForTesting() again. It is still owned by the test.
+ g_testing_provider = nullptr;
g_created_policy_service = false;
}
--
2.16.2

View file

@ -0,0 +1,706 @@
From 0c98e899346fc34dca126f81c1a1e7feb692a96a Mon Sep 17 00:00:00 2001
From: Scott Violet <sky@chromium.org>
Date: Wed, 31 Jan 2018 00:58:18 +0000
Subject: [PATCH 10/10] cleanup how ConfigurationPolicyProviders are set
It's expected that the providers are available when the service is
committed, so it make it explicit.
BUG=none
TEST=none
Change-Id: Ibc2f9be6ecec9261e1a2b9ebef18b16557882398
Reviewed-on: https://chromium-review.googlesource.com/890722
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533126}
---
.../browser/aw_browser_policy_connector.cc | 11 ++--
.../browser/aw_browser_policy_connector.h | 15 +++--
chrome/browser/browser_process_impl.cc | 1 -
.../policy/browser_policy_connector_chromeos.cc | 8 ++-
.../policy/browser_policy_connector_chromeos.h | 5 +-
...cloud_external_data_policy_observer_unittest.cc | 6 +-
.../network_configuration_updater_unittest.cc | 3 +-
.../policy/chrome_browser_policy_connector.cc | 28 +++++----
.../policy/chrome_browser_policy_connector.h | 12 +---
chrome/browser/policy/profile_policy_connector.cc | 5 +-
.../policy/profile_policy_connector_factory.cc | 3 +-
chrome/browser/prefs/proxy_policy_unittest.cc | 3 +-
chrome/test/base/testing_profile.cc | 3 +-
.../core/browser/browser_policy_connector_base.cc | 67 +++++++++-------------
.../core/browser/browser_policy_connector_base.h | 19 +++---
.../configuration_policy_pref_store_test.cc | 3 +-
.../core/browser/proxy_policy_handler_unittest.cc | 3 +-
.../policy/core/common/policy_service_impl.cc | 50 ++++++----------
.../policy/core/common/policy_service_impl.h | 18 ++----
.../core/common/policy_service_impl_unittest.cc | 6 +-
remoting/host/policy_watcher.cc | 3 +-
21 files changed, 113 insertions(+), 159 deletions(-)
diff --git a/android_webview/browser/aw_browser_policy_connector.cc b/android_webview/browser/aw_browser_policy_connector.cc
index df0be7eaa5e2..523da4a49146 100644
--- a/android_webview/browser/aw_browser_policy_connector.cc
+++ b/android_webview/browser/aw_browser_policy_connector.cc
@@ -62,14 +62,17 @@ std::unique_ptr<policy::ConfigurationPolicyHandlerList> BuildHandlerList(
} // namespace
AwBrowserPolicyConnector::AwBrowserPolicyConnector()
- : BrowserPolicyConnectorBase(base::Bind(&BuildHandlerList)) {
+ : BrowserPolicyConnectorBase(base::Bind(&BuildHandlerList)) {}
+
+AwBrowserPolicyConnector::~AwBrowserPolicyConnector() = default;
+
+std::vector<std::unique_ptr<policy::ConfigurationPolicyProvider>>
+AwBrowserPolicyConnector::CreatePolicyProviders() {
std::vector<std::unique_ptr<policy::ConfigurationPolicyProvider>> providers;
providers.push_back(
std::make_unique<policy::android::AndroidCombinedPolicyProvider>(
GetSchemaRegistry()));
- SetPolicyProviders(std::move(providers));
+ return providers;
}
-AwBrowserPolicyConnector::~AwBrowserPolicyConnector() {}
-
} // namespace android_webview
diff --git a/android_webview/browser/aw_browser_policy_connector.h b/android_webview/browser/aw_browser_policy_connector.h
index 4530657c5150..65b2cce56f69 100644
--- a/android_webview/browser/aw_browser_policy_connector.h
+++ b/android_webview/browser/aw_browser_policy_connector.h
@@ -13,12 +13,17 @@ namespace android_webview {
// Sets up and keeps the browser-global policy objects such as the PolicyService
// and the platform-specific PolicyProvider.
class AwBrowserPolicyConnector : public policy::BrowserPolicyConnectorBase {
-public:
- AwBrowserPolicyConnector();
- ~AwBrowserPolicyConnector() override;
+ public:
+ AwBrowserPolicyConnector();
+ ~AwBrowserPolicyConnector() override;
-private:
- DISALLOW_COPY_AND_ASSIGN(AwBrowserPolicyConnector);
+ protected:
+ // policy::BrowserPolicyConnectorBase:
+ std::vector<std::unique_ptr<policy::ConfigurationPolicyProvider>>
+ CreatePolicyProviders() override;
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(AwBrowserPolicyConnector);
};
} // namespace android_webview
diff --git a/chrome/browser/browser_process_impl.cc b/chrome/browser/browser_process_impl.cc
index d96fa3319a81..a7b5b1323b0e 100644
--- a/chrome/browser/browser_process_impl.cc
+++ b/chrome/browser/browser_process_impl.cc
@@ -650,7 +650,6 @@ BrowserProcessImpl::browser_policy_connector() {
DCHECK(!browser_policy_connector_);
browser_policy_connector_ = platform_part_->CreateBrowserPolicyConnector();
created_browser_policy_connector_ = true;
- browser_policy_connector_->InitPolicyProviders();
}
return browser_policy_connector_.get();
}
diff --git a/chrome/browser/chromeos/policy/browser_policy_connector_chromeos.cc b/chrome/browser/chromeos/policy/browser_policy_connector_chromeos.cc
index 809baa402cdc..15e9a3841e96 100644
--- a/chrome/browser/chromeos/policy/browser_policy_connector_chromeos.cc
+++ b/chrome/browser/chromeos/policy/browser_policy_connector_chromeos.cc
@@ -355,11 +355,13 @@ void BrowserPolicyConnectorChromeOS::OnDeviceCloudPolicyManagerDisconnected() {
RestartDeviceCloudPolicyInitializer();
}
-void BrowserPolicyConnectorChromeOS::BuildPolicyProviders(
- std::vector<std::unique_ptr<ConfigurationPolicyProvider>>* providers) {
+std::vector<std::unique_ptr<policy::ConfigurationPolicyProvider>>
+BrowserPolicyConnectorChromeOS::CreatePolicyProviders() {
+ auto providers = ChromeBrowserPolicyConnector::CreatePolicyProviders();
for (auto& provider_ptr : providers_for_init_)
- providers->push_back(std::move(provider_ptr));
+ providers.push_back(std::move(provider_ptr));
providers_for_init_.clear();
+ return providers;
}
void BrowserPolicyConnectorChromeOS::SetTimezoneIfPolicyAvailable() {
diff --git a/chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h b/chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h
index d79f700a37f9..c72c00cbba4d 100644
--- a/chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h
+++ b/chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h
@@ -180,9 +180,8 @@ class BrowserPolicyConnectorChromeOS
protected:
// ChromeBrowserPolicyConnector:
- void BuildPolicyProviders(
- std::vector<std::unique_ptr<ConfigurationPolicyProvider>>* providers)
- override;
+ std::vector<std::unique_ptr<policy::ConfigurationPolicyProvider>>
+ CreatePolicyProviders() override;
private:
// Set the timezone as soon as the policies are available.
diff --git a/chrome/browser/chromeos/policy/cloud_external_data_policy_observer_unittest.cc b/chrome/browser/chromeos/policy/cloud_external_data_policy_observer_unittest.cc
index 70da940dae38..40465dc207e9 100644
--- a/chrome/browser/chromeos/policy/cloud_external_data_policy_observer_unittest.cc
+++ b/chrome/browser/chromeos/policy/cloud_external_data_policy_observer_unittest.cc
@@ -334,8 +334,7 @@ void CloudExternalDataPolicyObserverTest::LogInAsDeviceLocalAccount(
providers.push_back(device_local_account_policy_provider_.get());
TestingProfile::Builder builder;
std::unique_ptr<PolicyServiceImpl> policy_service =
- std::make_unique<PolicyServiceImpl>();
- policy_service->SetProviders(providers);
+ std::make_unique<PolicyServiceImpl>(std::move(providers));
builder.SetPolicyService(std::move(policy_service));
builder.SetPath(chromeos::ProfileHelper::Get()->GetProfilePathByUserIdHash(
chromeos::ProfileHelper::GetUserIdHashByUserIdForTesting(
@@ -370,8 +369,7 @@ void CloudExternalDataPolicyObserverTest::LogInAsRegularUser() {
providers.push_back(&user_policy_provider_);
TestingProfile::Builder builder;
std::unique_ptr<PolicyServiceImpl> policy_service =
- std::make_unique<PolicyServiceImpl>();
- policy_service->SetProviders(providers);
+ std::make_unique<PolicyServiceImpl>(std::move(providers));
builder.SetPolicyService(std::move(policy_service));
builder.SetPath(chromeos::ProfileHelper::Get()->GetProfilePathByUserIdHash(
chromeos::ProfileHelper::GetUserIdHashByUserIdForTesting(
diff --git a/chrome/browser/chromeos/policy/network_configuration_updater_unittest.cc b/chrome/browser/chromeos/policy/network_configuration_updater_unittest.cc
index 6dfce500dacc..a184fd895368 100644
--- a/chrome/browser/chromeos/policy/network_configuration_updater_unittest.cc
+++ b/chrome/browser/chromeos/policy/network_configuration_updater_unittest.cc
@@ -220,8 +220,7 @@ class NetworkConfigurationUpdaterTest : public testing::Test {
provider_.Init();
PolicyServiceImpl::Providers providers;
providers.push_back(&provider_);
- policy_service_ = std::make_unique<PolicyServiceImpl>();
- policy_service_->SetProviders(providers);
+ policy_service_ = std::make_unique<PolicyServiceImpl>(std::move(providers));
std::unique_ptr<base::DictionaryValue> fake_toplevel_onc =
chromeos::onc::ReadDictionaryFromJson(kFakeONC);
diff --git a/chrome/browser/policy/chrome_browser_policy_connector.cc b/chrome/browser/policy/chrome_browser_policy_connector.cc
index 4facd0f2fee5..683e9ad3a267 100644
--- a/chrome/browser/policy/chrome_browser_policy_connector.cc
+++ b/chrome/browser/policy/chrome_browser_policy_connector.cc
@@ -71,18 +71,6 @@ void ChromeBrowserPolicyConnector::OnResourceBundleCreated() {
BrowserPolicyConnectorBase::OnResourceBundleCreated();
}
-void ChromeBrowserPolicyConnector::InitPolicyProviders() {
- std::vector<std::unique_ptr<ConfigurationPolicyProvider>> providers;
- std::unique_ptr<ConfigurationPolicyProvider> platform_provider =
- CreatePlatformProvider();
- if (platform_provider) {
- platform_provider_ = platform_provider.get();
- providers.push_back(std::move(platform_provider));
- }
- BuildPolicyProviders(&providers);
- SetPolicyProviders(std::move(providers));
-}
-
void ChromeBrowserPolicyConnector::Init(
PrefService* local_state,
scoped_refptr<net::URLRequestContextGetter> request_context) {
@@ -104,6 +92,19 @@ ChromeBrowserPolicyConnector::GetPlatformProvider() {
return provider ? provider : platform_provider_;
}
+std::vector<std::unique_ptr<policy::ConfigurationPolicyProvider>>
+ChromeBrowserPolicyConnector::CreatePolicyProviders() {
+ auto providers = BrowserPolicyConnector::CreatePolicyProviders();
+ std::unique_ptr<ConfigurationPolicyProvider> platform_provider =
+ CreatePlatformProvider();
+ if (platform_provider) {
+ platform_provider_ = platform_provider.get();
+ // PlatformProvider should be before all other providers (highest priority).
+ providers.insert(providers.begin(), std::move(platform_provider));
+ }
+ return providers;
+}
+
std::unique_ptr<ConfigurationPolicyProvider>
ChromeBrowserPolicyConnector::CreatePlatformProvider() {
#if defined(OS_WIN)
@@ -140,7 +141,4 @@ ChromeBrowserPolicyConnector::CreatePlatformProvider() {
#endif
}
-void ChromeBrowserPolicyConnector::BuildPolicyProviders(
- std::vector<std::unique_ptr<ConfigurationPolicyProvider>>* providers) {}
-
} // namespace policy
diff --git a/chrome/browser/policy/chrome_browser_policy_connector.h b/chrome/browser/policy/chrome_browser_policy_connector.h
index 14f1ddfae882..5b21e20fd5b3 100644
--- a/chrome/browser/policy/chrome_browser_policy_connector.h
+++ b/chrome/browser/policy/chrome_browser_policy_connector.h
@@ -42,9 +42,6 @@ class ChromeBrowserPolicyConnector : public BrowserPolicyConnector {
// class to notify observers.
void OnResourceBundleCreated();
- // TODO(sky): remove. Temporary until resolve ordering.
- void InitPolicyProviders();
-
void Init(
PrefService* local_state,
scoped_refptr<net::URLRequestContextGetter> request_context) override;
@@ -52,12 +49,9 @@ class ChromeBrowserPolicyConnector : public BrowserPolicyConnector {
ConfigurationPolicyProvider* GetPlatformProvider();
protected:
- // Called from Init() to build the list of ConfigurationPolicyProviders that
- // is supplied to SetPolicyProviders(). This implementation does nothing
- // and is provided for subclasses. NOTE: |providers| may already contain
- // some providers, generally subclasses should append.
- virtual void BuildPolicyProviders(
- std::vector<std::unique_ptr<ConfigurationPolicyProvider>>* providers);
+ // BrowserPolicyConnector:
+ std::vector<std::unique_ptr<policy::ConfigurationPolicyProvider>>
+ CreatePolicyProviders() override;
private:
std::unique_ptr<ConfigurationPolicyProvider> CreatePlatformProvider();
diff --git a/chrome/browser/policy/profile_policy_connector.cc b/chrome/browser/policy/profile_policy_connector.cc
index 1ca34bab0e5f..de4286dcdbf0 100644
--- a/chrome/browser/policy/profile_policy_connector.cc
+++ b/chrome/browser/policy/profile_policy_connector.cc
@@ -104,10 +104,7 @@ void ProfilePolicyConnector::Init(
}
#endif
- std::unique_ptr<PolicyServiceImpl> policy_service =
- std::make_unique<PolicyServiceImpl>();
- policy_service->SetProviders(policy_providers_);
- policy_service_ = std::move(policy_service);
+ policy_service_ = std::make_unique<PolicyServiceImpl>(policy_providers_);
#if defined(OS_CHROMEOS)
if (is_primary_user_) {
diff --git a/chrome/browser/policy/profile_policy_connector_factory.cc b/chrome/browser/policy/profile_policy_connector_factory.cc
index e96938ca1ea7..e8a5f0814d34 100644
--- a/chrome/browser/policy/profile_policy_connector_factory.cc
+++ b/chrome/browser/policy/profile_policy_connector_factory.cc
@@ -154,8 +154,7 @@ ProfilePolicyConnectorFactory::CreateForBrowserContextInternal(
providers.push_back(test_providers_.front());
test_providers_.pop_front();
std::unique_ptr<PolicyServiceImpl> service =
- std::make_unique<PolicyServiceImpl>();
- service->SetProviders(providers);
+ std::make_unique<PolicyServiceImpl>(std::move(providers));
connector->InitForTesting(std::move(service));
}
diff --git a/chrome/browser/prefs/proxy_policy_unittest.cc b/chrome/browser/prefs/proxy_policy_unittest.cc
index 4ed5293deeab..f387aa4640ad 100644
--- a/chrome/browser/prefs/proxy_policy_unittest.cc
+++ b/chrome/browser/prefs/proxy_policy_unittest.cc
@@ -98,8 +98,7 @@ class ProxyPolicyTest : public testing::Test {
PolicyServiceImpl::Providers providers;
providers.push_back(&provider_);
- policy_service_ = std::make_unique<PolicyServiceImpl>();
- policy_service_->SetProviders(providers);
+ policy_service_ = std::make_unique<PolicyServiceImpl>(std::move(providers));
provider_.Init();
}
diff --git a/chrome/test/base/testing_profile.cc b/chrome/test/base/testing_profile.cc
index 19a9b0e57d09..429b1cfa6d11 100644
--- a/chrome/test/base/testing_profile.cc
+++ b/chrome/test/base/testing_profile.cc
@@ -800,8 +800,7 @@ void TestingProfile::CreateProfilePolicyConnector() {
if (!policy_service_) {
std::vector<policy::ConfigurationPolicyProvider*> providers;
std::unique_ptr<policy::PolicyServiceImpl> policy_service =
- std::make_unique<policy::PolicyServiceImpl>();
- policy_service->SetProviders(providers);
+ std::make_unique<policy::PolicyServiceImpl>(std::move(providers));
policy_service_ = std::move(policy_service);
}
profile_policy_connector_.reset(new policy::ProfilePolicyConnector());
diff --git a/components/policy/core/browser/browser_policy_connector_base.cc b/components/policy/core/browser/browser_policy_connector_base.cc
index cc9625f4f8af..ccac08072846 100644
--- a/components/policy/core/browser/browser_policy_connector_base.cc
+++ b/components/policy/core/browser/browser_policy_connector_base.cc
@@ -26,8 +26,7 @@ ConfigurationPolicyProvider* g_testing_provider = nullptr;
} // namespace
BrowserPolicyConnectorBase::BrowserPolicyConnectorBase(
- const HandlerListFactory& handler_list_factory)
- : is_initialized_(false) {
+ const HandlerListFactory& handler_list_factory) {
// GetPolicyService() must be ready after the constructor is done.
// The connector is created very early during startup, when the browser
// threads aren't running yet; initialize components that need local_state,
@@ -56,10 +55,8 @@ void BrowserPolicyConnectorBase::Shutdown() {
is_initialized_ = false;
if (g_testing_provider)
g_testing_provider->Shutdown();
- if (policy_providers_) {
- for (const auto& provider : *policy_providers_)
- provider->Shutdown();
- }
+ for (const auto& provider : policy_providers_)
+ provider->Shutdown();
// Drop g_testing_provider so that tests executed with --single_process can
// call SetPolicyProviderForTesting() again. It is still owned by the test.
g_testing_provider = nullptr;
@@ -75,12 +72,23 @@ CombinedSchemaRegistry* BrowserPolicyConnectorBase::GetSchemaRegistry() {
}
PolicyService* BrowserPolicyConnectorBase::GetPolicyService() {
- if (!policy_service_) {
- g_created_policy_service = true;
- policy_service_ = std::make_unique<PolicyServiceImpl>();
- if (policy_providers_ || g_testing_provider)
- policy_service_->SetProviders(GetProvidersForPolicyService());
- }
+ if (policy_service_)
+ return policy_service_.get();
+
+ DCHECK(!is_initialized_);
+ is_initialized_ = true;
+
+ policy_providers_ = CreatePolicyProviders();
+
+ if (g_testing_provider)
+ g_testing_provider->Init(GetSchemaRegistry());
+
+ for (const auto& provider : policy_providers_)
+ provider->Init(GetSchemaRegistry());
+
+ g_created_policy_service = true;
+ policy_service_ =
+ std::make_unique<PolicyServiceImpl>(GetProvidersForPolicyService());
return policy_service_.get();
}
@@ -111,32 +119,6 @@ BrowserPolicyConnectorBase::GetPolicyProviderForTesting() {
return g_testing_provider;
}
-void BrowserPolicyConnectorBase::SetPolicyProviders(
- std::vector<std::unique_ptr<ConfigurationPolicyProvider>> providers) {
- // SetPolicyProviders() should only called once.
- DCHECK(!is_initialized_);
- policy_providers_ = std::move(providers);
-
- if (g_testing_provider)
- g_testing_provider->Init(GetSchemaRegistry());
-
- for (const auto& provider : *policy_providers_)
- provider->Init(GetSchemaRegistry());
-
- is_initialized_ = true;
-
- if (policy_service_) {
- if (!policy_service_->has_providers()) {
- policy_service_->SetProviders(GetProvidersForPolicyService());
- } else {
- // GetPolicyService() triggers calling SetProviders() if
- // |g_testing_provider| has been set. That's the only way that should
- // result in ending up in this branch.
- DCHECK(g_testing_provider);
- }
- }
-}
-
std::vector<ConfigurationPolicyProvider*>
BrowserPolicyConnectorBase::GetProvidersForPolicyService() {
std::vector<ConfigurationPolicyProvider*> providers;
@@ -144,12 +126,17 @@ BrowserPolicyConnectorBase::GetProvidersForPolicyService() {
providers.push_back(g_testing_provider);
return providers;
}
- providers.reserve(policy_providers_->size());
- for (const auto& policy : *policy_providers_)
+ providers.reserve(policy_providers_.size());
+ for (const auto& policy : policy_providers_)
providers.push_back(policy.get());
return providers;
}
+std::vector<std::unique_ptr<ConfigurationPolicyProvider>>
+BrowserPolicyConnectorBase::CreatePolicyProviders() {
+ return {};
+}
+
void BrowserPolicyConnectorBase::OnResourceBundleCreated() {
std::vector<base::OnceClosure> resource_bundle_callbacks;
std::swap(resource_bundle_callbacks, resource_bundle_callbacks_);
diff --git a/components/policy/core/browser/browser_policy_connector_base.h b/components/policy/core/browser/browser_policy_connector_base.h
index a7674b55cdf4..d2d05f788c5f 100644
--- a/components/policy/core/browser/browser_policy_connector_base.h
+++ b/components/policy/core/browser/browser_policy_connector_base.h
@@ -10,7 +10,6 @@
#include "base/callback_forward.h"
#include "base/macros.h"
-#include "base/optional.h"
#include "components/policy/core/browser/configuration_policy_handler_list.h"
#include "components/policy/core/common/schema.h"
#include "components/policy/core/common/schema_registry.h"
@@ -73,10 +72,11 @@ class POLICY_EXPORT BrowserPolicyConnectorBase {
explicit BrowserPolicyConnectorBase(
const HandlerListFactory& handler_list_factory);
- // Sets the set of providers, in decreasing order of priority. May only be
- // called once.
- void SetPolicyProviders(
- std::vector<std::unique_ptr<ConfigurationPolicyProvider>> providers);
+ // Called from GetPolicyService() to create the set of
+ // ConfigurationPolicyProviders that are used, in decreasing order of
+ // priority.
+ virtual std::vector<std::unique_ptr<ConfigurationPolicyProvider>>
+ CreatePolicyProviders();
// Must be called when ui::ResourceBundle has been loaded, results in running
// any callbacks scheduled in NotifyWhenResourceBundleReady().
@@ -88,8 +88,10 @@ class POLICY_EXPORT BrowserPolicyConnectorBase {
// called.
std::vector<ConfigurationPolicyProvider*> GetProvidersForPolicyService();
- // Whether SetPolicyProviders() but not Shutdown() has been invoked.
- bool is_initialized_;
+ // Set to true when the PolicyService has been created, and false in
+ // Shutdown(). Once created the PolicyService is destroyed in the destructor,
+ // not Shutdown().
+ bool is_initialized_ = false;
// Used to convert policies to preferences. The providers declared below
// may trigger policy updates during shutdown, which will result in
@@ -105,8 +107,7 @@ class POLICY_EXPORT BrowserPolicyConnectorBase {
CombinedSchemaRegistry schema_registry_;
// The browser-global policy providers, in decreasing order of priority.
- base::Optional<std::vector<std::unique_ptr<ConfigurationPolicyProvider>>>
- policy_providers_;
+ std::vector<std::unique_ptr<ConfigurationPolicyProvider>> policy_providers_;
// Must be deleted before all the policy providers.
std::unique_ptr<PolicyServiceImpl> policy_service_;
diff --git a/components/policy/core/browser/configuration_policy_pref_store_test.cc b/components/policy/core/browser/configuration_policy_pref_store_test.cc
index fbdc51e938a6..efd4559a95a3 100644
--- a/components/policy/core/browser/configuration_policy_pref_store_test.cc
+++ b/components/policy/core/browser/configuration_policy_pref_store_test.cc
@@ -30,8 +30,7 @@ ConfigurationPolicyPrefStoreTest::ConfigurationPolicyPrefStoreTest()
.WillRepeatedly(Return(false));
provider_.Init();
providers_.push_back(&provider_);
- policy_service_ = std::make_unique<PolicyServiceImpl>();
- policy_service_->SetProviders(providers_);
+ policy_service_ = std::make_unique<PolicyServiceImpl>(providers_);
store_ = new ConfigurationPolicyPrefStore(
nullptr, policy_service_.get(), &handler_list_, POLICY_LEVEL_MANDATORY);
}
diff --git a/components/policy/core/browser/proxy_policy_handler_unittest.cc b/components/policy/core/browser/proxy_policy_handler_unittest.cc
index 8ee359336b62..6bd9bd54c8dc 100644
--- a/components/policy/core/browser/proxy_policy_handler_unittest.cc
+++ b/components/policy/core/browser/proxy_policy_handler_unittest.cc
@@ -32,8 +32,7 @@ class ProxyPolicyHandlerTest
// preprocessor. The previous store must be nulled out first so that it
// removes itself from the service's observer list.
store_ = nullptr;
- policy_service_ = std::make_unique<PolicyServiceImpl>();
- policy_service_->SetProviders(providers_);
+ policy_service_ = std::make_unique<PolicyServiceImpl>(providers_);
store_ = new ConfigurationPolicyPrefStore(
nullptr, policy_service_.get(), &handler_list_, POLICY_LEVEL_MANDATORY);
}
diff --git a/components/policy/core/common/policy_service_impl.cc b/components/policy/core/common/policy_service_impl.cc
index 42b6138c853c..dd555a14634e 100644
--- a/components/policy/core/common/policy_service_impl.cc
+++ b/components/policy/core/common/policy_service_impl.cc
@@ -72,25 +72,12 @@ void RemapProxyPolicies(PolicyMap* policies) {
} // namespace
-PolicyServiceImpl::PolicyServiceImpl() : update_task_ptr_factory_(this) {
- for (int domain = 0; domain < POLICY_DOMAIN_SIZE; ++domain)
- initialization_complete_[domain] = false;
-}
-
-PolicyServiceImpl::~PolicyServiceImpl() {
- DCHECK(thread_checker_.CalledOnValidThread());
- if (providers_) {
- for (auto* provider : *providers_)
- provider->RemoveObserver(this);
- }
-}
-
-void PolicyServiceImpl::SetProviders(Providers providers) {
- DCHECK(!providers_);
+PolicyServiceImpl::PolicyServiceImpl(Providers providers)
+ : update_task_ptr_factory_(this) {
providers_ = std::move(providers);
for (int domain = 0; domain < POLICY_DOMAIN_SIZE; ++domain)
initialization_complete_[domain] = true;
- for (auto* provider : *providers_) {
+ for (auto* provider : providers_) {
provider->AddObserver(this);
for (int domain = 0; domain < POLICY_DOMAIN_SIZE; ++domain) {
initialization_complete_[domain] &=
@@ -102,6 +89,12 @@ void PolicyServiceImpl::SetProviders(Providers providers) {
MergeAndTriggerUpdates();
}
+PolicyServiceImpl::~PolicyServiceImpl() {
+ DCHECK(thread_checker_.CalledOnValidThread());
+ for (auto* provider : providers_)
+ provider->RemoveObserver(this);
+}
+
void PolicyServiceImpl::AddObserver(PolicyDomain domain,
PolicyService::Observer* observer) {
DCHECK(thread_checker_.CalledOnValidThread());
@@ -143,7 +136,7 @@ void PolicyServiceImpl::RefreshPolicies(const base::Closure& callback) {
if (!callback.is_null())
refresh_callbacks_.push_back(callback);
- if (!providers_ || providers_->empty()) {
+ if (providers_.empty()) {
// Refresh is immediately complete if there are no providers. See the note
// on OnUpdatePolicy() about why this is a posted task.
update_task_ptr_factory_.InvalidateWeakPtrs();
@@ -153,15 +146,15 @@ void PolicyServiceImpl::RefreshPolicies(const base::Closure& callback) {
} else {
// Some providers might invoke OnUpdatePolicy synchronously while handling
// RefreshPolicies. Mark all as pending before refreshing.
- for (auto* provider : *providers_)
+ for (auto* provider : providers_)
refresh_pending_.insert(provider);
- for (auto* provider : *providers_)
+ for (auto* provider : providers_)
provider->RefreshPolicies();
}
}
void PolicyServiceImpl::OnUpdatePolicy(ConfigurationPolicyProvider* provider) {
- DCHECK_EQ(1, std::count(providers_->begin(), providers_->end(), provider));
+ DCHECK_EQ(1, std::count(providers_.begin(), providers_.end(), provider));
refresh_pending_.erase(provider);
// Note: a policy change may trigger further policy changes in some providers.
@@ -194,13 +187,11 @@ void PolicyServiceImpl::MergeAndTriggerUpdates() {
// Merge from each provider in their order of priority.
const PolicyNamespace chrome_namespace(POLICY_DOMAIN_CHROME, std::string());
PolicyBundle bundle;
- if (providers_) {
- for (auto* provider : *providers_) {
- PolicyBundle provided_bundle;
- provided_bundle.CopyFrom(provider->policies());
- RemapProxyPolicies(&provided_bundle.Get(chrome_namespace));
- bundle.MergeFrom(provided_bundle);
- }
+ for (auto* provider : providers_) {
+ PolicyBundle provided_bundle;
+ provided_bundle.CopyFrom(provider->policies());
+ RemapProxyPolicies(&provided_bundle.Get(chrome_namespace));
+ bundle.MergeFrom(provided_bundle);
}
// Swap first, so that observers that call GetPolicies() see the current
@@ -247,9 +238,6 @@ void PolicyServiceImpl::MergeAndTriggerUpdates() {
void PolicyServiceImpl::CheckInitializationComplete() {
DCHECK(thread_checker_.CalledOnValidThread());
- if (!providers_)
- return;
-
// Check if all the providers just became initialized for each domain; if so,
// notify that domain's observers.
for (int domain = 0; domain < POLICY_DOMAIN_SIZE; ++domain) {
@@ -259,7 +247,7 @@ void PolicyServiceImpl::CheckInitializationComplete() {
PolicyDomain policy_domain = static_cast<PolicyDomain>(domain);
bool all_complete = true;
- for (auto* provider : *providers_) {
+ for (auto* provider : providers_) {
if (!provider->IsInitializationComplete(policy_domain)) {
all_complete = false;
break;
diff --git a/components/policy/core/common/policy_service_impl.h b/components/policy/core/common/policy_service_impl.h
index 0e6003e87b89..985b27e257f1 100644
--- a/components/policy/core/common/policy_service_impl.h
+++ b/components/policy/core/common/policy_service_impl.h
@@ -15,7 +15,6 @@
#include "base/macros.h"
#include "base/memory/weak_ptr.h"
#include "base/observer_list.h"
-#include "base/optional.h"
#include "base/threading/thread_checker.h"
#include "components/policy/core/common/configuration_policy_provider.h"
#include "components/policy/core/common/policy_bundle.h"
@@ -32,18 +31,11 @@ class POLICY_EXPORT PolicyServiceImpl
public:
using Providers = std::vector<ConfigurationPolicyProvider*>;
- // Creates a new PolicyServiceImpl, it is expected SetProviders() is called
- // once to complete initialization.
- PolicyServiceImpl();
-
+ // Creates a new PolicyServiceImpl with the list of
+ // ConfigurationPolicyProviders, in order of decreasing priority.
+ explicit PolicyServiceImpl(Providers providers);
~PolicyServiceImpl() override;
- // Sets the providers; see description of constructor for details.
- void SetProviders(Providers providers);
-
- // Returns true if SetProviders() was called.
- bool has_providers() const { return providers_.has_value(); }
-
// PolicyService overrides:
void AddObserver(PolicyDomain domain,
PolicyService::Observer* observer) override;
@@ -76,8 +68,8 @@ class POLICY_EXPORT PolicyServiceImpl
// Invokes all the refresh callbacks if there are no more refreshes pending.
void CheckRefreshComplete();
- // The providers set via SetProviders(), in order of decreasing priority.
- base::Optional<Providers> providers_;
+ // The providers, in order of decreasing priority.
+ Providers providers_;
// Maps each policy namespace to its current policies.
PolicyBundle policy_bundle_;
diff --git a/components/policy/core/common/policy_service_impl_unittest.cc b/components/policy/core/common/policy_service_impl_unittest.cc
index 857e38226e83..bc84baa9d694 100644
--- a/components/policy/core/common/policy_service_impl_unittest.cc
+++ b/components/policy/core/common/policy_service_impl_unittest.cc
@@ -120,8 +120,7 @@ class PolicyServiceTest : public testing::Test {
providers.push_back(&provider0_);
providers.push_back(&provider1_);
providers.push_back(&provider2_);
- policy_service_ = std::make_unique<PolicyServiceImpl>();
- policy_service_->SetProviders(providers);
+ policy_service_ = std::make_unique<PolicyServiceImpl>(std::move(providers));
}
void TearDown() override {
@@ -561,8 +560,7 @@ TEST_F(PolicyServiceTest, IsInitializationComplete) {
providers.push_back(&provider0_);
providers.push_back(&provider1_);
providers.push_back(&provider2_);
- policy_service_ = std::make_unique<PolicyServiceImpl>();
- policy_service_->SetProviders(providers);
+ policy_service_ = std::make_unique<PolicyServiceImpl>(std::move(providers));
EXPECT_FALSE(policy_service_->IsInitializationComplete(POLICY_DOMAIN_CHROME));
EXPECT_FALSE(
policy_service_->IsInitializationComplete(POLICY_DOMAIN_EXTENSIONS));
diff --git a/remoting/host/policy_watcher.cc b/remoting/host/policy_watcher.cc
index 297dcd7b2a59..c428bad430f2 100644
--- a/remoting/host/policy_watcher.cc
+++ b/remoting/host/policy_watcher.cc
@@ -376,8 +376,7 @@ std::unique_ptr<PolicyWatcher> PolicyWatcher::CreateFromPolicyLoader(
policy::PolicyServiceImpl::Providers providers;
providers.push_back(policy_provider.get());
std::unique_ptr<policy::PolicyServiceImpl> policy_service =
- std::make_unique<policy::PolicyServiceImpl>();
- policy_service->SetProviders(providers);
+ std::make_unique<policy::PolicyServiceImpl>(std::move(providers));
policy::PolicyService* borrowed_policy_service = policy_service.get();
return base::WrapUnique(new PolicyWatcher(
--
2.16.2

View file

@ -37,8 +37,17 @@ source=(https://commondatastorage.googleapis.com/chromium-browser-official/$pkgn
chromium-math.h-r0.patch
chromium-stdint.patch
chromium-widevine.patch
chromium-angle-r0.patch
0001-ARM-toolchain-fixes.patch)
0001-ARM-toolchain-fixes.patch
0002-GCC-build-fix-mark-is_trivially_copy_constructible-f.patch
0003-Workaround-for-g-7-is_trivially_copy_constructible-f.patch
0004-GCC-build-fix-base-Optional-T-requires-the-full-decl.patch
0005-GCC-IDB-methods-String-renamed-to-GetString.patch
0006-GCC-PlaybackImageProvider-Settings-explicitely-set-c.patch
0007-Fix-non-copyable-class-s-optional-move.patch
0008-Implement-conditional-copy-move-ctors-assign-operato.patch
0009-Clear-the-testing-provider-on-shutdown.patch
0010-cleanup-how-ConfigurationPolicyProviders-are-set.patch
skia_buildfix.patch)
sha256sums=('627e7bfd84795de1553fac305239130d25186acf2d3c77d39d824327cd116cce'
'4dc3428f2c927955d9ae117f2fb24d098cc6dd67adb760ac9c82b522ec8b0587'
'bed2a7ef4b1ebd53b28e2f38963a2dd761267ccc8818693c34ce8596db53dd4c'
@ -47,8 +56,17 @@ sha256sums=('627e7bfd84795de1553fac305239130d25186acf2d3c77d39d824327cd116cce'
'fe0ab86aa5b0072db730eccda3e1582ebed4af25815bfd49fe0da24cf63ca902'
'c00d2506f1078b38a8ebec474a7318e76a61db1298afb40088a34210f137210f'
'd6fdcb922e5a7fbe15759d39ccc8ea4225821c44d98054ce0f23f9d1f00c9808'
'ec9dbff87e8a8655090eaa456905b67936c3d0d48d6fd84e088864af39ceed47'
'8489d2a85b32e9b08f5c30d47b40ae41911d1915f48b6de3c41fe350dfecfef6')
'66c665564d2055c570a058a5ba75d92cb39b6e1d09e58d6d1ccbed3a432c5efc'
'6a59ec7dbf3e09c5fb991f8902262faf951abc7e9a0b46aa08f3ccee7d36531b'
'06723f032086a80002e0982ce5872a392c82d99ce7ccd93719775423bea6b360'
'8bfe11e36521d00dbec018ab5e8d3e41da72b90173cafd3768c7c80ff096d49a'
'96646072a129166ac62ba269caddbbe6dd814f70f34d55014f78d1e69d47bc06'
'd512efc5780b0d187764cb7b52a71297b5423480d91a7cf5ec36690714b9551b'
'1b756d2d23724003e2f0516525c86218dcf49b14e776081f9d904fee847a243d'
'5087247e56a1196e3ef6b27bbb0aa9986288687183de78b8dea3d68f9f69451d'
'2cc6abc906f267a6c2621a793dffd20d99b3d7f017bd574c79dbe7ef8f6f8730'
'df836800052c1a9a29b048cffb542169a9f4dfe0383207322a2d85fa87c9423e'
'3da0ab55bf92307fe80ed7c9cce6443f5bebc96ba50e751d24f7302786a0406e')
# Possible replacements are listed in build/linux/unbundle/replace_gn_files.py
# Keys are the names in the above script; values are the dependencies in Arch
@ -100,6 +118,16 @@ prepare() {
# Arch Linux ARM fixes
patch -Np1 -i ../0001-ARM-toolchain-fixes.patch
patch -Np1 -i ../0002-GCC-build-fix-mark-is_trivially_copy_constructible-f.patch
patch -Np1 -i ../0003-Workaround-for-g-7-is_trivially_copy_constructible-f.patch
patch -Np1 -i ../0004-GCC-build-fix-base-Optional-T-requires-the-full-decl.patch
patch -Np1 -i ../0005-GCC-IDB-methods-String-renamed-to-GetString.patch
patch -Np1 -i ../0006-GCC-PlaybackImageProvider-Settings-explicitely-set-c.patch
patch -Np1 -i ../0007-Fix-non-copyable-class-s-optional-move.patch
patch -Np1 -i ../0008-Implement-conditional-copy-move-ctors-assign-operato.patch
patch -Np1 -i ../0009-Clear-the-testing-provider-on-shutdown.patch
patch -Np1 -i ../0010-cleanup-how-ConfigurationPolicyProviders-are-set.patch
patch -Np1 -i ../skia_buildfix.patch
[[ $CARCH == "armv7h" ]] && MAKEFLAGS="-j4" && CFLAGS=`echo $CFLAGS | sed -e 's/vfpv3-d16/neon/'` && CXXFLAGS="$CFLAGS"
@ -116,7 +144,6 @@ prepare() {
patch -Np1 -i ../chromium-clang-r2.patch
patch -Np1 -i ../chromium-math.h-r0.patch
patch -Np1 -i ../chromium-stdint.patch
patch -Np1 -i ../chromium-angle-r0.patch
# Use Python 2
find . -name '*.py' -exec sed -i -r 's|/usr/bin/python$|&2|g' {} +

View file

@ -1,89 +0,0 @@
From 030017a4855c7b6e7f2ff8d9566c146f31eb301b Mon Sep 17 00:00:00 2001
From: Kai Ninomiya <kainino@chromium.org>
Date: Wed, 06 Dec 2017 14:06:53 -0800
Subject: [PATCH] Mark StaticType related functions as constexpr
Fixes compilation on some versions of GCC and probably Clang.
Follow-up to http://crrev.com/c/786317
Bug: angleproject:1432
Change-Id: I3fc3ad0f65492f9543eb27fcdce6ca29a9ad06e5
Reviewed-on: https://chromium-review.googlesource.com/812220
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Commit-Queue: Kai Ninomiya <kainino@chromium.org>
---
diff --git a/third_party/angle/src/compiler/translator/StaticType.h b/third_party/angle/src/compiler/translator/StaticType.h
index e26e5ff..30b391a 100644
--- a/third_party/angle/src/compiler/translator/StaticType.h
+++ b/third_party/angle/src/compiler/translator/StaticType.h
@@ -160,7 +160,7 @@
TPrecision precision,
TQualifier qualifier,
unsigned char secondarySize>
-const TType *GetForVecMatHelper(unsigned char primarySize)
+constexpr const TType *GetForVecMatHelper(unsigned char primarySize)
{
static_assert(basicType == EbtFloat || basicType == EbtInt || basicType == EbtUInt ||
basicType == EbtBool,
@@ -186,7 +186,7 @@
template <TBasicType basicType,
TPrecision precision = EbpUndefined,
TQualifier qualifier = EvqGlobal>
-const TType *GetForVecMat(unsigned char primarySize, unsigned char secondarySize = 1)
+constexpr const TType *GetForVecMat(unsigned char primarySize, unsigned char secondarySize = 1)
{
static_assert(basicType == EbtFloat || basicType == EbtInt || basicType == EbtUInt ||
basicType == EbtBool,
@@ -208,7 +208,7 @@
}
template <TBasicType basicType, TPrecision precision = EbpUndefined>
-const TType *GetForVec(TQualifier qualifier, unsigned char size)
+constexpr const TType *GetForVec(TQualifier qualifier, unsigned char size)
{
switch (qualifier)
{
diff --git a/third_party/angle/src/compiler/translator/SymbolTable.cpp b/third_party/angle/src/compiler/translator/SymbolTable.cpp
index adf1e4e..90d4c15 100644
--- a/third_party/angle/src/compiler/translator/SymbolTable.cpp
+++ b/third_party/angle/src/compiler/translator/SymbolTable.cpp
@@ -236,7 +236,7 @@
pop();
}
-bool IsGenType(const TType *type)
+constexpr bool IsGenType(const TType *type)
{
if (type)
{
@@ -248,7 +248,7 @@
return false;
}
-bool IsVecType(const TType *type)
+constexpr bool IsVecType(const TType *type)
{
if (type)
{
diff --git a/third_party/angle/src/compiler/translator/Types.h b/third_party/angle/src/compiler/translator/Types.h
index 04f46f1..a54d447 100644
--- a/third_party/angle/src/compiler/translator/Types.h
+++ b/third_party/angle/src/compiler/translator/Types.h
@@ -142,13 +142,13 @@
{
}
- TBasicType getBasicType() const { return type; }
+ constexpr TBasicType getBasicType() const { return type; }
void setBasicType(TBasicType t);
TPrecision getPrecision() const { return precision; }
void setPrecision(TPrecision p) { precision = p; }
- TQualifier getQualifier() const { return qualifier; }
+ constexpr TQualifier getQualifier() const { return qualifier; }
void setQualifier(TQualifier q) { qualifier = q; }
bool isInvariant() const { return invariant; }

View file

@ -0,0 +1,22 @@
Index: chromium-browser-65.0.3325.73/third_party/skia/src/jumper/SkJumper_stages.cpp
===================================================================
--- chromium-browser-65.0.3325.73.orig/third_party/skia/src/jumper/SkJumper_stages.cpp
+++ chromium-browser-65.0.3325.73/third_party/skia/src/jumper/SkJumper_stages.cpp
@@ -666,7 +666,7 @@ SI F approx_powf(F x, F y) {
}
SI F from_half(U16 h) {
-#if defined(__aarch64__) && !defined(SK_BUILD_FOR_GOOGLE3) // Temporary workaround for some Google3 builds.
+#if defined(JUMPER_IS_NEON) && defined(__aarch64__) && !defined(SK_BUILD_FOR_GOOGLE3) // Temporary workaround for some Google3 builds.
return vcvt_f32_f16(h);
#elif defined(JUMPER_IS_HSW) || defined(JUMPER_IS_AVX512)
@@ -686,7 +686,7 @@ SI F from_half(U16 h) {
}
SI U16 to_half(F f) {
-#if defined(__aarch64__) && !defined(SK_BUILD_FOR_GOOGLE3) // Temporary workaround for some Google3 builds.
+#if defined(JUMPER_IS_NEON) && defined(__aarch64__) && !defined(SK_BUILD_FOR_GOOGLE3) // Temporary workaround for some Google3 builds.
return vcvt_f16_f32(f);
#elif defined(JUMPER_IS_HSW) || defined(JUMPER_IS_AVX512)