diff --git a/DEPS b/DEPS index 6be27ab..45cc132 100644 --- a/DEPS +++ b/DEPS
@@ -40,7 +40,7 @@ # Three lines of non-changing comments so that # the commit queue can handle CLs rolling Skia # and whatever else without interference from each other. - 'skia_revision': '319ca08ba27d4582d716ce21b290eeb3f997394e', + 'skia_revision': 'a6e431b2c1baa564d2619bdc2a51a3b5bfa7e276', # Three lines of non-changing comments so that # the commit queue can handle CLs rolling V8 # and whatever else without interference from each other.
diff --git a/android_webview/BUILD.gn b/android_webview/BUILD.gn index 6823a70..a4aa096c 100644 --- a/android_webview/BUILD.gn +++ b/android_webview/BUILD.gn
@@ -776,7 +776,6 @@ java_files = [ "java/src/org/chromium/android_webview/crash/CrashReceiverService.java", "java/src/org/chromium/android_webview/crash/MinidumpUploaderImpl.java", - "java/src/org/chromium/android_webview/crash/MinidumpUploader.java", "java/src/org/chromium/android_webview/crash/MinidumpUploadJobService.java", ] deps = [
diff --git a/android_webview/java/src/org/chromium/android_webview/crash/MinidumpUploadJobService.java b/android_webview/java/src/org/chromium/android_webview/crash/MinidumpUploadJobService.java index 88735ea3..82a684b2 100644 --- a/android_webview/java/src/org/chromium/android_webview/crash/MinidumpUploadJobService.java +++ b/android_webview/java/src/org/chromium/android_webview/crash/MinidumpUploadJobService.java
@@ -10,6 +10,7 @@ import org.chromium.android_webview.command_line.CommandLineUtil; import org.chromium.base.ContextUtils; +import org.chromium.components.minidump_uploader.MinidumpUploader; /** * Class that interacts with the Android JobScheduler to upload Minidumps at appropriate times.
diff --git a/android_webview/java/src/org/chromium/android_webview/crash/MinidumpUploaderImpl.java b/android_webview/java/src/org/chromium/android_webview/crash/MinidumpUploaderImpl.java index daa8e1a8..14227a5e 100644 --- a/android_webview/java/src/org/chromium/android_webview/crash/MinidumpUploaderImpl.java +++ b/android_webview/java/src/org/chromium/android_webview/crash/MinidumpUploaderImpl.java
@@ -17,6 +17,7 @@ import org.chromium.base.VisibleForTesting; import org.chromium.components.minidump_uploader.CrashFileManager; import org.chromium.components.minidump_uploader.MinidumpUploadCallable; +import org.chromium.components.minidump_uploader.MinidumpUploader; import org.chromium.components.minidump_uploader.util.CrashReportingPermissionManager; import java.io.File;
diff --git a/android_webview/javatests/src/org/chromium/android_webview/test/crash/MinidumpUploaderTest.java b/android_webview/javatests/src/org/chromium/android_webview/test/crash/MinidumpUploaderTest.java index 8ceb0c3..a12000f9 100644 --- a/android_webview/javatests/src/org/chromium/android_webview/test/crash/MinidumpUploaderTest.java +++ b/android_webview/javatests/src/org/chromium/android_webview/test/crash/MinidumpUploaderTest.java
@@ -13,7 +13,6 @@ import org.chromium.android_webview.PlatformServiceBridge; import org.chromium.android_webview.crash.CrashReceiverService; -import org.chromium.android_webview.crash.MinidumpUploader; import org.chromium.android_webview.crash.MinidumpUploaderImpl; import org.chromium.base.FileUtils; import org.chromium.base.ThreadUtils; @@ -21,6 +20,7 @@ import org.chromium.components.minidump_uploader.CrashTestCase; import org.chromium.components.minidump_uploader.MinidumpUploadCallable; import org.chromium.components.minidump_uploader.MinidumpUploadCallableTest; +import org.chromium.components.minidump_uploader.MinidumpUploader; import org.chromium.components.minidump_uploader.util.CrashReportingPermissionManager; import org.chromium.components.minidump_uploader.util.HttpURLConnectionFactory;
diff --git a/base/metrics/histogram_macros_internal.h b/base/metrics/histogram_macros_internal.h index 2deb928..53e4f11 100644 --- a/base/metrics/histogram_macros_internal.h +++ b/base/metrics/histogram_macros_internal.h
@@ -96,9 +96,17 @@ base::Histogram::FactoryGet(name, min, max, bucket_count, flag)) // This is a helper macro used by other macros and shouldn't be used directly. -// One additional bucket is created in the LinearHistogram for the illegal -// values >= boundary_value so that mistakes in calling the UMA enumeration -// macros can be detected. +// For an enumeration with N items, recording values in the range [0, N - 1], +// this macro creates a linear histogram with N + 1 buckets: +// [0, 1), [1, 2), ..., [N - 1, N), and an overflow bucket [N, infinity). +// Code should never emit to the overflow bucket; only to the other N buckets. +// This allows future versions of Chrome to safely append new entries to the +// enumeration. Otherwise, the histogram would have [N - 1, infinity) as its +// overflow bucket, and so the maximal value (N - 1) would be emitted to this +// overflow bucket. But, if an additional enumerated value were later added, the +// bucket label for the value (N - 1) would change to [N - 1, N), which would +// result in different versions of Chrome using different bucket labels for +// identical data. #define INTERNAL_HISTOGRAM_ENUMERATION_WITH_FLAG(name, sample, boundary, flag) \ do { \ static_assert( \
diff --git a/base/strings/string_util.cc b/base/strings/string_util.cc index cb668ed..49ae943 100644 --- a/base/strings/string_util.cc +++ b/base/strings/string_util.cc
@@ -64,6 +64,21 @@ return elem1.parameter < elem2.parameter; } +// Overloaded function to append one string onto the end of another. Having a +// separate overload for |source| as both string and StringPiece allows for more +// efficient usage from functions templated to work with either type (avoiding a +// redundant call to the BasicStringPiece constructor in both cases). +template <typename string_type> +inline void AppendToString(string_type* target, const string_type& source) { + target->append(source); +} + +template <typename string_type> +inline void AppendToString(string_type* target, + const BasicStringPiece<string_type>& source) { + source.AppendToString(target); +} + // Assuming that a pointer is the size of a "machine word", then // uintptr_t is an integer type that is also a machine word. typedef uintptr_t MachineWord; @@ -853,19 +868,28 @@ return WriteIntoT(str, length_with_null); } -template<typename STR> -static STR JoinStringT(const std::vector<STR>& parts, - BasicStringPiece<STR> sep) { - if (parts.empty()) - return STR(); - - STR result(parts[0]); +// Generic version for all JoinString overloads. |list_type| must be a sequence +// (std::vector or std::initializer_list) of strings/StringPieces (std::string, +// string16, StringPiece or StringPiece16). |string_type| is either std::string +// or string16. +template <typename list_type, typename string_type> +static string_type JoinStringT(const list_type& parts, + BasicStringPiece<string_type> sep) { auto iter = parts.begin(); + // Early-exit to avoid bad access to the first element. + if (iter == parts.end()) + return string_type(); + + // Begin constructing the result from the first element. + string_type result(iter->data(), iter->size()); ++iter; for (; iter != parts.end(); ++iter) { sep.AppendToString(&result); - result += *iter; + // Using the overloaded AppendToString allows this template function to work + // on both strings and StringPieces without creating an intermediate + // StringPiece object. + AppendToString(&result, *iter); } return result; @@ -881,6 +905,26 @@ return JoinStringT(parts, separator); } +std::string JoinString(const std::vector<StringPiece>& parts, + StringPiece separator) { + return JoinStringT(parts, separator); +} + +string16 JoinString(const std::vector<StringPiece16>& parts, + StringPiece16 separator) { + return JoinStringT(parts, separator); +} + +std::string JoinString(std::initializer_list<StringPiece> parts, + StringPiece separator) { + return JoinStringT(parts, separator); +} + +string16 JoinString(std::initializer_list<StringPiece16> parts, + StringPiece16 separator) { + return JoinStringT(parts, separator); +} + template<class FormatStringType, class OutStringType> OutStringType DoReplaceStringPlaceholders( const FormatStringType& format_string,
diff --git a/base/strings/string_util.h b/base/strings/string_util.h index e5cce8f..35b26037 100644 --- a/base/strings/string_util.h +++ b/base/strings/string_util.h
@@ -12,6 +12,7 @@ #include <stddef.h> #include <stdint.h> +#include <initializer_list> #include <string> #include <vector> @@ -429,11 +430,30 @@ BASE_EXPORT wchar_t* WriteInto(std::wstring* str, size_t length_with_null); #endif -// Does the opposite of SplitString(). +// Does the opposite of SplitString()/SplitStringPiece(). Joins a vector or list +// of strings into a single string, inserting |separator| (which may be empty) +// in between all elements. +// +// If possible, callers should build a vector of StringPieces and use the +// StringPiece variant, so that they do not create unnecessary copies of +// strings. For example, instead of using SplitString, modifying the vector, +// then using JoinString, use SplitStringPiece followed by JoinString so that no +// copies of those strings are created until the final join operation. BASE_EXPORT std::string JoinString(const std::vector<std::string>& parts, StringPiece separator); BASE_EXPORT string16 JoinString(const std::vector<string16>& parts, StringPiece16 separator); +BASE_EXPORT std::string JoinString(const std::vector<StringPiece>& parts, + StringPiece separator); +BASE_EXPORT string16 JoinString(const std::vector<StringPiece16>& parts, + StringPiece16 separator); +// Explicit initializer_list overloads are required to break ambiguity when used +// with a literal initializer list (otherwise the compiler would not be able to +// decide between the string and StringPiece overloads). +BASE_EXPORT std::string JoinString(std::initializer_list<StringPiece> parts, + StringPiece separator); +BASE_EXPORT string16 JoinString(std::initializer_list<StringPiece16> parts, + StringPiece16 separator); // Replace $1-$2-$3..$9 in the format string with values from |subst|. // Additionally, any number of consecutive '$' characters is replaced by that
diff --git a/base/strings/string_util_unittest.cc b/base/strings/string_util_unittest.cc index df2226e..6c054f8 100644 --- a/base/strings/string_util_unittest.cc +++ b/base/strings/string_util_unittest.cc
@@ -707,6 +707,92 @@ EXPECT_EQ(ASCIIToUTF16("a|b|c|| "), JoinString(parts, ASCIIToUTF16("|"))); } +TEST(StringUtilTest, JoinStringPiece) { + std::string separator(", "); + std::vector<base::StringPiece> parts; + EXPECT_EQ(base::StringPiece(), JoinString(parts, separator)); + + parts.push_back("a"); + EXPECT_EQ("a", JoinString(parts, separator)); + + parts.push_back("b"); + parts.push_back("c"); + EXPECT_EQ("a, b, c", JoinString(parts, separator)); + + parts.push_back(base::StringPiece()); + EXPECT_EQ("a, b, c, ", JoinString(parts, separator)); + parts.push_back(" "); + EXPECT_EQ("a|b|c|| ", JoinString(parts, "|")); +} + +TEST(StringUtilTest, JoinStringPiece16) { + string16 separator = ASCIIToUTF16(", "); + std::vector<base::StringPiece16> parts; + EXPECT_EQ(base::StringPiece16(), JoinString(parts, separator)); + + const string16 kA = ASCIIToUTF16("a"); + parts.push_back(kA); + EXPECT_EQ(ASCIIToUTF16("a"), JoinString(parts, separator)); + + const string16 kB = ASCIIToUTF16("b"); + parts.push_back(kB); + const string16 kC = ASCIIToUTF16("c"); + parts.push_back(kC); + EXPECT_EQ(ASCIIToUTF16("a, b, c"), JoinString(parts, separator)); + + parts.push_back(base::StringPiece16()); + EXPECT_EQ(ASCIIToUTF16("a, b, c, "), JoinString(parts, separator)); + const string16 kSpace = ASCIIToUTF16(" "); + parts.push_back(kSpace); + EXPECT_EQ(ASCIIToUTF16("a|b|c|| "), JoinString(parts, ASCIIToUTF16("|"))); +} + +TEST(StringUtilTest, JoinStringInitializerList) { + std::string separator(", "); + EXPECT_EQ(base::StringPiece(), JoinString({}, separator)); + + // With const char*s. + EXPECT_EQ("a", JoinString({"a"}, separator)); + EXPECT_EQ("a, b, c", JoinString({"a", "b", "c"}, separator)); + EXPECT_EQ("a, b, c, ", JoinString({"a", "b", "c", ""}, separator)); + EXPECT_EQ("a|b|c|| ", JoinString({"a", "b", "c", "", " "}, "|")); + + // With std::strings. + const std::string kA = "a"; + const std::string kB = "b"; + EXPECT_EQ("a, b", JoinString({kA, kB}, separator)); + + // With StringPieces. + const StringPiece kPieceA = kA; + const StringPiece kPieceB = kB; + EXPECT_EQ("a, b", JoinString({kPieceA, kPieceB}, separator)); +} + +TEST(StringUtilTest, JoinStringInitializerList16) { + string16 separator = ASCIIToUTF16(", "); + EXPECT_EQ(base::StringPiece16(), JoinString({}, separator)); + + // With string16s. + const string16 kA = ASCIIToUTF16("a"); + EXPECT_EQ(ASCIIToUTF16("a"), JoinString({kA}, separator)); + + const string16 kB = ASCIIToUTF16("b"); + const string16 kC = ASCIIToUTF16("c"); + EXPECT_EQ(ASCIIToUTF16("a, b, c"), JoinString({kA, kB, kC}, separator)); + + const string16 kEmpty = ASCIIToUTF16(""); + EXPECT_EQ(ASCIIToUTF16("a, b, c, "), + JoinString({kA, kB, kC, kEmpty}, separator)); + const string16 kSpace = ASCIIToUTF16(" "); + EXPECT_EQ(ASCIIToUTF16("a|b|c|| "), + JoinString({kA, kB, kC, kEmpty, kSpace}, ASCIIToUTF16("|"))); + + // With StringPiece16s. + const StringPiece16 kPieceA = kA; + const StringPiece16 kPieceB = kB; + EXPECT_EQ(ASCIIToUTF16("a, b"), JoinString({kPieceA, kPieceB}, separator)); +} + TEST(StringUtilTest, StartsWith) { EXPECT_TRUE(StartsWith("javascript:url", "javascript", base::CompareCase::SENSITIVE));
diff --git a/chrome/browser/chromeos/arc/arc_service_launcher.cc b/chrome/browser/chromeos/arc/arc_service_launcher.cc index cc6f0e5..1a5ffa2 100644 --- a/chrome/browser/chromeos/arc/arc_service_launcher.cc +++ b/chrome/browser/chromeos/arc/arc_service_launcher.cc
@@ -104,8 +104,6 @@ arc_service_manager_->AddService(base::MakeUnique<ArcCrashCollectorBridge>( arc_bridge_service, arc_service_manager_->blocking_task_runner())); arc_service_manager_->AddService( - base::MakeUnique<ArcFileSystemOperationRunner>(arc_bridge_service)); - arc_service_manager_->AddService( base::MakeUnique<ArcDownloadsWatcherService>(arc_bridge_service)); arc_service_manager_->AddService( base::MakeUnique<ArcEnterpriseReportingService>(arc_bridge_service)); @@ -147,6 +145,9 @@ arc_service_manager_->AddService(base::MakeUnique<ArcBootPhaseMonitorBridge>( arc_service_manager_->arc_bridge_service(), multi_user_util::GetAccountIdFromProfile(profile))); + arc_service_manager_->AddService( + base::MakeUnique<ArcFileSystemOperationRunner>( + arc_service_manager_->arc_bridge_service(), profile)); arc_service_manager_->AddService(base::MakeUnique<ArcNotificationManager>( arc_service_manager_->arc_bridge_service(), multi_user_util::GetAccountIdFromProfile(profile)));
diff --git a/chrome/browser/chromeos/arc/fileapi/arc_file_system_operation_runner.cc b/chrome/browser/chromeos/arc/fileapi/arc_file_system_operation_runner.cc index 4668743..9e914c1 100644 --- a/chrome/browser/chromeos/arc/fileapi/arc_file_system_operation_runner.cc +++ b/chrome/browser/chromeos/arc/fileapi/arc_file_system_operation_runner.cc
@@ -29,17 +29,22 @@ // We can't use base::MakeUnique() here because we are calling a private // constructor. return base::WrapUnique<ArcFileSystemOperationRunner>( - new ArcFileSystemOperationRunner(bridge_service, false)); + new ArcFileSystemOperationRunner(bridge_service, nullptr, false)); } ArcFileSystemOperationRunner::ArcFileSystemOperationRunner( - ArcBridgeService* bridge_service) - : ArcFileSystemOperationRunner(bridge_service, true) {} + ArcBridgeService* bridge_service, + const Profile* profile) + : ArcFileSystemOperationRunner(bridge_service, profile, true) { + DCHECK(profile); +} ArcFileSystemOperationRunner::ArcFileSystemOperationRunner( ArcBridgeService* bridge_service, + const Profile* profile, bool observe_events) : ArcService(bridge_service), + profile_(profile), observe_events_(observe_events), weak_ptr_factory_(this) { DCHECK_CURRENTLY_ON(BrowserThread::UI); @@ -162,11 +167,8 @@ void ArcFileSystemOperationRunner::OnStateChanged() { DCHECK_CURRENTLY_ON(BrowserThread::UI); - // TODO(hidehiko): Revisit the condition, when ARC is running without - // profile. - SetShouldDefer( - IsArcPlayStoreEnabledForProfile(ArcSessionManager::Get()->profile()) && - !arc_bridge_service()->file_system()->has_instance()); + SetShouldDefer(IsArcPlayStoreEnabledForProfile(profile_) && + !arc_bridge_service()->file_system()->has_instance()); } void ArcFileSystemOperationRunner::SetShouldDefer(bool should_defer) {
diff --git a/chrome/browser/chromeos/arc/fileapi/arc_file_system_operation_runner.h b/chrome/browser/chromeos/arc/fileapi/arc_file_system_operation_runner.h index 958e462..65dcf5d 100644 --- a/chrome/browser/chromeos/arc/fileapi/arc_file_system_operation_runner.h +++ b/chrome/browser/chromeos/arc/fileapi/arc_file_system_operation_runner.h
@@ -66,7 +66,8 @@ // The standard constructor. A production instance should be created by // this constructor. - explicit ArcFileSystemOperationRunner(ArcBridgeService* bridge_service); + ArcFileSystemOperationRunner(ArcBridgeService* bridge_service, + const Profile* profile); ~ArcFileSystemOperationRunner() override; // Runs file system operations. See file_system.mojom for documentation. @@ -90,6 +91,7 @@ friend class ArcFileSystemOperationRunnerTest; ArcFileSystemOperationRunner(ArcBridgeService* bridge_service, + const Profile* profile, bool observe_events); // Called whenever ARC states related to |should_defer_| are changed. @@ -100,9 +102,13 @@ // deferring. void SetShouldDefer(bool should_defer); + // Profile this runner is associated with. This can be nullptr in + // unit tests. + const Profile* const profile_; + // Indicates if this instance should register observers to receive events. // Usually true, but set to false in unit tests. - bool observe_events_; + const bool observe_events_; // Set to true if operations should be deferred at this moment. // The default is set to false so that operations are not deferred in
diff --git a/chrome/browser/chromeos/arc/fileapi/arc_file_system_operation_runner_util.cc b/chrome/browser/chromeos/arc/fileapi/arc_file_system_operation_runner_util.cc index b13ff28..8f6fd07 100644 --- a/chrome/browser/chromeos/arc/fileapi/arc_file_system_operation_runner_util.cc +++ b/chrome/browser/chromeos/arc/fileapi/arc_file_system_operation_runner_util.cc
@@ -34,7 +34,10 @@ auto* runner = ArcServiceManager::GetGlobalService<ArcFileSystemOperationRunner>(); if (!runner) { + LOG(ERROR) << "ArcFileSystemOperationRunner unavailable. " + << "File system operations are dropped."; callback.Run(-1); + return; } runner->GetFileSize(url, callback); } @@ -45,6 +48,8 @@ auto* runner = ArcServiceManager::GetGlobalService<ArcFileSystemOperationRunner>(); if (!runner) { + LOG(ERROR) << "ArcFileSystemOperationRunner unavailable. " + << "File system operations are dropped."; callback.Run(mojo::ScopedHandle()); return; } @@ -58,6 +63,8 @@ auto* runner = ArcServiceManager::GetGlobalService<ArcFileSystemOperationRunner>(); if (!runner) { + LOG(ERROR) << "ArcFileSystemOperationRunner unavailable. " + << "File system operations are dropped."; callback.Run(mojom::DocumentPtr()); return; } @@ -71,6 +78,8 @@ auto* runner = ArcServiceManager::GetGlobalService<ArcFileSystemOperationRunner>(); if (!runner) { + LOG(ERROR) << "ArcFileSystemOperationRunner unavailable. " + << "File system operations are dropped."; callback.Run(base::nullopt); return; }
diff --git a/chrome/browser/google/google_update_win_unittest.cc b/chrome/browser/google/google_update_win_unittest.cc index 2ea70c0..944c3955 100644 --- a/chrome/browser/google/google_update_win_unittest.cc +++ b/chrome/browser/google/google_update_win_unittest.cc
@@ -26,6 +26,7 @@ #include "base/win/registry.h" #include "base/win/scoped_comptr.h" #include "chrome/common/chrome_version.h" +#include "chrome/install_static/test/scoped_install_details.h" #include "chrome/installer/util/browser_distribution.h" #include "chrome/installer/util/google_update_settings.h" #include "chrome/installer/util/helper.h" @@ -531,7 +532,8 @@ GoogleUpdateWinTest() : task_runner_(new base::TestSimpleTaskRunner()), task_runner_handle_(task_runner_), - system_level_install_(GetParam()) {} + system_level_install_(GetParam()), + scoped_install_details_(system_level_install_, 0) {} void SetUp() override { ::testing::TestWithParam<bool>::SetUp(); @@ -624,6 +626,7 @@ scoped_refptr<base::TestSimpleTaskRunner> task_runner_; base::ThreadTaskRunnerHandle task_runner_handle_; bool system_level_install_; + install_static::ScopedInstallDetails scoped_install_details_; std::unique_ptr<base::ScopedPathOverride> file_exe_override_; std::unique_ptr<base::ScopedPathOverride> program_files_override_; std::unique_ptr<base::ScopedPathOverride> program_files_x86_override_;
diff --git a/chrome/browser/permissions/permission_manager.cc b/chrome/browser/permissions/permission_manager.cc index 145868d..8758ef7 100644 --- a/chrome/browser/permissions/permission_manager.cc +++ b/chrome/browser/permissions/permission_manager.cc
@@ -81,6 +81,8 @@ // Helper method to convert PermissionType to ContentSettingType. ContentSettingsType PermissionTypeToContentSetting(PermissionType permission) { switch (permission) { + case PermissionType::MIDI: + return CONTENT_SETTINGS_TYPE_MIDI; case PermissionType::MIDI_SYSEX: return CONTENT_SETTINGS_TYPE_MIDI_SYSEX; case PermissionType::PUSH_MESSAGING: @@ -98,9 +100,6 @@ #endif case PermissionType::DURABLE_STORAGE: return CONTENT_SETTINGS_TYPE_DURABLE_STORAGE; - case PermissionType::MIDI: - // This will hit the NOTREACHED below. - break; case PermissionType::AUDIO_CAPTURE: return CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC; case PermissionType::VIDEO_CAPTURE: @@ -121,16 +120,10 @@ // Returns whether the permission has a constant PermissionStatus value (i.e. // always approved or always denied) -// The PermissionTypes for which true is returned should be exactly those which -// return nullptr in PermissionManager::GetPermissionContext since they don't -// have a context. -bool IsConstantPermission(PermissionType type) { - switch (type) { - case PermissionType::MIDI: - return true; - default: - return false; - } +// The ContentSettingsTypes for which true is returned will also return nullptr +// in PermissionManager::GetPermissionContext since they don't have a context. +bool IsConstantPermission(ContentSettingsType type) { + return type == CONTENT_SETTINGS_TYPE_MIDI; } void PermissionRequestResponseCallbackWrapper( @@ -146,10 +139,10 @@ // This function should only be called when IsConstantPermission has returned // true for the PermissionType. blink::mojom::PermissionStatus GetPermissionStatusForConstantPermission( - PermissionType type) { + ContentSettingsType type) { DCHECK(IsConstantPermission(type)); switch (type) { - case PermissionType::MIDI: + case CONTENT_SETTINGS_TYPE_MIDI: return PermissionStatus::GRANTED; default: return PermissionStatus::DENIED; @@ -160,10 +153,11 @@ class PermissionManager::PendingRequest { public: - PendingRequest(content::RenderFrameHost* render_frame_host, - const std::vector<PermissionType> permissions, - const base::Callback< - void(const std::vector<PermissionStatus>&)>& callback) + PendingRequest( + content::RenderFrameHost* render_frame_host, + const std::vector<ContentSettingsType>& permissions, + const base::Callback<void(const std::vector<PermissionStatus>&)>& + callback) : render_process_id_(render_frame_host->GetProcess()->GetID()), render_frame_id_(render_frame_host->GetRoutingID()), callback_(callback), @@ -190,7 +184,7 @@ return callback_; } - std::vector<PermissionType> permissions() const { + std::vector<ContentSettingsType> permissions() const { return permissions_; } @@ -202,13 +196,13 @@ int render_process_id_; int render_frame_id_; const base::Callback<void(const std::vector<PermissionStatus>&)> callback_; - std::vector<PermissionType> permissions_; + std::vector<ContentSettingsType> permissions_; std::vector<PermissionStatus> results_; size_t remaining_results_; }; struct PermissionManager::Subscription { - PermissionType permission; + ContentSettingsType permission; GURL requesting_origin; GURL embedding_origin; base::Callback<void(PermissionStatus)> callback; @@ -270,50 +264,19 @@ const GURL& requesting_origin, bool user_gesture, const base::Callback<void(PermissionStatus)>& callback) { - // TODO(timloh): We should be operating on ContentSettingsType instead of - // converting these back to PermissionType. - PermissionType permission_type; - bool success = PermissionUtil::GetPermissionType(content_settings_type, - &permission_type); - DCHECK(success); return RequestPermissions( - std::vector<PermissionType>(1, permission_type), render_frame_host, - requesting_origin, user_gesture, - base::Bind(&PermissionRequestResponseCallbackWrapper, callback)); -} - -PermissionStatus PermissionManager::GetPermissionStatus( - ContentSettingsType permission, - const GURL& requesting_origin, - const GURL& embedding_origin) { - PermissionContextBase* context = GetPermissionContext(permission); - return ContentSettingToPermissionStatus( - context->GetPermissionStatus(requesting_origin.GetOrigin(), - embedding_origin.GetOrigin()) - .content_setting); -} - -int PermissionManager::RequestPermission( - PermissionType permission, - content::RenderFrameHost* render_frame_host, - const GURL& requesting_origin, - bool user_gesture, - const base::Callback<void(PermissionStatus)>& callback) { - return RequestPermissions( - std::vector<PermissionType>(1, permission), - render_frame_host, - requesting_origin, - user_gesture, + std::vector<ContentSettingsType>(1, content_settings_type), + render_frame_host, requesting_origin, user_gesture, base::Bind(&PermissionRequestResponseCallbackWrapper, callback)); } int PermissionManager::RequestPermissions( - const std::vector<PermissionType>& permissions, + const std::vector<ContentSettingsType>& permissions, content::RenderFrameHost* render_frame_host, const GURL& requesting_origin, bool user_gesture, - const base::Callback<void( - const std::vector<PermissionStatus>&)>& callback) { + const base::Callback<void(const std::vector<PermissionStatus>&)>& + callback) { DCHECK_CURRENTLY_ON(content::BrowserThread::UI); if (permissions.empty()) { callback.Run(std::vector<PermissionStatus>()); @@ -330,10 +293,9 @@ const PermissionRequestID request(render_frame_host, request_id); for (size_t i = 0; i < permissions.size(); ++i) { - const PermissionType permission = permissions[i]; + const ContentSettingsType permission = permissions[i]; - if (IsConstantPermission(permission) || - !GetPermissionContext(PermissionTypeToContentSetting(permission))) { + if (IsConstantPermission(permission) || !GetPermissionContext(permission)) { // Track permission request usages even for constant permissions. PermissionUmaUtil::PermissionRequested(permission, requesting_origin, embedding_origin, profile_); @@ -342,8 +304,7 @@ continue; } - PermissionContextBase* context = - GetPermissionContext(PermissionTypeToContentSetting(permission)); + PermissionContextBase* context = GetPermissionContext(permission); context->RequestPermission( web_contents, request, requesting_origin, user_gesture, base::Bind(&ContentSettingToPermissionStatusCallbackWrapper, @@ -358,6 +319,49 @@ return request_id; } +PermissionStatus PermissionManager::GetPermissionStatus( + ContentSettingsType permission, + const GURL& requesting_origin, + const GURL& embedding_origin) { + if (IsConstantPermission(permission)) + return GetPermissionStatusForConstantPermission(permission); + PermissionContextBase* context = GetPermissionContext(permission); + return ContentSettingToPermissionStatus( + context + ->GetPermissionStatus(requesting_origin.GetOrigin(), + embedding_origin.GetOrigin()) + .content_setting); +} + +int PermissionManager::RequestPermission( + PermissionType permission, + content::RenderFrameHost* render_frame_host, + const GURL& requesting_origin, + bool user_gesture, + const base::Callback<void(PermissionStatus)>& callback) { + ContentSettingsType content_settings_type = + PermissionTypeToContentSetting(permission); + return RequestPermissions( + std::vector<ContentSettingsType>(1, content_settings_type), + render_frame_host, requesting_origin, user_gesture, + base::Bind(&PermissionRequestResponseCallbackWrapper, callback)); +} + +int PermissionManager::RequestPermissions( + const std::vector<PermissionType>& permissions, + content::RenderFrameHost* render_frame_host, + const GURL& requesting_origin, + bool user_gesture, + const base::Callback<void(const std::vector<PermissionStatus>&)>& + callback) { + std::vector<ContentSettingsType> content_settings_types; + std::transform(permissions.begin(), permissions.end(), + back_inserter(content_settings_types), + PermissionTypeToContentSetting); + return RequestPermissions(content_settings_types, render_frame_host, + requesting_origin, user_gesture, callback); +} + PermissionContextBase* PermissionManager::GetPermissionContext( ContentSettingsType type) { const auto& it = permission_contexts_.find(type); @@ -391,9 +395,8 @@ const PermissionRequestID request(pending_request->render_process_id(), pending_request->render_frame_id(), request_id); - for (PermissionType permission : pending_request->permissions()) { - PermissionContextBase* context = - GetPermissionContext(PermissionTypeToContentSetting(permission)); + for (ContentSettingsType permission : pending_request->permissions()) { + PermissionContextBase* context = GetPermissionContext(permission); if (!context) continue; context->CancelPermissionRequest(web_contents, request); @@ -419,8 +422,6 @@ const GURL& requesting_origin, const GURL& embedding_origin) { DCHECK_CURRENTLY_ON(content::BrowserThread::UI); - if (IsConstantPermission(permission)) - return GetPermissionStatusForConstantPermission(permission); return GetPermissionStatus(PermissionTypeToContentSetting(permission), requesting_origin, embedding_origin); } @@ -434,14 +435,15 @@ if (subscriptions_.IsEmpty()) HostContentSettingsMapFactory::GetForProfile(profile_)->AddObserver(this); + ContentSettingsType content_type = PermissionTypeToContentSetting(permission); auto subscription = base::MakeUnique<Subscription>(); - subscription->permission = permission; + subscription->permission = content_type; subscription->requesting_origin = requesting_origin; subscription->embedding_origin = embedding_origin; subscription->callback = callback; subscription->current_value = - GetPermissionStatus(permission, requesting_origin, embedding_origin); + GetPermissionStatus(content_type, requesting_origin, embedding_origin); return subscriptions_.Add(std::move(subscription)); } @@ -475,10 +477,8 @@ Subscription* subscription = iter.GetCurrentValue(); if (IsConstantPermission(subscription->permission)) continue; - if (PermissionTypeToContentSetting(subscription->permission) != - content_type) { + if (subscription->permission != content_type) continue; - } if (primary_pattern.IsValid() && !primary_pattern.Matches(subscription->requesting_origin))
diff --git a/chrome/browser/permissions/permission_manager.h b/chrome/browser/permissions/permission_manager.h index 31e1a22..b8be948 100644 --- a/chrome/browser/permissions/permission_manager.h +++ b/chrome/browser/permissions/permission_manager.h
@@ -37,12 +37,21 @@ // ContentSettingsType enum. The methods which take PermissionType values // are for the content::PermissionManager overrides and shouldn't be used // from chrome/. + int RequestPermission( ContentSettingsType permission, content::RenderFrameHost* render_frame_host, const GURL& requesting_origin, bool user_gesture, const base::Callback<void(blink::mojom::PermissionStatus)>& callback); + int RequestPermissions( + const std::vector<ContentSettingsType>& permissions, + content::RenderFrameHost* render_frame_host, + const GURL& requesting_origin, + bool user_gesture, + const base::Callback< + void(const std::vector<blink::mojom::PermissionStatus>&)>& callback); + blink::mojom::PermissionStatus GetPermissionStatus( ContentSettingsType permission, const GURL& requesting_origin,
diff --git a/chrome/browser/permissions/permission_uma_util.cc b/chrome/browser/permissions/permission_uma_util.cc index 1f6f48d..94ec8298 100644 --- a/chrome/browser/permissions/permission_uma_util.cc +++ b/chrome/browser/permissions/permission_uma_util.cc
@@ -94,14 +94,14 @@ permission_str.c_str(), action_str.c_str()); } -void RecordPermissionRequest(PermissionType permission, +void RecordPermissionRequest(ContentSettingsType content_type, const GURL& requesting_origin, const GURL& embedding_origin, Profile* profile) { rappor::RapporServiceImpl* rappor_service = g_browser_process->rappor_service(); if (rappor_service) { - if (permission == PermissionType::GEOLOCATION) { + if (content_type == CONTENT_SETTINGS_TYPE_GEOLOCATION) { // TODO(dominickn): remove this deprecated metric - crbug.com/605836. rappor::SampleDomainAndRegistryFromGURL( rappor_service, "ContentSettings.PermissionRequested.Geolocation.Url", @@ -110,7 +110,7 @@ "ContentSettings.PermissionRequested.Geolocation.Url2", rappor::LOW_FREQUENCY_ETLD_PLUS_ONE_RAPPOR_TYPE, rappor::GetDomainAndRegistrySampleFromGURL(requesting_origin)); - } else if (permission == PermissionType::NOTIFICATIONS) { + } else if (content_type == CONTENT_SETTINGS_TYPE_NOTIFICATIONS) { // TODO(dominickn): remove this deprecated metric - crbug.com/605836. rappor::SampleDomainAndRegistryFromGURL( rappor_service, @@ -120,8 +120,8 @@ "ContentSettings.PermissionRequested.Notifications.Url2", rappor::LOW_FREQUENCY_ETLD_PLUS_ONE_RAPPOR_TYPE, rappor::GetDomainAndRegistrySampleFromGURL(requesting_origin)); - } else if (permission == PermissionType::MIDI || - permission == PermissionType::MIDI_SYSEX) { + } else if (content_type == CONTENT_SETTINGS_TYPE_MIDI || + content_type == CONTENT_SETTINGS_TYPE_MIDI_SYSEX) { // TODO(dominickn): remove this deprecated metric - crbug.com/605836. rappor::SampleDomainAndRegistryFromGURL( rappor_service, "ContentSettings.PermissionRequested.Midi.Url", @@ -130,7 +130,8 @@ "ContentSettings.PermissionRequested.Midi.Url2", rappor::LOW_FREQUENCY_ETLD_PLUS_ONE_RAPPOR_TYPE, rappor::GetDomainAndRegistrySampleFromGURL(requesting_origin)); - } else if (permission == PermissionType::PROTECTED_MEDIA_IDENTIFIER) { + } else if (content_type == + CONTENT_SETTINGS_TYPE_PROTECTED_MEDIA_IDENTIFIER) { rappor_service->RecordSampleString( "ContentSettings.PermissionRequested.ProtectedMedia.Url2", rappor::LOW_FREQUENCY_ETLD_PLUS_ONE_RAPPOR_TYPE, @@ -138,6 +139,10 @@ } } + PermissionType permission; + bool success = PermissionUtil::GetPermissionType(content_type, &permission); + DCHECK(success); + bool secure_origin = content::IsOriginSecure(requesting_origin); UMA_HISTOGRAM_ENUMERATION( "ContentSettings.PermissionRequested", @@ -161,16 +166,16 @@ // ratio could be somewhat biased by repeated requests coming from a // single frame, but we expect this to be insignificant. if (requesting_origin.GetOrigin() != embedding_origin.GetOrigin()) { - content::PermissionManager* manager = profile->GetPermissionManager(); + PermissionManager* manager = PermissionManager::Get(profile); if (!manager) return; blink::mojom::PermissionStatus embedding_permission_status = - manager->GetPermissionStatus(permission, embedding_origin, + manager->GetPermissionStatus(content_type, embedding_origin, embedding_origin); base::HistogramBase* histogram = base::LinearHistogram::FactoryGet( "Permissions.Requested.CrossOrigin_" + - PermissionUtil::GetPermissionString(permission), + PermissionUtil::GetPermissionString(content_type), 1, static_cast<int>(blink::mojom::PermissionStatus::LAST), static_cast<int>(blink::mojom::PermissionStatus::LAST) + 1, base::HistogramBase::kUmaTargetedHistogramFlag); @@ -259,22 +264,11 @@ // Make sure you update histograms.xml permission histogram_suffix if you // add new permission -void PermissionUmaUtil::PermissionRequested(PermissionType permission, - const GURL& requesting_origin, - const GURL& embedding_origin, - Profile* profile) { - RecordPermissionRequest(permission, requesting_origin, embedding_origin, - profile); -} - void PermissionUmaUtil::PermissionRequested(ContentSettingsType content_type, const GURL& requesting_origin, const GURL& embedding_origin, Profile* profile) { - PermissionType permission; - bool success = PermissionUtil::GetPermissionType(content_type, &permission); - DCHECK(success); - RecordPermissionRequest(permission, requesting_origin, embedding_origin, + RecordPermissionRequest(content_type, requesting_origin, embedding_origin, profile); }
diff --git a/chrome/browser/permissions/permission_uma_util.h b/chrome/browser/permissions/permission_uma_util.h index f1efe4c..6a4de411 100644 --- a/chrome/browser/permissions/permission_uma_util.h +++ b/chrome/browser/permissions/permission_uma_util.h
@@ -106,12 +106,6 @@ static const char kPermissionsPromptIgnoredPriorDismissCountPrefix[]; static const char kPermissionsPromptIgnoredPriorIgnoreCountPrefix[]; - // TODO(timloh): Remove this content::PermissionType overload when we add MIDI - // to ContentSettingsType. - static void PermissionRequested(content::PermissionType permission, - const GURL& requesting_origin, - const GURL& embedding_origin, - Profile* profile); static void PermissionRequested(ContentSettingsType permission, const GURL& requesting_origin, const GURL& embedding_origin,
diff --git a/chrome/browser/permissions/permission_util.cc b/chrome/browser/permissions/permission_util.cc index a6a0e84..bae51e0 100644 --- a/chrome/browser/permissions/permission_util.cc +++ b/chrome/browser/permissions/permission_util.cc
@@ -15,41 +15,33 @@ using content::PermissionType; -std::string PermissionUtil::GetPermissionString( - ContentSettingsType content_type) { - PermissionType permission_type; - bool success = PermissionUtil::GetPermissionType( - content_type, &permission_type); - DCHECK(success); - return GetPermissionString(permission_type); -} - // The returned strings must match the RAPPOR metrics in rappor.xml, // and any Field Trial configs for the Permissions kill switch e.g. // Permissions.Action.Geolocation etc.. -std::string PermissionUtil::GetPermissionString(PermissionType permission) { - switch (permission) { - case PermissionType::GEOLOCATION: +std::string PermissionUtil::GetPermissionString( + ContentSettingsType content_type) { + switch (content_type) { + case CONTENT_SETTINGS_TYPE_GEOLOCATION: return "Geolocation"; - case PermissionType::NOTIFICATIONS: + case CONTENT_SETTINGS_TYPE_NOTIFICATIONS: return "Notifications"; - case PermissionType::MIDI_SYSEX: + case CONTENT_SETTINGS_TYPE_MIDI_SYSEX: return "MidiSysEx"; - case PermissionType::PUSH_MESSAGING: + case CONTENT_SETTINGS_TYPE_PUSH_MESSAGING: return "PushMessaging"; - case PermissionType::DURABLE_STORAGE: + case CONTENT_SETTINGS_TYPE_DURABLE_STORAGE: return "DurableStorage"; - case PermissionType::PROTECTED_MEDIA_IDENTIFIER: + case CONTENT_SETTINGS_TYPE_PROTECTED_MEDIA_IDENTIFIER: return "ProtectedMediaIdentifier"; - case PermissionType::AUDIO_CAPTURE: + case CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC: return "AudioCapture"; - case PermissionType::VIDEO_CAPTURE: + case CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA: return "VideoCapture"; - case PermissionType::MIDI: + case CONTENT_SETTINGS_TYPE_MIDI: return "Midi"; - case PermissionType::BACKGROUND_SYNC: + case CONTENT_SETTINGS_TYPE_BACKGROUND_SYNC: return "BackgroundSync"; - case PermissionType::FLASH: + case CONTENT_SETTINGS_TYPE_PLUGINS: return "Flash"; default: break;
diff --git a/chrome/browser/permissions/permission_util.h b/chrome/browser/permissions/permission_util.h index b8faf941..097203a 100644 --- a/chrome/browser/permissions/permission_util.h +++ b/chrome/browser/permissions/permission_util.h
@@ -40,9 +40,6 @@ public: // Returns the permission string for the given permission. static std::string GetPermissionString(ContentSettingsType); - // TODO(timloh): Remove this content::PermissionType overload when we add MIDI - // to ContentSettingsType. - static std::string GetPermissionString(content::PermissionType); // Return the stringified version of the ContentSettingsType enum that // Safe Browsing uses for the API Blacklist.
diff --git a/chrome/browser/resources/chromeos/login/oobe_a11y_option.html b/chrome/browser/resources/chromeos/login/oobe_a11y_option.html index 4f7b080..fcf4d1c 100644 --- a/chrome/browser/resources/chromeos/login/oobe_a11y_option.html +++ b/chrome/browser/resources/chromeos/login/oobe_a11y_option.html
@@ -17,11 +17,14 @@ left: 4px; width: 24px; }; + --paper-toggle-button-checked-bar-color: var(--google-blue-500); --paper-toggle-button-checked-bar: var(--oobe-toggle-bar-size); --paper-toggle-button-checked-button: { @apply(--oobe-toggle-button-size); transform: translate(14px, 0); }; + --paper-toggle-button-checked-button-color: var(--google-blue-500); + --paper-toggle-button-checked-ink-color: var(--google-blue-500); --paper-toggle-button-label-spacing: 0; --paper-toggle-button-unchecked-bar: var(--oobe-toggle-bar-size); --paper-toggle-button-unchecked-button:
diff --git a/chrome/browser/resources/chromeos/login/oobe_welcome.css b/chrome/browser/resources/chromeos/login/oobe_welcome.css index 03484304..6f01d3f 100644 --- a/chrome/browser/resources/chromeos/login/oobe_welcome.css +++ b/chrome/browser/resources/chromeos/login/oobe_welcome.css
@@ -2,15 +2,19 @@ * Use of this source code is governed by a BSD-style license that can be * found in the LICENSE file. */ +:root { + --oobe-dialog-list-item-border: 1px solid rgba(0, 0, 0, 0.06); +} + /************* Language Screen **************/ #languageScreen .language-selection-entry { - border-top: 1px solid lightgrey; + border-top: var(--oobe-dialog-list-item-border); height: 44px; padding: 0 10px; } #languageScreen .language-selection-entry:last-of-type { - border-bottom: 1px solid lightgrey; + border-bottom: var(--oobe-dialog-list-item-border); } #languageScreen .language-selection-title { @@ -24,12 +28,12 @@ /************* Accessibility Screen **************/ #accessibilityScreen oobe-a11y-option { - border-top: 1px solid lightgrey; + border-top: var(--oobe-dialog-list-item-border); min-height: 56px; } #accessibilityScreen oobe-a11y-option:last-of-type { - border-bottom: 1px solid rgba(0, 0, 0, 0.14); + border-bottom: var(--oobe-dialog-list-item-border); } #accessibilityScreen .bottom-buttons { @@ -47,13 +51,13 @@ /************* Timezone Screen **************/ #timezoneScreen .timezone-selection-entry { - border-top: 1px solid lightgrey; + border-top: var(--oobe-dialog-list-item-border); height: 44px; padding: 0 20px; } #timezoneScreen .timezone-selection-entry:last-of-type { - border-bottom: 1px solid lightgrey; + border-bottom: var(--oobe-dialog-list-item-border); } #timezoneScreen .timezone-selection-title {
diff --git a/chrome/browser/tracing/crash_service_uploader.cc b/chrome/browser/tracing/crash_service_uploader.cc index 95c42f1..0116b649 100644 --- a/chrome/browser/tracing/crash_service_uploader.cc +++ b/chrome/browser/tracing/crash_service_uploader.cc
@@ -21,10 +21,12 @@ #include "components/version_info/version_info.h" #include "content/public/browser/browser_thread.h" #include "content/public/common/content_switches.h" +#include "net/base/load_flags.h" #include "net/base/mime_util.h" #include "net/base/network_delegate.h" #include "net/proxy/proxy_config.h" #include "net/proxy/proxy_config_service.h" +#include "net/traffic_annotation/network_traffic_annotation.h" #include "net/url_request/url_fetcher.h" #include "net/url_request/url_request_context.h" #include "net/url_request/url_request_context_builder.h" @@ -308,11 +310,50 @@ content_type.append("; boundary="); content_type.append(kMultipartBoundary); - url_fetcher_ = - net::URLFetcher::Create(GURL(upload_url), net::URLFetcher::POST, this); + // Create traffic annotation tag. + net::NetworkTrafficAnnotationTag traffic_annotation = + net::DefineNetworkTrafficAnnotation("background_performance_tracer", R"( + semantics { + sender: "Background Performance Traces" + description: + "Under certain conditions, Google Chrome will send anonymized " + "performance timeline data to Google for the purposes of improving " + "Chrome performance. We can set up a percentage of the population " + "to send back trace reports when a certain UMA histogram bucket is " + "incremented, for example, \"For 1% of the Beta population, send " + "us a trace if it ever takes more than 1 seconds for the Omnibox " + "to respond to a typed character\". The possible types of triggers " + "right now are UMA histograms, and manually triggered events from " + "code (think of them like asserts, that'll cause a report to be " + "sent if enabled for that population)." + trigger: + "Google-controlled triggering conditions, usually when a bad " + "performance situation occurs." + data: "An anonymized Chrome trace (see about://tracing)." + destination: GOOGLE_OWNED_SERVICE + } + policy { + cookies_allowed: false + setting: + "You can enable or disable this feature via 'Automatically send " + "usage statistics and crash reports to Google' in Chrome's " + "settings under Advanced, Privacy. This feature is enabled by " + "default." + policy { + MetricsReportingEnabled { + policy_options {mode: MANDATORY} + value: false + } + } + })"); + + url_fetcher_ = net::URLFetcher::Create( + GURL(upload_url), net::URLFetcher::POST, this, traffic_annotation); data_use_measurement::DataUseUserData::AttachToFetcher( url_fetcher_.get(), data_use_measurement::DataUseUserData::TRACING_UPLOADER); + url_fetcher_->SetLoadFlags(net::LOAD_DO_NOT_SAVE_COOKIES | + net::LOAD_DO_NOT_SEND_COOKIES); url_fetcher_->SetRequestContext(request_context_); url_fetcher_->SetUploadData(content_type, post_data); url_fetcher_->Start();
diff --git a/chrome/browser/ui/BUILD.gn b/chrome/browser/ui/BUILD.gn index 09f800b5..9d9951f 100644 --- a/chrome/browser/ui/BUILD.gn +++ b/chrome/browser/ui/BUILD.gn
@@ -235,8 +235,6 @@ "webui/chromeos/login/arc_kiosk_splash_screen_handler.h", "webui/chromeos/login/arc_terms_of_service_screen_handler.cc", "webui/chromeos/login/arc_terms_of_service_screen_handler.h", - "webui/chromeos/login/authenticated_user_email_retriever.cc", - "webui/chromeos/login/authenticated_user_email_retriever.h", "webui/chromeos/login/auto_enrollment_check_screen_handler.cc", "webui/chromeos/login/auto_enrollment_check_screen_handler.h", "webui/chromeos/login/base_screen_handler.cc",
diff --git a/chrome/browser/ui/webui/chromeos/login/authenticated_user_email_retriever.cc b/chrome/browser/ui/webui/chromeos/login/authenticated_user_email_retriever.cc deleted file mode 100644 index 91fe87e..0000000 --- a/chrome/browser/ui/webui/chromeos/login/authenticated_user_email_retriever.cc +++ /dev/null
@@ -1,45 +0,0 @@ -// Copyright 2014 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "chrome/browser/ui/webui/chromeos/login/authenticated_user_email_retriever.h" - -#include <utility> -#include <vector> - -#include "google_apis/gaia/gaia_auth_util.h" -#include "google_apis/gaia/gaia_constants.h" -#include "google_apis/gaia/google_service_auth_error.h" -#include "net/url_request/url_request_context_getter.h" - -namespace chromeos { - -AuthenticatedUserEmailRetriever::AuthenticatedUserEmailRetriever( - const AuthenticatedUserEmailCallback& callback, - scoped_refptr<net::URLRequestContextGetter> url_request_context_getter) - : callback_(callback), - gaia_auth_fetcher_(this, - GaiaConstants::kChromeSource, - url_request_context_getter.get()) { - gaia_auth_fetcher_.StartListAccounts(); -} - -AuthenticatedUserEmailRetriever::~AuthenticatedUserEmailRetriever() { -} - -void AuthenticatedUserEmailRetriever::OnListAccountsSuccess( - const std::string& data) { - std::vector<gaia::ListedAccount> accounts; - gaia::ParseListAccountsData(data, &accounts, nullptr); - if (accounts.size() != 1) - callback_.Run(std::string()); - else - callback_.Run(accounts.front().email); -} - -void AuthenticatedUserEmailRetriever::OnListAccountsFailure( - const GoogleServiceAuthError& error) { - callback_.Run(std::string()); -} - -} // namespace chromeos
diff --git a/chrome/browser/ui/webui/chromeos/login/authenticated_user_email_retriever.h b/chrome/browser/ui/webui/chromeos/login/authenticated_user_email_retriever.h deleted file mode 100644 index 910f496..0000000 --- a/chrome/browser/ui/webui/chromeos/login/authenticated_user_email_retriever.h +++ /dev/null
@@ -1,53 +0,0 @@ -// Copyright 2014 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef CHROME_BROWSER_UI_WEBUI_CHROMEOS_LOGIN_AUTHENTICATED_USER_EMAIL_RETRIEVER_H_ -#define CHROME_BROWSER_UI_WEBUI_CHROMEOS_LOGIN_AUTHENTICATED_USER_EMAIL_RETRIEVER_H_ - -#include <string> - -#include "base/callback.h" -#include "base/compiler_specific.h" -#include "base/macros.h" -#include "base/memory/ref_counted.h" -#include "google_apis/gaia/gaia_auth_consumer.h" -#include "google_apis/gaia/gaia_auth_fetcher.h" - -class GaiaAuthFetcher; - -namespace net { -class URLRequestContextGetter; -} - -namespace chromeos { - -// Helper class that retrieves the authenticated user's e-mail address. -class AuthenticatedUserEmailRetriever : public GaiaAuthConsumer { - public: - typedef base::Callback<void(const std::string&)> - AuthenticatedUserEmailCallback; - - // Retrieves the authenticated user's e-mail address from GAIA and passes it - // to |callback|. Requires that |url_request_context_getter| contain the GAIA - // cookies for exactly one user. If the e-mail address cannot be retrieved, an - // empty string is passed to the |callback|. - AuthenticatedUserEmailRetriever( - const AuthenticatedUserEmailCallback& callback, - scoped_refptr<net::URLRequestContextGetter> url_request_context_getter); - ~AuthenticatedUserEmailRetriever() override; - - // GaiaAuthConsumer: - void OnListAccountsSuccess(const std::string& data) override; - void OnListAccountsFailure(const GoogleServiceAuthError& error) override; - - private: - const AuthenticatedUserEmailCallback callback_; - GaiaAuthFetcher gaia_auth_fetcher_; - - DISALLOW_COPY_AND_ASSIGN(AuthenticatedUserEmailRetriever); -}; - -} // namespace chromeos - -#endif // CHROME_BROWSER_UI_WEBUI_CHROMEOS_LOGIN_AUTHENTICATED_USER_EMAIL_RETRIEVER_H_
diff --git a/chrome/browser/webshare/share_service_impl.cc b/chrome/browser/webshare/share_service_impl.cc index 2ae7404a..9ce3e71 100644 --- a/chrome/browser/webshare/share_service_impl.cc +++ b/chrome/browser/webshare/share_service_impl.cc
@@ -29,23 +29,6 @@ return base::IsAsciiAlpha(c) || base::IsAsciiDigit(c) || c == '-' || c == '_'; } -// Joins a std::vector<base::StringPiece> into a single std::string. -// TODO(constantina): Implement a base::JoinString() that takes StringPieces. -// i.e. move this to base/strings/string_util.h, and thoroughly test. -std::string JoinString(const std::vector<base::StringPiece>& pieces) { - size_t total_size = 0; - for (const auto& piece : pieces) { - total_size += piece.size(); - } - std::string joined_pieces; - joined_pieces.reserve(total_size); - - for (const auto& piece : pieces) { - piece.AppendToString(&joined_pieces); - } - return joined_pieces; -} - } // namespace ShareServiceImpl::ShareServiceImpl() : weak_factory_(this) {} @@ -115,7 +98,7 @@ split_template.push_back(url_template.substr( start_index_to_copy, url_template.size() - start_index_to_copy)); - *url_template_filled = JoinString(split_template); + *url_template_filled = base::JoinString(split_template, base::StringPiece()); return true; }
diff --git a/chrome/install_static/install_util_unittest.cc b/chrome/install_static/install_util_unittest.cc index 486b0fe..8e8f7b9ff 100644 --- a/chrome/install_static/install_util_unittest.cc +++ b/chrome/install_static/install_util_unittest.cc
@@ -9,6 +9,7 @@ #include "base/memory/ptr_util.h" #include "base/test/test_reg_util_win.h" #include "chrome/install_static/install_details.h" +#include "chrome/install_static/install_modes.h" #include "chrome/install_static/test/scoped_install_details.h" #include "chrome_elf/nt_registry/nt_registry.h" #include "testing/gmock/include/gmock/gmock.h"
diff --git a/chrome/install_static/test/scoped_install_details.h b/chrome/install_static/test/scoped_install_details.h index 0fa0c28..1fc54cd 100644 --- a/chrome/install_static/test/scoped_install_details.h +++ b/chrome/install_static/test/scoped_install_details.h
@@ -20,6 +20,7 @@ // Installs an InstallDetails instance that will report the install as being // at |system_level| and of mode |install_mode_index| (an InstallConstantIndex // value) of the current brand; see ../install_modes.h for details. + // TODO(grt): replace bool and int with more obvious types (e.g., enum). explicit ScopedInstallDetails(bool system_level = false, int install_mode_index = 0); ~ScopedInstallDetails();
diff --git a/chrome/installer/util/BUILD.gn b/chrome/installer/util/BUILD.gn index 7602dcf8..c09fc936 100644 --- a/chrome/installer/util/BUILD.gn +++ b/chrome/installer/util/BUILD.gn
@@ -332,6 +332,7 @@ "//base/test:test_support", "//chrome:other_version", "//chrome/install_static:install_static_util", + "//chrome/install_static/test:test_support", "//chrome/installer/setup:lib", "//chrome/installer/test:alternate_version_generator_lib", "//components/variations",
diff --git a/chrome/installer/util/DEPS b/chrome/installer/util/DEPS index e301ba2..6a5d974 100644 --- a/chrome/installer/util/DEPS +++ b/chrome/installer/util/DEPS
@@ -3,7 +3,7 @@ # "-chrome" rule in a parent dir. "+chrome/grit/chromium_strings.h", - "+chrome/install_static/install_util.h", + "+chrome/install_static", "+components/base32", "+components/metrics",
diff --git a/chrome/installer/util/beacons_unittest.cc b/chrome/installer/util/beacons_unittest.cc index 1ca2a8ad..185b3e15 100644 --- a/chrome/installer/util/beacons_unittest.cc +++ b/chrome/installer/util/beacons_unittest.cc
@@ -4,15 +4,20 @@ #include "chrome/installer/util/beacons.h" -#include "base/base_paths.h" +#include <memory> +#include <tuple> + +#include "base/memory/ptr_util.h" #include "base/memory/scoped_vector.h" #include "base/path_service.h" -#include "base/test/scoped_path_override.h" #include "base/test/test_reg_util_win.h" #include "base/test/test_timeouts.h" #include "base/threading/platform_thread.h" #include "base/win/registry.h" #include "base/win/win_util.h" +#include "chrome/install_static/install_details.h" +#include "chrome/install_static/install_modes.h" +#include "chrome/install_static/test/scoped_install_details.h" #include "chrome/installer/util/browser_distribution.h" #include "chrome/installer/util/install_util.h" #include "chrome/installer/util/test_app_registration_data.h" @@ -139,46 +144,27 @@ BeaconScope::PER_INSTALL), Bool())); -enum class DistributionVariant { - SYSTEM_LEVEL, - USER_LEVEL, - SXS, -}; - class DefaultBrowserBeaconTest - : public ::testing::TestWithParam<DistributionVariant> { + : public ::testing::TestWithParam< + std::tuple<install_static::InstallConstantIndex, const char*>> { protected: - using Super = ::testing::TestWithParam<DistributionVariant>; - - DefaultBrowserBeaconTest() - : system_install_(GetParam() == DistributionVariant::SYSTEM_LEVEL), - chrome_sxs_(GetParam() == DistributionVariant::SXS), - chrome_exe_(GetChromePathForParams()), - distribution_(nullptr) {} + using Super = ::testing::TestWithParam< + std::tuple<install_static::InstallConstantIndex, const char*>>; void SetUp() override { Super::SetUp(); - // Override FILE_EXE so that various InstallUtil functions will consider - // this to be a user/system Chrome or Chrome SxS. - path_overrides_.push_back(new base::ScopedPathOverride( - base::FILE_EXE, chrome_exe_, true /* is_absolute */, - false /* !create */)); + install_static::InstallConstantIndex mode_index; + const char* level; + std::tie(mode_index, level) = GetParam(); - // Override these paths with their own values so that they can be found - // after the registry override manager is in place. Getting them would - // otherwise fail since the underlying calls to the OS need to see the real - // contents of the registry. - static const int kPathKeys[] = { - base::DIR_PROGRAM_FILES, - base::DIR_PROGRAM_FILESX86, - base::DIR_LOCAL_APP_DATA, - }; - for (int key : kPathKeys) { - base::FilePath temp; - PathService::Get(key, &temp); - path_overrides_.push_back(new base::ScopedPathOverride(key, temp)); - } + system_install_ = (std::string(level) != "user"); + + // Configure InstallDetails for the test. + scoped_install_details_ = + base::MakeUnique<install_static::ScopedInstallDetails>(system_install_, + mode_index); + chrome_exe_ = GetChromePath(); // Override the registry so that tests can freely push state to it. ASSERT_NO_FATAL_FAILURE( @@ -192,13 +178,12 @@ distribution_ = BrowserDistribution::GetDistribution(); } - bool system_install_; - bool chrome_sxs_; + bool system_install_ = false; base::FilePath chrome_exe_; - BrowserDistribution* distribution_; + BrowserDistribution* distribution_ = nullptr; private: - base::FilePath GetChromePathForParams() const { + base::FilePath GetChromePath() const { base::FilePath chrome_exe; int dir_key = base::DIR_LOCAL_APP_DATA; @@ -211,23 +196,16 @@ dir_key = kSystemKey; } PathService::Get(dir_key, &chrome_exe); -#if defined(GOOGLE_CHROME_BUILD) - chrome_exe = chrome_exe.Append(installer::kGoogleChromeInstallSubDir1); - if (chrome_sxs_) { - chrome_exe = chrome_exe.Append( - base::string16(installer::kGoogleChromeInstallSubDir2) + - installer::kSxSSuffix); - } else { - chrome_exe = chrome_exe.Append(installer::kGoogleChromeInstallSubDir2); - } -#else - chrome_exe = chrome_exe.AppendASCII("Chromium"); -#endif + if (*install_static::kCompanyPathName) + chrome_exe = chrome_exe.Append(install_static::kCompanyPathName); + chrome_exe = chrome_exe.Append( + base::string16(install_static::kProductPathName) + .append(install_static::InstallDetails::Get().install_suffix())); chrome_exe = chrome_exe.Append(installer::kInstallBinaryDir); return chrome_exe.Append(installer::kChromeExe); } - ScopedVector<base::ScopedPathOverride> path_overrides_; + std::unique_ptr<install_static::ScopedInstallDetails> scoped_install_details_; registry_util::RegistryOverrideManager registry_override_manager_; }; @@ -272,16 +250,26 @@ ASSERT_FALSE(first_not_default->Get().is_null()); } -INSTANTIATE_TEST_CASE_P(SystemLevelChrome, - DefaultBrowserBeaconTest, - Values(DistributionVariant::SYSTEM_LEVEL)); -INSTANTIATE_TEST_CASE_P(UserLevelChrome, - DefaultBrowserBeaconTest, - Values(DistributionVariant::USER_LEVEL)); -#if 0 && defined(GOOGLE_CHROME_BUILD) -// Disabled for now since InstallUtil::IsChromeSxSProcess makes this impossible. -INSTANTIATE_TEST_CASE_P(ChromeSxS, DefaultBrowserBeaconTest, - Values(DistributionVariant::SXS)); -#endif +#if defined(GOOGLE_CHROME_BUILD) +// Stable supports user and system levels. +INSTANTIATE_TEST_CASE_P( + Stable, + DefaultBrowserBeaconTest, + testing::Combine(testing::Values(install_static::STABLE_INDEX), + testing::Values("user", "system"))); +// Canary is only at user level. +INSTANTIATE_TEST_CASE_P( + Canary, + DefaultBrowserBeaconTest, + testing::Combine(testing::Values(install_static::CANARY_INDEX), + testing::Values("user"))); +#else // GOOGLE_CHROME_BUILD +// Chromium supports user and system levels. +INSTANTIATE_TEST_CASE_P( + Chromium, + DefaultBrowserBeaconTest, + testing::Combine(testing::Values(install_static::CHROMIUM_INDEX), + testing::Values("user", "system"))); +#endif // GOOGLE_CHROME_BUILD } // namespace installer_util
diff --git a/chrome/installer/util/channel_info.cc b/chrome/installer/util/channel_info.cc index 8d1eb726..39d72b0 100644 --- a/chrome/installer/util/channel_info.cc +++ b/chrome/installer/util/channel_info.cc
@@ -8,8 +8,6 @@ #include "base/logging.h" #include "base/macros.h" -#include "base/strings/string_piece.h" -#include "base/strings/string_util.h" #include "base/win/registry.h" #include "chrome/installer/util/google_update_constants.h" #include "chrome/installer/util/util_constants.h" @@ -138,31 +136,6 @@ return false; } -// Returns the position of the first case-insensitive match of |pattern| found -// in |str|, or base::string16::npos if none found. |pattern| must be non-empty -// lower-case ASCII, and may contain any number of '.' wildcard characters. -size_t FindSubstringMatch(const base::string16& str, - base::StringPiece16 pattern) { - DCHECK(!pattern.empty()); - DCHECK(base::IsStringASCII(pattern)); - DCHECK(pattern == base::StringPiece16(base::ToLowerASCII(pattern))); - - if (str.size() < pattern.size()) - return base::string16::npos; - - for (size_t i = 0; i < str.size() - pattern.size() + 1; ++i) { - size_t j = 0; - while (j < pattern.size() && - (pattern[j] == L'.' || - pattern[j] == base::ToLowerASCII(str[i+j]))) { - ++j; - } - if (j == pattern.size()) - return i; - } - return base::string16::npos; -} - // Returns the value of a modifier - that is for a modifier of the form // "-foo:bar", returns "bar". Returns an empty string if the modifier // is not present or does not have a value. @@ -204,55 +177,6 @@ return true; } -bool ChannelInfo::GetChannelName(base::string16* channel_name) const { - static const wchar_t kChromeChannelBetaPattern[] = L"1.1-"; - static const wchar_t kChromeChannelBetaX64Pattern[] = L"x64-beta"; - static const wchar_t kChromeChannelDevPattern[] = L"2.0-d"; - static const wchar_t kChromeChannelDevX64Pattern[] = L"x64-dev"; - - DCHECK(channel_name); - // Report channels that are empty string or contain "stable" as stable - // (empty string). - if (value_.empty() || value_.find(installer::kChromeChannelStableExplicit) != - base::string16::npos) { - channel_name->erase(); - return true; - } - // Report channels that match "/^2.0-d.*/i" as dev. - if (FindSubstringMatch(value_, kChromeChannelDevPattern) == 0) { - channel_name->assign(installer::kChromeChannelDev); - return true; - } - // Report channels that match "/.*x64-dev.*/" as dev. - if (value_.find(kChromeChannelDevX64Pattern) != base::string16::npos) { - channel_name->assign(installer::kChromeChannelDev); - return true; - } - // Report channels that match "/^1.1-.*/i" as beta. - if (FindSubstringMatch(value_, kChromeChannelBetaPattern) == 0) { - channel_name->assign(installer::kChromeChannelBeta); - return true; - } - // Report channels that match "/.*x64-beta.*/" as beta. - if (value_.find(kChromeChannelBetaX64Pattern) != base::string16::npos) { - channel_name->assign(installer::kChromeChannelBeta); - return true; - } - - // There may be modifiers present. Strip them off and see if we're left - // with the empty string (stable channel). - base::string16 tmp_value = value_; - for (int i = 0; i != NUM_MODIFIERS; ++i) { - SetModifier(static_cast<ModifierIndex>(i), false, &tmp_value); - } - if (tmp_value.empty()) { - channel_name->erase(); - return true; - } - - return false; -} - bool ChannelInfo::IsChrome() const { return HasModifier(MOD_CHROME, value_); }
diff --git a/chrome/installer/util/channel_info.h b/chrome/installer/util/channel_info.h index 76bb255..331f314 100644 --- a/chrome/installer/util/channel_info.h +++ b/chrome/installer/util/channel_info.h
@@ -37,12 +37,6 @@ return value_ == other.value_; } - // Determines the update channel for the value. Possible |channel_name| - // results are the empty string (stable channel), "beta", and "dev". Returns - // false (without modifying |channel_name|) if the channel could not be - // determined. - bool GetChannelName(base::string16* channel_name) const; - // Returns true if the -chrome modifier is present in the value. bool IsChrome() const;
diff --git a/chrome/installer/util/channel_info_unittest.cc b/chrome/installer/util/channel_info_unittest.cc index e25163d..f041cf2 100644 --- a/chrome/installer/util/channel_info_unittest.cc +++ b/chrome/installer/util/channel_info_unittest.cc
@@ -11,95 +11,6 @@ using installer::ChannelInfo; -namespace { - -const base::string16 kChannelStable(installer::kChromeChannelStable); -const base::string16 kChannelBeta(installer::kChromeChannelBeta); -const base::string16 kChannelDev(installer::kChromeChannelDev); - -} // namespace - -TEST(ChannelInfoTest, Channels) { - ChannelInfo ci; - base::string16 channel; - - ci.set_value(L""); - EXPECT_TRUE(ci.GetChannelName(&channel)); - EXPECT_EQ(kChannelStable, channel); - ci.set_value(L"-full"); - EXPECT_TRUE(ci.GetChannelName(&channel)); - EXPECT_EQ(kChannelStable, channel); - - ci.set_value(L"1.1-beta"); - EXPECT_TRUE(ci.GetChannelName(&channel)); - EXPECT_EQ(kChannelBeta, channel); - ci.set_value(L"1.1-beta-foo"); - EXPECT_TRUE(ci.GetChannelName(&channel)); - EXPECT_EQ(kChannelBeta, channel); - ci.set_value(L"1.1-bar"); - EXPECT_TRUE(ci.GetChannelName(&channel)); - EXPECT_EQ(kChannelBeta, channel); - ci.set_value(L"1n1-foobar"); - EXPECT_TRUE(ci.GetChannelName(&channel)); - EXPECT_EQ(kChannelBeta, channel); - ci.set_value(L"foo-1.1-beta"); - EXPECT_FALSE(ci.GetChannelName(&channel)); - ci.set_value(L"2.0-beta"); - EXPECT_FALSE(ci.GetChannelName(&channel)); - - ci.set_value(L"2.0-dev"); - EXPECT_TRUE(ci.GetChannelName(&channel)); - EXPECT_EQ(kChannelDev, channel); - ci.set_value(L"2.0-DEV"); - EXPECT_TRUE(ci.GetChannelName(&channel)); - EXPECT_EQ(kChannelDev, channel); - ci.set_value(L"2.0-dev-eloper"); - EXPECT_TRUE(ci.GetChannelName(&channel)); - EXPECT_EQ(kChannelDev, channel); - ci.set_value(L"2.0-doom"); - EXPECT_TRUE(ci.GetChannelName(&channel)); - EXPECT_EQ(kChannelDev, channel); - ci.set_value(L"250-doom"); - EXPECT_TRUE(ci.GetChannelName(&channel)); - EXPECT_EQ(kChannelDev, channel); - ci.set_value(L"bar-2.0-dev"); - EXPECT_FALSE(ci.GetChannelName(&channel)); - ci.set_value(L"1.0-dev"); - EXPECT_FALSE(ci.GetChannelName(&channel)); - - ci.set_value(L"x64-dev"); - EXPECT_TRUE(ci.GetChannelName(&channel)); - EXPECT_EQ(kChannelDev, channel); - ci.set_value(L"foo-x64-dev"); - EXPECT_TRUE(ci.GetChannelName(&channel)); - EXPECT_EQ(kChannelDev, channel); - ci.set_value(L"x64-Dev"); - EXPECT_FALSE(ci.GetChannelName(&channel)); - - ci.set_value(L"x64-beta"); - EXPECT_TRUE(ci.GetChannelName(&channel)); - EXPECT_EQ(kChannelBeta, channel); - ci.set_value(L"bar-x64-beta"); - EXPECT_TRUE(ci.GetChannelName(&channel)); - EXPECT_EQ(kChannelBeta, channel); - ci.set_value(L"x64-Beta"); - EXPECT_FALSE(ci.GetChannelName(&channel)); - - ci.set_value(L"x64-stable"); - EXPECT_TRUE(ci.GetChannelName(&channel)); - EXPECT_EQ(kChannelStable, channel); - ci.set_value(L"baz-x64-stable"); - EXPECT_TRUE(ci.GetChannelName(&channel)); - EXPECT_EQ(kChannelStable, channel); - ci.set_value(L"x64-Stable"); - EXPECT_FALSE(ci.GetChannelName(&channel)); - - ci.set_value(L"fuzzy"); - EXPECT_FALSE(ci.GetChannelName(&channel)); - ci.set_value(L"foo"); - EXPECT_FALSE(ci.GetChannelName(&channel)); -} - TEST(ChannelInfoTest, FullInstall) { ChannelInfo ci; @@ -235,23 +146,12 @@ } TEST(ChannelInfoTest, GetStatsDefault) { - struct { - const base::string16 base_value; - const base::string16& expected_channel; - } test_cases[] = { - {L"", kChannelStable}, - {L"x64-stable", kChannelStable}, - {L"1.1-beta", kChannelBeta}, - {L"x64-beta", kChannelBeta}, - {L"2.0-dev", kChannelDev}, - {L"x64-dev", kChannelDev}, + const base::string16 base_values[] = { + L"", L"x64-stable", L"1.1-beta", L"x64-beta", L"2.0-dev", L"x64-dev", }; const base::string16 suffixes[] = {L"", L"-multi", L"-multi-chrome"}; - for (const auto& test_case : test_cases) { - const base::string16& base_value = test_case.base_value; - const base::string16& expected_channel = test_case.expected_channel; - + for (const auto& base_value : base_values) { for (const auto& suffix : suffixes) { ChannelInfo ci; base::string16 channel; @@ -264,12 +164,8 @@ EXPECT_EQ(L"", ci.GetStatsDefault()); ci.set_value(base_value + L"-statsdef_0" + suffix); EXPECT_EQ(L"0", ci.GetStatsDefault()); - EXPECT_TRUE(ci.GetChannelName(&channel)); - EXPECT_EQ(expected_channel, channel); ci.set_value(base_value + L"-statsdef_1" + suffix); EXPECT_EQ(L"1", ci.GetStatsDefault()); - EXPECT_TRUE(ci.GetChannelName(&channel)); - EXPECT_EQ(expected_channel, channel); } } }
diff --git a/chrome/installer/util/google_update_settings.cc b/chrome/installer/util/google_update_settings.cc index 19a6b78..190d1de9 100644 --- a/chrome/installer/util/google_update_settings.cc +++ b/chrome/installer/util/google_update_settings.cc
@@ -22,6 +22,7 @@ #include "base/win/registry.h" #include "base/win/win_util.h" #include "chrome/common/chrome_switches.h" +#include "chrome/install_static/install_util.h" #include "chrome/installer/util/app_registration_data.h" #include "chrome/installer/util/browser_distribution.h" #include "chrome/installer/util/channel_info.h" @@ -165,24 +166,6 @@ return channel_info->Initialize(key); } -bool GetChromeChannelInternal(bool system_install, - base::string16* channel) { - // Shortcut in case this distribution knows what channel it is (canary). - if (BrowserDistribution::GetDistribution()->GetChromeChannel(channel)) - return true; - - installer::ChannelInfo channel_info; - if (!InitChannelInfo(system_install, &channel_info)) { - channel->assign(installer::kChromeChannelUnknown); - return false; - } - - if (!channel_info.GetChannelName(channel)) - channel->assign(installer::kChromeChannelUnknown); - - return true; -} - #if defined(GOOGLE_CHROME_BUILD) // Populates |update_policy| with the UpdatePolicy enum value corresponding to a // DWORD read from the registry and returns true if |value| is within range. @@ -207,15 +190,9 @@ } // namespace bool GoogleUpdateSettings::IsSystemInstall() { - bool system_install = false; - base::FilePath module_dir; - if (!PathService::Get(base::DIR_MODULE, &module_dir)) { - LOG(WARNING) - << "Failed to get directory of module; assuming per-user install."; - } else { - system_install = !InstallUtil::IsPerUserInstall(module_dir); - } - return system_install; + // TODO(grt): Remove this unused argument. + base::FilePath unused; + return !InstallUtil::IsPerUserInstall(unused); } bool GoogleUpdateSettings::GetCollectStatsConsent() { @@ -430,9 +407,7 @@ } base::string16 GoogleUpdateSettings::GetChromeChannel(bool system_install) { - base::string16 channel; - GetChromeChannelInternal(system_install, &channel); - return channel; + return install_static::GetChromeChannelName(); } void GoogleUpdateSettings::UpdateInstallStatus(bool system_install,
diff --git a/chrome/installer/util/google_update_settings_unittest.cc b/chrome/installer/util/google_update_settings_unittest.cc index 17a93bf9..ea59e998 100644 --- a/chrome/installer/util/google_update_settings_unittest.cc +++ b/chrome/installer/util/google_update_settings_unittest.cc
@@ -12,6 +12,7 @@ #include "base/base_paths.h" #include "base/macros.h" +#include "base/memory/ptr_util.h" #include "base/path_service.h" #include "base/strings/utf_string_conversions.h" #include "base/test/scoped_path_override.h" @@ -19,6 +20,7 @@ #include "base/win/registry.h" #include "base/win/win_util.h" #include "chrome/common/chrome_constants.h" +#include "chrome/install_static/test/scoped_install_details.h" #include "chrome/installer/util/app_registration_data.h" #include "chrome/installer/util/browser_distribution.h" #include "chrome/installer/util/channel_info.h" @@ -61,76 +63,12 @@ registry_overrides_.OverrideRegistry(HKEY_CURRENT_USER)); } - void SetApField(SystemUserInstall is_system, const wchar_t* value) { - HKEY root = is_system == SYSTEM_INSTALL ? - HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER; - - RegKey update_key; - BrowserDistribution* dist = BrowserDistribution::GetDistribution(); - base::string16 path = dist->GetStateKey(); - ASSERT_EQ(ERROR_SUCCESS, update_key.Create(root, path.c_str(), KEY_WRITE)); - ASSERT_EQ(ERROR_SUCCESS, update_key.WriteValue(L"ap", value)); - } - - // Tests setting the ap= value to various combinations of values with - // suffixes, while asserting on the correct channel value. - // Note that ap= value has to match "^2.0-d.*" or ".*x64-dev.*" and "^1.1-.*" - // or ".*x64-beta.*" for dev and beta channels respectively. - void TestCurrentChromeChannelWithVariousApValues(SystemUserInstall install) { - static struct Expectation { - const wchar_t* ap_value; - const wchar_t* channel; - bool supports_prefixes; - } expectations[] = { - { L"2.0-dev", installer::kChromeChannelDev, false}, - { L"1.1-beta", installer::kChromeChannelBeta, false}, - { L"x64-dev", installer::kChromeChannelDev, true}, - { L"x64-beta", installer::kChromeChannelBeta, true}, - { L"x64-stable", installer::kChromeChannelStable, true}, - }; - bool is_system = install == SYSTEM_INSTALL; - const wchar_t* prefixes[] = { - L"", - L"prefix", - L"prefix-with-dash", - }; - const wchar_t* suffixes[] = { - L"", - L"suffix", - L"suffix-with-dash", - }; - - for (const wchar_t* prefix : prefixes) { - for (const Expectation& expectation : expectations) { - for (const wchar_t* suffix : suffixes) { - base::string16 ap = prefix; - ap += expectation.ap_value; - ap += suffix; - const wchar_t* channel = expectation.channel; - - SetApField(install, ap.c_str()); - const base::string16 ret_channel = - GoogleUpdateSettings::GetChromeChannel(is_system); - - // If prefixes are not supported for a channel, we expect the channel - // to be "unknown" if a non-empty prefix is present in ap_value. - if (!expectation.supports_prefixes && wcslen(prefix) > 0) { - EXPECT_STREQ(installer::kChromeChannelUnknown, ret_channel.c_str()) - << "Expecting channel \"" << installer::kChromeChannelUnknown - << "\" for ap=\"" << ap << "\""; - } else { - EXPECT_STREQ(channel, ret_channel.c_str()) - << "Expecting channel \"" << channel - << "\" for ap=\"" << ap << "\""; - } - } - } - } - } - // Test the writing and deleting functionality of the experiments label // helper. void TestExperimentsLabelHelper(SystemUserInstall install) { + // Install a basic InstallDetails instance. + install_static::ScopedInstallDetails details(install == SYSTEM_INSTALL); + BrowserDistribution* chrome = BrowserDistribution::GetDistribution(); base::string16 value; #if defined(GOOGLE_CHROME_BUILD) @@ -297,51 +235,6 @@ } // namespace -// Verify that we return success on no registration (which means stable), -// whether per-system or per-user install. -TEST_F(GoogleUpdateSettingsTest, CurrentChromeChannelAbsent) { - // Per-system first. - base::string16 channel; - channel = GoogleUpdateSettings::GetChromeChannel(true); - EXPECT_STREQ(L"", channel.c_str()); - - // Then per-user. - channel = GoogleUpdateSettings::GetChromeChannel(false); - EXPECT_STREQ(L"", channel.c_str()); -} - -// Test an empty Ap key for system and user. -TEST_F(GoogleUpdateSettingsTest, CurrentChromeChannelEmptySystem) { - SetApField(SYSTEM_INSTALL, L""); - base::string16 channel; - channel = GoogleUpdateSettings::GetChromeChannel(true); - EXPECT_STREQ(L"", channel.c_str()); - - // Per-user lookups still succeed and return empty string. - channel = GoogleUpdateSettings::GetChromeChannel(false); - EXPECT_STREQ(L"", channel.c_str()); -} - -TEST_F(GoogleUpdateSettingsTest, CurrentChromeChannelEmptyUser) { - SetApField(USER_INSTALL, L""); - // Per-system lookups still succeed and return empty string. - base::string16 channel; - channel = GoogleUpdateSettings::GetChromeChannel(true); - EXPECT_STREQ(L"", channel.c_str()); - - // Per-user lookup should succeed. - channel = GoogleUpdateSettings::GetChromeChannel(false); - EXPECT_STREQ(L"", channel.c_str()); -} - -TEST_F(GoogleUpdateSettingsTest, CurrentChromeChannelVariousApValuesSystem) { - TestCurrentChromeChannelWithVariousApValues(SYSTEM_INSTALL); -} - -TEST_F(GoogleUpdateSettingsTest, CurrentChromeChannelVariousApValuesUser) { - TestCurrentChromeChannelWithVariousApValues(USER_INSTALL); -} - // Run through all combinations of diff vs. full install, success and failure // results, and a fistful of initial "ap" values checking that the expected // final "ap" value is generated by @@ -579,18 +472,8 @@ } TEST_F(GoogleUpdateSettingsTest, UpdateProfileCountsSystemInstall) { - // Override FILE_MODULE and FILE_EXE with a path somewhere in the default - // system-level install location so that - // GoogleUpdateSettings::IsSystemInstall() returns true. - base::FilePath file_exe; - ASSERT_TRUE(PathService::Get(base::FILE_EXE, &file_exe)); - base::FilePath install_dir(installer::GetChromeInstallPath( - true /* system_install */, BrowserDistribution::GetDistribution())); - file_exe = install_dir.Append(file_exe.BaseName()); - base::ScopedPathOverride file_module_override( - base::FILE_MODULE, file_exe, true /* is_absolute */, false /* create */); - base::ScopedPathOverride file_exe_override( - base::FILE_EXE, file_exe, true /* is_absolute */, false /* create */); + // Set up a basic system-level InstallDetails. + install_static::ScopedInstallDetails details(true /* system_level */); // No profile count keys present yet. const base::string16& state_key = BrowserDistribution::GetDistribution()-> @@ -1171,38 +1054,20 @@ // A value parameterized test for testing the stats collection consent setting. class CollectStatsConsent : public ::testing::TestWithParam<StatsState> { - public: - static void SetUpTestCase(); - static void TearDownTestCase(); protected: + CollectStatsConsent(); void SetUp() override; - static void ApplySetting(StatsState::StateSetting setting, - HKEY root_key, - const base::string16& reg_key); + void ApplySetting(StatsState::StateSetting setting, + HKEY root_key, + const base::string16& reg_key); - // TODO(grt): Get rid of these statics and SetUpTestCase. - static base::string16* chrome_version_key_; - static base::string16* chrome_state_key_; - static base::string16* chrome_state_medium_key_; + BrowserDistribution* const dist_; registry_util::RegistryOverrideManager override_manager_; + std::unique_ptr<install_static::ScopedInstallDetails> scoped_install_details_; }; -base::string16* CollectStatsConsent::chrome_version_key_; -base::string16* CollectStatsConsent::chrome_state_key_; -base::string16* CollectStatsConsent::chrome_state_medium_key_; - -void CollectStatsConsent::SetUpTestCase() { - BrowserDistribution* dist = BrowserDistribution::GetDistribution(); - chrome_version_key_ = new base::string16(dist->GetVersionKey()); - chrome_state_key_ = new base::string16(dist->GetStateKey()); - chrome_state_medium_key_ = new base::string16(dist->GetStateMediumKey()); -} - -void CollectStatsConsent::TearDownTestCase() { - delete chrome_version_key_; - delete chrome_state_key_; - delete chrome_state_medium_key_; -} +CollectStatsConsent::CollectStatsConsent() + : dist_(BrowserDistribution::GetDistribution()) {} // Install the registry override and apply the settings to the registry. void CollectStatsConsent::SetUp() { @@ -1213,10 +1078,14 @@ override_manager_.OverrideRegistry(HKEY_CURRENT_USER)); const StatsState& stats_state = GetParam(); + scoped_install_details_ = + base::MakeUnique<install_static::ScopedInstallDetails>( + stats_state.system_level(), 0 /* install_mode_index */); const HKEY root_key = stats_state.root_key(); - ApplySetting(stats_state.state_value(), root_key, *chrome_state_key_); - ApplySetting(stats_state.state_medium_value(), root_key, - *chrome_state_medium_key_); + ASSERT_NO_FATAL_FAILURE( + ApplySetting(stats_state.state_value(), root_key, dist_->GetStateKey())); + ASSERT_NO_FATAL_FAILURE(ApplySetting(stats_state.state_medium_value(), + root_key, dist_->GetStateMediumKey())); } // Write the correct value to represent |setting| in the registry. @@ -1261,9 +1130,9 @@ GetParam().system_level(), !GetParam().is_consent_granted())); - const base::string16& reg_key = GetParam().system_level() - ? *chrome_state_medium_key_ - : *chrome_state_key_; + const base::string16 reg_key = GetParam().system_level() + ? dist_->GetStateMediumKey() + : dist_->GetStateKey(); DWORD value = 0; EXPECT_EQ( ERROR_SUCCESS,
diff --git a/chrome/installer/util/install_util.cc b/chrome/installer/util/install_util.cc index dbaaf2b..277102c 100644 --- a/chrome/installer/util/install_util.cc +++ b/chrome/installer/util/install_util.cc
@@ -11,16 +11,10 @@ #include <shlobj.h> #include <shlwapi.h> -#include <memory> -#include <string> -#include <vector> - #include "base/command_line.h" -#include "base/environment.h" #include "base/files/file_util.h" #include "base/logging.h" #include "base/macros.h" -#include "base/numerics/safe_conversions.h" #include "base/path_service.h" #include "base/process/launch.h" #include "base/strings/string_util.h" @@ -32,6 +26,8 @@ #include "base/win/windows_version.h" #include "chrome/common/chrome_constants.h" #include "chrome/common/chrome_paths.h" +#include "chrome/install_static/install_details.h" +#include "chrome/install_static/install_modes.h" #include "chrome/installer/util/browser_distribution.h" #include "chrome/installer/util/google_update_constants.h" #include "chrome/installer/util/installation_state.h" @@ -44,7 +40,6 @@ namespace { -const char kEnvProgramFilesPath[] = "CHROME_PROBED_PROGRAM_FILES_PATH"; const wchar_t kRegDowngradeVersion[] = L"DowngradeVersion"; // Creates a zero-sized non-decorated foreground window that doesn't appear @@ -276,87 +271,17 @@ } } -bool InstallUtil::IsPerUserInstall(const base::FilePath& exe_path) { - std::unique_ptr<base::Environment> env(base::Environment::Create()); - std::string env_program_files_path; - // Check environment variable to find program files path. - base::FilePath program_files_path; - if (env->GetVar(kEnvProgramFilesPath, &env_program_files_path) && - !env_program_files_path.empty()) { - program_files_path = - base::FilePath(base::UTF8ToWide(env_program_files_path)); - } else { - const int kProgramFilesKey = -#if defined(_WIN64) - // TODO(wfh): Revise this when Chrome is/can be installed in the 64-bit - // program files directory. - base::DIR_PROGRAM_FILESX86; -#else - base::DIR_PROGRAM_FILES; -#endif - if (!PathService::Get(kProgramFilesKey, &program_files_path)) { - NOTREACHED(); - return true; - } - env->SetVar(kEnvProgramFilesPath, - base::WideToUTF8(program_files_path.value())); - } - - // Return true if the program files path is not a case-insensitive prefix of - // the exe path. - if (exe_path.value().size() < program_files_path.value().size()) - return true; - DWORD prefix_len = - base::saturated_cast<DWORD>(program_files_path.value().size()); - return ::CompareString(LOCALE_USER_DEFAULT, NORM_IGNORECASE, - exe_path.value().data(), prefix_len, - program_files_path.value().data(), prefix_len) != - CSTR_EQUAL; -} - -void InstallUtil::ResetIsPerUserInstallForTest() { - std::unique_ptr<base::Environment> env(base::Environment::Create()); - env->UnSetVar(kEnvProgramFilesPath); -} - -bool CheckIsChromeSxSProcess() { - base::CommandLine* command_line = base::CommandLine::ForCurrentProcess(); - CHECK(command_line); - - if (command_line->HasSwitch(installer::switches::kChromeSxS)) - return true; - - // Also return true if we are running from Chrome SxS installed path. - base::FilePath exe_dir; - PathService::Get(base::DIR_EXE, &exe_dir); - base::string16 chrome_sxs_dir(installer::kGoogleChromeInstallSubDir2); - chrome_sxs_dir.append(installer::kSxSSuffix); - - // This is SxS if current EXE is in or under (possibly multiple levels under) - // |chrome_sxs_dir|\|installer::kInstallBinaryDir| - std::vector<base::FilePath::StringType> components; - exe_dir.GetComponents(&components); - // We need at least 1 element in the array for the behavior of the following - // loop to be defined. This should always be true, since we're splitting the - // path to our executable and one of the components will be the drive letter. - DCHECK(!components.empty()); - typedef std::vector<base::FilePath::StringType>::const_reverse_iterator - ComponentsIterator; - for (ComponentsIterator current = components.rbegin(), parent = current + 1; - parent != components.rend(); current = parent++) { - if (base::FilePath::CompareEqualIgnoreCase( - *current, installer::kInstallBinaryDir) && - base::FilePath::CompareEqualIgnoreCase(*parent, chrome_sxs_dir)) { - return true; - } - } - - return false; +bool InstallUtil::IsPerUserInstall(const base::FilePath& /* exe_path */) { + return !install_static::InstallDetails::Get().system_level(); } bool InstallUtil::IsChromeSxSProcess() { - static bool sxs = CheckIsChromeSxSProcess(); - return sxs; +#if defined(GOOGLE_CHROME_BUILD) + return install_static::InstallDetails::Get().install_mode_index() == + install_static::CANARY_INDEX; +#else + return false; +#endif } // static
diff --git a/chrome/installer/util/install_util.h b/chrome/installer/util/install_util.h index 198d3385..3a586a6 100644 --- a/chrome/installer/util/install_util.h +++ b/chrome/installer/util/install_util.h
@@ -86,12 +86,10 @@ // Returns true if this installation path is per user, otherwise returns false // (per machine install, meaning: the exe_path contains the path to Program // Files). + // TODO(grt): remove |exe_path| and consider replacing all callers with + // direct use of InstallDetails. static bool IsPerUserInstall(const base::FilePath& exe_path); - // Resets internal state for IsPerUserInstall so that the next call recomputes - // with fresh data. - static void ResetIsPerUserInstallForTest(); - // Returns true if this is running setup process for Chrome SxS (as // indicated by the presence of --chrome-sxs on the command line) or if this // is running Chrome process from the Chrome SxS installation (as indicated
diff --git a/chrome/installer/util/install_util_unittest.cc b/chrome/installer/util/install_util_unittest.cc index ad1fbff..e52324ae 100644 --- a/chrome/installer/util/install_util_unittest.cc +++ b/chrome/installer/util/install_util_unittest.cc
@@ -369,39 +369,6 @@ L"\"" + short_expect + L"\"")); } -// Win64 Chrome is always installed in the 32-bit Program Files directory. Test -// that IsPerUserInstall returns false for an arbitrary path with -// DIR_PROGRAM_FILESX86 as a suffix but not DIR_PROGRAM_FILES when the two are -// unrelated. -TEST_F(InstallUtilTest, IsPerUserInstall) { - InstallUtil::ResetIsPerUserInstallForTest(); - -#if defined(_WIN64) - const int kChromeProgramFilesKey = base::DIR_PROGRAM_FILESX86; -#else - const int kChromeProgramFilesKey = base::DIR_PROGRAM_FILES; -#endif - base::ScopedPathOverride program_files_override(kChromeProgramFilesKey); - base::FilePath some_exe; - ASSERT_TRUE(PathService::Get(kChromeProgramFilesKey, &some_exe)); - some_exe = some_exe.AppendASCII("Company") - .AppendASCII("Product") - .AppendASCII("product.exe"); - EXPECT_FALSE(InstallUtil::IsPerUserInstall(some_exe)); - InstallUtil::ResetIsPerUserInstallForTest(); - -#if defined(_WIN64) - const int kOtherProgramFilesKey = base::DIR_PROGRAM_FILES; - base::ScopedPathOverride other_program_files_override(kOtherProgramFilesKey); - ASSERT_TRUE(PathService::Get(kOtherProgramFilesKey, &some_exe)); - some_exe = some_exe.AppendASCII("Company") - .AppendASCII("Product") - .AppendASCII("product.exe"); - EXPECT_TRUE(InstallUtil::IsPerUserInstall(some_exe)); - InstallUtil::ResetIsPerUserInstallForTest(); -#endif // defined(_WIN64) -} - TEST_F(InstallUtilTest, AddDowngradeVersion) { TestBrowserDistribution dist; bool system_install = true;
diff --git a/chrome/installer/util/run_all_unittests.cc b/chrome/installer/util/run_all_unittests.cc index a667fa4..9cfc94e 100644 --- a/chrome/installer/util/run_all_unittests.cc +++ b/chrome/installer/util/run_all_unittests.cc
@@ -7,7 +7,7 @@ #include "base/test/test_suite.h" #include "base/win/scoped_com_initializer.h" #include "chrome/common/chrome_paths.h" -#include "chrome/installer/util/install_util.h" +#include "chrome/install_static/test/scoped_install_details.h" int main(int argc, char** argv) { base::TestSuite test_suite(argc, argv); @@ -15,12 +15,12 @@ // Register Chrome Path provider so that we can get test data dir. chrome::RegisterPathProvider(); - // Drop CHROME_PROBED_PROGRAM_FILES_PATH if it leaked into the environment. - InstallUtil::ResetIsPerUserInstallForTest(); - base::win::ScopedCOMInitializer com_initializer; if (!com_initializer.succeeded()) return -1; + + install_static::ScopedInstallDetails scoped_install_details; + return base::LaunchUnitTests( argc, argv,
diff --git a/chrome/test/BUILD.gn b/chrome/test/BUILD.gn index 29df373..471f316 100644 --- a/chrome/test/BUILD.gn +++ b/chrome/test/BUILD.gn
@@ -5114,6 +5114,12 @@ "//chrome/common", "//mojo/edk/test:test_support", ] + + if (is_win) { + deps = [ + "//chrome/install_static/test:test_support", + ] + } } if (is_android) {
diff --git a/components/content_settings/core/common/content_settings_types.h b/components/content_settings/core/common/content_settings_types.h index 236cee90..6f6214e 100644 --- a/components/content_settings/core/common/content_settings_types.h +++ b/components/content_settings/core/common/content_settings_types.h
@@ -50,6 +50,10 @@ CONTENT_SETTINGS_TYPE_PERMISSION_AUTOBLOCKER_DATA, CONTENT_SETTINGS_TYPE_SUBRESOURCE_FILTER, + // This is special-cased in the permissions layer to always allow, and as + // such doesn't have associated prefs data. + CONTENT_SETTINGS_TYPE_MIDI, + // This is only here temporarily and will be removed when we further unify // it with notifications, see crbug.com/563297. No prefs data is stored for // this content type, we instead share values with NOTIFICATIONS.
diff --git a/components/minidump_uploader/BUILD.gn b/components/minidump_uploader/BUILD.gn index 1da865ad..e8c9b72 100644 --- a/components/minidump_uploader/BUILD.gn +++ b/components/minidump_uploader/BUILD.gn
@@ -13,6 +13,7 @@ java_files = [ "android/java/src/org/chromium/components/minidump_uploader/CrashFileManager.java", "android/java/src/org/chromium/components/minidump_uploader/MinidumpUploadCallable.java", + "android/java/src/org/chromium/components/minidump_uploader/MinidumpUploader.java", "android/java/src/org/chromium/components/minidump_uploader/util/CrashReportingPermissionManager.java", "android/java/src/org/chromium/components/minidump_uploader/util/HttpURLConnectionFactory.java", "android/java/src/org/chromium/components/minidump_uploader/util/HttpURLConnectionFactoryImpl.java",
diff --git a/android_webview/java/src/org/chromium/android_webview/crash/MinidumpUploader.java b/components/minidump_uploader/android/java/src/org/chromium/components/minidump_uploader/MinidumpUploader.java similarity index 94% rename from android_webview/java/src/org/chromium/android_webview/crash/MinidumpUploader.java rename to components/minidump_uploader/android/java/src/org/chromium/components/minidump_uploader/MinidumpUploader.java index 1957f36a..7264aeb3 100644 --- a/android_webview/java/src/org/chromium/android_webview/crash/MinidumpUploader.java +++ b/components/minidump_uploader/android/java/src/org/chromium/components/minidump_uploader/MinidumpUploader.java
@@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -package org.chromium.android_webview.crash; +package org.chromium.components.minidump_uploader; /** * Interface for uploading minidumps.
diff --git a/content/browser/indexed_db/indexed_db_backing_store.cc b/content/browser/indexed_db/indexed_db_backing_store.cc index 29934dc..ff549bb 100644 --- a/content/browser/indexed_db/indexed_db_backing_store.cc +++ b/content/browser/indexed_db/indexed_db_backing_store.cc
@@ -699,8 +699,20 @@ Status DeleteRangeBasic(LevelDBTransaction* transaction, const std::string& begin, const std::string& end, - bool upper_open) { - return transaction->RemoveRange(begin, end, upper_open); + bool upper_open, + size_t* delete_count) { + DCHECK(delete_count); + std::unique_ptr<LevelDBIterator> it = transaction->CreateIterator(); + Status s; + *delete_count = 0; + for (s = it->Seek(begin); s.ok() && it->IsValid() && + (upper_open ? CompareKeys(it->Key(), end) < 0 + : CompareKeys(it->Key(), end) <= 0); + s = it->Next()) { + if (transaction->Remove(it->Key())) + (*delete_count)++; + } + return s; } Status DeleteBlobsInRange(IndexedDBBackingStore::Transaction* transaction, @@ -1988,6 +2000,7 @@ LevelDBTransaction* leveldb_transaction = transaction->transaction(); base::string16 object_store_name; + size_t delete_count = 0; bool found = false; Status s = GetString(leveldb_transaction, ObjectStoreMetaDataKey::Encode( database_id, object_store_id, @@ -2011,7 +2024,8 @@ s = DeleteRangeBasic( leveldb_transaction, ObjectStoreMetaDataKey::Encode(database_id, object_store_id, 0), - ObjectStoreMetaDataKey::EncodeMaxKey(database_id, object_store_id), true); + ObjectStoreMetaDataKey::EncodeMaxKey(database_id, object_store_id), true, + &delete_count); if (s.ok()) { leveldb_transaction->Remove( @@ -2020,14 +2034,16 @@ s = DeleteRangeBasic( leveldb_transaction, IndexFreeListKey::Encode(database_id, object_store_id, 0), - IndexFreeListKey::EncodeMaxKey(database_id, object_store_id), true); + IndexFreeListKey::EncodeMaxKey(database_id, object_store_id), true, + &delete_count); } if (s.ok()) { s = DeleteRangeBasic( leveldb_transaction, IndexMetaDataKey::Encode(database_id, object_store_id, 0, 0), - IndexMetaDataKey::EncodeMaxKey(database_id, object_store_id), true); + IndexMetaDataKey::EncodeMaxKey(database_id, object_store_id), true, + &delete_count); } if (!s.ok()) { @@ -2174,8 +2190,9 @@ KeyPrefix(database_id, object_store_id).Encode(); const std::string stop_key = KeyPrefix(database_id, object_store_id + 1).Encode(); - Status s = - DeleteRangeBasic(transaction->transaction(), start_key, stop_key, true); + size_t delete_count = 0; + Status s = DeleteRangeBasic(transaction->transaction(), start_key, stop_key, + true, &delete_count); if (!s.ok()) { INTERNAL_WRITE_ERROR(CLEAR_OBJECT_STORE); return s; @@ -2211,8 +2228,11 @@ IndexedDBBackingStore::Transaction* transaction, int64_t database_id, int64_t object_store_id, - const IndexedDBKeyRange& key_range) { + const IndexedDBKeyRange& key_range, + size_t* exists_delete_count) { + DCHECK(exists_delete_count); Status s; + *exists_delete_count = 0; std::unique_ptr<IndexedDBBackingStore::Cursor> start_cursor = OpenObjectStoreCursor(transaction, database_id, object_store_id, key_range, blink::WebIDBCursorDirectionNext, &s); @@ -2249,7 +2269,9 @@ false); if (!s.ok()) return s; - s = DeleteRangeBasic(transaction->transaction(), start_key, stop_key, false); + size_t data_delete_count = 0; + s = DeleteRangeBasic(transaction->transaction(), start_key, stop_key, false, + &data_delete_count); if (!s.ok()) return s; start_key = @@ -2257,7 +2279,9 @@ stop_key = ExistsEntryKey::Encode(database_id, object_store_id, end_cursor->key()); - s = DeleteRangeBasic(transaction->transaction(), start_key, stop_key, false); + s = DeleteRangeBasic(transaction->transaction(), start_key, stop_key, false, + exists_delete_count); + DCHECK_EQ(data_delete_count, *exists_delete_count); return s; } @@ -3005,12 +3029,13 @@ return InvalidDBKeyStatus(); LevelDBTransaction* leveldb_transaction = transaction->transaction(); + size_t delete_count = 0; const std::string index_meta_data_start = IndexMetaDataKey::Encode(database_id, object_store_id, index_id, 0); const std::string index_meta_data_end = IndexMetaDataKey::EncodeMaxKey(database_id, object_store_id, index_id); Status s = DeleteRangeBasic(leveldb_transaction, index_meta_data_start, - index_meta_data_end, true); + index_meta_data_end, true, &delete_count); if (s.ok()) { const std::string index_data_start = @@ -3018,7 +3043,7 @@ const std::string index_data_end = IndexDataKey::EncodeMaxKey(database_id, object_store_id, index_id); s = DeleteRangeBasic(leveldb_transaction, index_data_start, index_data_end, - true); + true, &delete_count); } if (!s.ok())
diff --git a/content/browser/indexed_db/indexed_db_backing_store.h b/content/browser/indexed_db/indexed_db_backing_store.h index 58d4003..46d8591 100644 --- a/content/browser/indexed_db/indexed_db_backing_store.h +++ b/content/browser/indexed_db/indexed_db_backing_store.h
@@ -484,7 +484,8 @@ IndexedDBBackingStore::Transaction* transaction, int64_t database_id, int64_t object_store_id, - const IndexedDBKeyRange&) WARN_UNUSED_RESULT; + const IndexedDBKeyRange&, + size_t* delete_count) WARN_UNUSED_RESULT; virtual leveldb::Status GetKeyGeneratorCurrentNumber( IndexedDBBackingStore::Transaction* transaction, int64_t database_id,
diff --git a/content/browser/indexed_db/indexed_db_backing_store_unittest.cc b/content/browser/indexed_db/indexed_db_backing_store_unittest.cc index 3406b3b..ca6ee07 100644 --- a/content/browser/indexed_db/indexed_db_backing_store_unittest.cc +++ b/content/browser/indexed_db/indexed_db_backing_store_unittest.cc
@@ -473,10 +473,12 @@ IndexedDBBackingStore::Transaction transaction3(backing_store_.get()); transaction3.Begin(); IndexedDBValue result_value; - EXPECT_TRUE( - backing_store_ - ->DeleteRange(&transaction3, 1, 1, IndexedDBKeyRange(m_key3)) - .ok()); + size_t delete_count = 0; + EXPECT_TRUE(backing_store_ + ->DeleteRange(&transaction3, 1, 1, + IndexedDBKeyRange(m_key3), &delete_count) + .ok()); + EXPECT_EQ(1UL, delete_count); scoped_refptr<TestCallback> callback(new TestCallback()); EXPECT_TRUE(transaction3.CommitPhaseOne(callback).ok()); task_runner_->RunUntilIdle(); @@ -561,8 +563,12 @@ IndexedDBBackingStore::Transaction transaction2(backing_store_.get()); transaction2.Begin(); IndexedDBValue result_value; + size_t delete_count = 0; EXPECT_TRUE( - backing_store_->DeleteRange(&transaction2, 1, i + 1, ranges[i]).ok()); + backing_store_ + ->DeleteRange(&transaction2, 1, i + 1, ranges[i], &delete_count) + .ok()); + EXPECT_EQ(2UL, delete_count); scoped_refptr<TestCallback> callback(new TestCallback()); EXPECT_TRUE(transaction2.CommitPhaseOne(callback).ok()); task_runner_->RunUntilIdle(); @@ -651,8 +657,12 @@ IndexedDBBackingStore::Transaction transaction2(backing_store_.get()); transaction2.Begin(); IndexedDBValue result_value; + size_t delete_count = 0; EXPECT_TRUE( - backing_store_->DeleteRange(&transaction2, 1, i + 1, ranges[i]).ok()); + backing_store_ + ->DeleteRange(&transaction2, 1, i + 1, ranges[i], &delete_count) + .ok()); + EXPECT_EQ(0UL, delete_count); scoped_refptr<TestCallback> callback(new TestCallback()); EXPECT_TRUE(transaction2.CommitPhaseOne(callback).ok()); task_runner_->RunUntilIdle(); @@ -746,10 +756,12 @@ { IndexedDBBackingStore::Transaction transaction3(backing_store_.get()); transaction3.Begin(); - EXPECT_TRUE( - backing_store_ - ->DeleteRange(&transaction3, 1, 1, IndexedDBKeyRange(m_key3)) - .ok()); + size_t delete_count = 0; + EXPECT_TRUE(backing_store_ + ->DeleteRange(&transaction3, 1, 1, + IndexedDBKeyRange(m_key3), &delete_count) + .ok()); + EXPECT_EQ(1UL, delete_count); scoped_refptr<TestCallback> callback(new TestCallback()); EXPECT_TRUE(transaction3.CommitPhaseOne(callback).ok()); task_runner_->RunUntilIdle();
diff --git a/content/browser/indexed_db/indexed_db_database.cc b/content/browser/indexed_db/indexed_db_database.cc index ca13e95..2392229 100644 --- a/content/browser/indexed_db/indexed_db_database.cc +++ b/content/browser/indexed_db/indexed_db_database.cc
@@ -1679,9 +1679,10 @@ IndexedDBTransaction* transaction) { IDB_TRACE1("IndexedDBDatabase::DeleteRangeOperation", "txn.id", transaction->id()); + size_t delete_count = 0; leveldb::Status s = backing_store_->DeleteRange(transaction->BackingStoreTransaction(), id(), - object_store_id, *key_range); + object_store_id, *key_range, &delete_count); if (!s.ok()) return s; callbacks->OnSuccess();
diff --git a/content/browser/indexed_db/leveldb/leveldb_transaction.cc b/content/browser/indexed_db/leveldb/leveldb_transaction.cc index 76a7403..016e7de 100644 --- a/content/browser/indexed_db/leveldb/leveldb_transaction.cc +++ b/content/browser/indexed_db/leveldb/leveldb_transaction.cc
@@ -25,7 +25,7 @@ data_(data_comparator_), finished_(false) {} -LevelDBTransaction::Record::Record() {} +LevelDBTransaction::Record::Record() : deleted(false) {} LevelDBTransaction::Record::~Record() {} LevelDBTransaction::~LevelDBTransaction() {} @@ -60,29 +60,6 @@ return !Set(key, &empty, true); } -leveldb::Status LevelDBTransaction::RemoveRange(const StringPiece& begin, - const StringPiece& end, - bool upper_open) { - IDB_TRACE("LevelDBTransaction::RemoveRange"); - data_.erase(data_.lower_bound(begin), data_.lower_bound(end)); - if (!upper_open) - data_.erase(end); - std::unique_ptr<LevelDBIterator> it = CreateIterator(); - leveldb::Status s; - for (s = it->Seek(begin); - s.ok() && it->IsValid() && - (upper_open ? comparator_->Compare(it->Key(), end) < 0 - : comparator_->Compare(it->Key(), end) <= 0); - s = it->Next()) { - std::unique_ptr<Record> record = base::MakeUnique<Record>(); - record->key = it->Key().as_string(); - record->deleted = true; - data_[record->key] = std::move(record); - } - NotifyIterators(); - return s; -} - leveldb::Status LevelDBTransaction::Get(const StringPiece& key, std::string* value, bool* found) {
diff --git a/content/browser/indexed_db/leveldb/leveldb_transaction.h b/content/browser/indexed_db/leveldb/leveldb_transaction.h index ef4862812..33392f2 100644 --- a/content/browser/indexed_db/leveldb/leveldb_transaction.h +++ b/content/browser/indexed_db/leveldb/leveldb_transaction.h
@@ -30,11 +30,6 @@ // Returns true if this operation performs a change, where the value wasn't // already deleted. bool Remove(const base::StringPiece& key); - - leveldb::Status RemoveRange(const base::StringPiece& begin, - const base::StringPiece& end, - bool upper_open); - virtual leveldb::Status Get(const base::StringPiece& key, std::string* value, bool* found); @@ -50,22 +45,16 @@ private: friend class base::RefCounted<LevelDBTransaction>; - FRIEND_TEST_ALL_PREFIXES(LevelDBTransactionTest, GetAndPut); - FRIEND_TEST_ALL_PREFIXES(LevelDBTransactionTest, Commit); - FRIEND_TEST_ALL_PREFIXES(LevelDBTransactionTest, Iterator); - FRIEND_TEST_ALL_PREFIXES(LevelDBTransactionTest, RemoveRangeBackingData); - FRIEND_TEST_ALL_PREFIXES(LevelDBTransactionTest, - RemoveRangeBackingDataUpperOpen); - FRIEND_TEST_ALL_PREFIXES(LevelDBTransactionTest, RemoveRangeMemoryData); - FRIEND_TEST_ALL_PREFIXES(LevelDBTransactionTest, - RemoveRangeMemoryDataUpperOpen); + FRIEND_TEST_ALL_PREFIXES(LevelDBDatabaseTest, Transaction); + FRIEND_TEST_ALL_PREFIXES(LevelDBDatabaseTest, TransactionCommitTest); + FRIEND_TEST_ALL_PREFIXES(LevelDBDatabaseTest, TransactionIterator); struct Record { Record(); ~Record(); std::string key; std::string value; - bool deleted = false; + bool deleted; }; class Comparator {
diff --git a/content/browser/indexed_db/leveldb/leveldb_transaction_unittest.cc b/content/browser/indexed_db/leveldb/leveldb_transaction_unittest.cc deleted file mode 100644 index bbdd6522..0000000 --- a/content/browser/indexed_db/leveldb/leveldb_transaction_unittest.cc +++ /dev/null
@@ -1,368 +0,0 @@ -// Copyright 2017 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include <stddef.h> - -#include <algorithm> -#include <cstring> -#include <string> - -#include "base/files/file.h" -#include "base/files/file_path.h" -#include "base/files/scoped_temp_dir.h" -#include "base/strings/string_piece.h" -#include "content/browser/indexed_db/leveldb/leveldb_comparator.h" -#include "content/browser/indexed_db/leveldb/leveldb_database.h" -#include "content/browser/indexed_db/leveldb/leveldb_env.h" -#include "content/browser/indexed_db/leveldb/leveldb_iterator.h" -#include "content/browser/indexed_db/leveldb/leveldb_transaction.h" -#include "testing/gtest/include/gtest/gtest.h" -#include "third_party/leveldatabase/env_chromium.h" - -namespace content { - -namespace { - -class SimpleComparator : public LevelDBComparator { - public: - int Compare(const base::StringPiece& a, - const base::StringPiece& b) const override { - size_t len = std::min(a.size(), b.size()); - return memcmp(a.begin(), b.begin(), len); - } - const char* Name() const override { return "temp_comparator"; } -}; - -class LevelDBTransactionTest : public testing::Test { - public: - LevelDBTransactionTest() {} - void SetUp() override { - ASSERT_TRUE(temp_directory_.CreateUniqueTempDir()); - LevelDBDatabase::Open(temp_directory_.GetPath(), &comparator_, &leveldb_); - ASSERT_TRUE(leveldb_); - } - void TearDown() override {} - - protected: - // Convenience methods to access the database outside any - // transaction to cut down on boilerplate around calls. - void Put(const base::StringPiece& key, const std::string& value) { - std::string put_value = value; - leveldb::Status s = leveldb_->Put(key, &put_value); - ASSERT_TRUE(s.ok()); - } - - void Remove(const base::StringPiece& key) { - leveldb::Status s = leveldb_->Remove(key); - ASSERT_TRUE(s.ok()); - } - - void Get(const base::StringPiece& key, std::string* value, bool* found) { - leveldb::Status s = leveldb_->Get(key, value, found); - ASSERT_TRUE(s.ok()); - } - - bool Has(const base::StringPiece& key) { - bool found; - std::string value; - leveldb::Status s = leveldb_->Get(key, &value, &found); - EXPECT_TRUE(s.ok()); - return found; - } - - // Convenience wrappers for LevelDBTransaction operations to - // avoid boilerplate in tests. - bool TransactionHas(LevelDBTransaction* transaction, - const base::StringPiece& key) { - std::string value; - bool found; - leveldb::Status s = transaction->Get(key, &value, &found); - EXPECT_TRUE(s.ok()); - return found; - } - - void TransactionPut(LevelDBTransaction* transaction, - const base::StringPiece& key, - const std::string& value) { - std::string put_value = value; - transaction->Put(key, &put_value); - } - - int Compare(const base::StringPiece& a, const base::StringPiece& b) const { - return comparator_.Compare(a, b); - } - - LevelDBDatabase* db() { return leveldb_.get(); } - - const std::string key_before_range_ = "a"; - const std::string range_start_ = "b"; - const std::string key_in_range1_ = "c"; - const std::string key_in_range2_ = "d"; - const std::string range_end_ = "e"; - const std::string key_after_range_ = "f"; - const std::string value_ = "value"; - - private: - base::ScopedTempDir temp_directory_; - SimpleComparator comparator_; - std::unique_ptr<LevelDBDatabase> leveldb_; - - DISALLOW_COPY_AND_ASSIGN(LevelDBTransactionTest); -}; - -} // namespace - -TEST_F(LevelDBTransactionTest, GetAndPut) { - leveldb::Status status; - - const std::string key("key"); - std::string got_value; - - const std::string old_value("value"); - Put(key, old_value); - - scoped_refptr<LevelDBTransaction> transaction = new LevelDBTransaction(db()); - - const std::string new_value("new value"); - Put(key, new_value); - - bool found = false; - status = transaction->Get(key, &got_value, &found); - EXPECT_TRUE(status.ok()); - EXPECT_TRUE(found); - EXPECT_EQ(Compare(got_value, old_value), 0); - - Get(key, &got_value, &found); - EXPECT_TRUE(found); - EXPECT_EQ(Compare(got_value, new_value), 0); - - const std::string added_key("added key"); - const std::string added_value("added value"); - Put(added_key, added_value); - - Get(added_key, &got_value, &found); - EXPECT_TRUE(found); - EXPECT_EQ(Compare(got_value, added_value), 0); - - EXPECT_FALSE(TransactionHas(transaction.get(), added_key)); - - const std::string another_key("another key"); - const std::string another_value("another value"); - TransactionPut(transaction.get(), another_key, another_value); - - status = transaction->Get(another_key, &got_value, &found); - EXPECT_TRUE(status.ok()); - EXPECT_TRUE(found); - EXPECT_EQ(Compare(got_value, another_value), 0); -} - -TEST_F(LevelDBTransactionTest, Iterator) { - const std::string key1("key1"); - const std::string value1("value1"); - const std::string key2("key2"); - const std::string value2("value2"); - - Put(key1, value1); - Put(key2, value2); - - scoped_refptr<LevelDBTransaction> transaction = new LevelDBTransaction(db()); - - Remove(key2); - - std::unique_ptr<LevelDBIterator> it = transaction->CreateIterator(); - - it->Seek(std::string()); - - EXPECT_TRUE(it->IsValid()); - EXPECT_EQ(Compare(it->Key(), key1), 0); - EXPECT_EQ(Compare(it->Value(), value1), 0); - - it->Next(); - - EXPECT_TRUE(it->IsValid()); - EXPECT_EQ(Compare(it->Key(), key2), 0); - EXPECT_EQ(Compare(it->Value(), value2), 0); - - it->Next(); - - EXPECT_FALSE(it->IsValid()); -} - -TEST_F(LevelDBTransactionTest, Commit) { - const std::string key1("key1"); - const std::string key2("key2"); - const std::string value1("value1"); - const std::string value2("value2"); - const std::string value3("value3"); - - std::string got_value; - bool found; - - scoped_refptr<LevelDBTransaction> transaction = new LevelDBTransaction(db()); - - TransactionPut(transaction.get(), key1, value1); - TransactionPut(transaction.get(), key2, value2); - TransactionPut(transaction.get(), key2, value3); - - leveldb::Status status = transaction->Commit(); - EXPECT_TRUE(status.ok()); - - Get(key1, &got_value, &found); - EXPECT_TRUE(found); - EXPECT_EQ(value1, got_value); - - Get(key2, &got_value, &found); - EXPECT_TRUE(found); - EXPECT_EQ(value3, got_value); -} - -// Test removals where the data is in the database, and the range has a closed -// upper bound (inclusive). -TEST_F(LevelDBTransactionTest, RemoveRangeBackingData) { - std::string got_value; - leveldb::Status status; - - Put(key_before_range_, value_); - Put(range_start_, value_); - Put(key_in_range1_, value_); - Put(key_in_range2_, value_); - Put(range_end_, value_); - Put(key_after_range_, value_); - - scoped_refptr<LevelDBTransaction> transaction = new LevelDBTransaction(db()); - - const bool upper_open = false; - status = transaction->RemoveRange(range_start_, range_end_, upper_open); - EXPECT_TRUE(status.ok()); - - EXPECT_TRUE(TransactionHas(transaction.get(), key_before_range_)); - EXPECT_FALSE(TransactionHas(transaction.get(), range_start_)); - EXPECT_FALSE(TransactionHas(transaction.get(), key_in_range1_)); - EXPECT_FALSE(TransactionHas(transaction.get(), key_in_range2_)); - EXPECT_FALSE(TransactionHas(transaction.get(), range_end_)); - EXPECT_TRUE(TransactionHas(transaction.get(), key_after_range_)); - - status = transaction->Commit(); - EXPECT_TRUE(status.ok()); - - EXPECT_TRUE(Has(key_before_range_)); - EXPECT_FALSE(Has(range_start_)); - EXPECT_FALSE(Has(key_in_range1_)); - EXPECT_FALSE(Has(key_in_range2_)); - EXPECT_FALSE(Has(range_end_)); - EXPECT_TRUE(Has(key_after_range_)); -} - -// Test removals where the data is in the database, and the range has an open -// upper bound (exclusive). -TEST_F(LevelDBTransactionTest, RemoveRangeBackingDataUpperOpen) { - std::string got_value; - leveldb::Status status; - - Put(key_before_range_, value_); - Put(range_start_, value_); - Put(key_in_range1_, value_); - Put(key_in_range2_, value_); - Put(range_end_, value_); - Put(key_after_range_, value_); - - scoped_refptr<LevelDBTransaction> transaction = new LevelDBTransaction(db()); - - const bool upper_open = true; - status = transaction->RemoveRange(range_start_, range_end_, upper_open); - EXPECT_TRUE(status.ok()); - - EXPECT_TRUE(TransactionHas(transaction.get(), key_before_range_)); - EXPECT_FALSE(TransactionHas(transaction.get(), range_start_)); - EXPECT_FALSE(TransactionHas(transaction.get(), key_in_range1_)); - EXPECT_FALSE(TransactionHas(transaction.get(), key_in_range2_)); - EXPECT_TRUE(TransactionHas(transaction.get(), range_end_)); - EXPECT_TRUE(TransactionHas(transaction.get(), key_after_range_)); - - status = transaction->Commit(); - EXPECT_TRUE(status.ok()); - - EXPECT_TRUE(Has(key_before_range_)); - EXPECT_FALSE(Has(range_start_)); - EXPECT_FALSE(Has(key_in_range1_)); - EXPECT_FALSE(Has(key_in_range2_)); - EXPECT_TRUE(Has(range_end_)); - EXPECT_TRUE(Has(key_after_range_)); -} - -// Test removals where the data is in transaction's memory map, and the range -// has a closed upper bound (inclusive). -TEST_F(LevelDBTransactionTest, RemoveRangeMemoryData) { - std::string got_value; - leveldb::Status status; - - scoped_refptr<LevelDBTransaction> transaction = new LevelDBTransaction(db()); - - TransactionPut(transaction.get(), key_before_range_, value_); - TransactionPut(transaction.get(), range_start_, value_); - TransactionPut(transaction.get(), key_in_range1_, value_); - TransactionPut(transaction.get(), key_in_range2_, value_); - TransactionPut(transaction.get(), range_end_, value_); - TransactionPut(transaction.get(), key_after_range_, value_); - - const bool upper_open = false; - status = transaction->RemoveRange(range_start_, range_end_, upper_open); - EXPECT_TRUE(status.ok()); - - EXPECT_TRUE(TransactionHas(transaction.get(), key_before_range_)); - EXPECT_FALSE(TransactionHas(transaction.get(), range_start_)); - EXPECT_FALSE(TransactionHas(transaction.get(), key_in_range1_)); - EXPECT_FALSE(TransactionHas(transaction.get(), key_in_range2_)); - EXPECT_FALSE(TransactionHas(transaction.get(), range_end_)); - EXPECT_TRUE(TransactionHas(transaction.get(), key_after_range_)); - - status = transaction->Commit(); - EXPECT_TRUE(status.ok()); - - EXPECT_TRUE(Has(key_before_range_)); - EXPECT_FALSE(Has(range_start_)); - EXPECT_FALSE(Has(key_in_range1_)); - EXPECT_FALSE(Has(key_in_range2_)); - EXPECT_FALSE(Has(range_end_)); - EXPECT_TRUE(Has(key_after_range_)); -} - -// Test removals where the data is in transaction's memory map, and the range -// has an open upper bound (exclusive). -TEST_F(LevelDBTransactionTest, RemoveRangeMemoryDataUpperOpen) { - std::string got_value; - leveldb::Status status; - - scoped_refptr<LevelDBTransaction> transaction = new LevelDBTransaction(db()); - - TransactionPut(transaction.get(), key_before_range_, value_); - TransactionPut(transaction.get(), range_start_, value_); - TransactionPut(transaction.get(), key_in_range1_, value_); - TransactionPut(transaction.get(), key_in_range2_, value_); - TransactionPut(transaction.get(), range_end_, value_); - TransactionPut(transaction.get(), key_after_range_, value_); - - const bool upper_open = true; - status = transaction->RemoveRange(range_start_, range_end_, upper_open); - EXPECT_TRUE(status.ok()); - - EXPECT_TRUE(TransactionHas(transaction.get(), key_before_range_)); - EXPECT_FALSE(TransactionHas(transaction.get(), range_start_)); - EXPECT_FALSE(TransactionHas(transaction.get(), key_in_range1_)); - EXPECT_FALSE(TransactionHas(transaction.get(), key_in_range2_)); - EXPECT_TRUE(TransactionHas(transaction.get(), range_end_)); - EXPECT_TRUE(TransactionHas(transaction.get(), key_after_range_)); - - status = transaction->Commit(); - EXPECT_TRUE(status.ok()); - - EXPECT_TRUE(Has(key_before_range_)); - EXPECT_FALSE(Has(range_start_)); - EXPECT_FALSE(Has(key_in_range1_)); - EXPECT_FALSE(Has(key_in_range2_)); - EXPECT_TRUE(Has(range_end_)); - EXPECT_TRUE(Has(key_after_range_)); -} - -} // namespace content
diff --git a/content/browser/indexed_db/leveldb/leveldb_unittest.cc b/content/browser/indexed_db/leveldb/leveldb_unittest.cc index 379a6ba..1ce48cb4 100644 --- a/content/browser/indexed_db/leveldb/leveldb_unittest.cc +++ b/content/browser/indexed_db/leveldb/leveldb_unittest.cc
@@ -11,10 +11,13 @@ #include "base/files/file.h" #include "base/files/file_path.h" #include "base/files/scoped_temp_dir.h" +#include "base/strings/string16.h" #include "base/strings/string_piece.h" #include "content/browser/indexed_db/leveldb/leveldb_comparator.h" #include "content/browser/indexed_db/leveldb/leveldb_database.h" #include "content/browser/indexed_db/leveldb/leveldb_env.h" +#include "content/browser/indexed_db/leveldb/leveldb_iterator.h" +#include "content/browser/indexed_db/leveldb/leveldb_transaction.h" #include "testing/gtest/include/gtest/gtest.h" #include "third_party/leveldatabase/env_chromium.h" @@ -86,6 +89,162 @@ EXPECT_FALSE(found); } +TEST(LevelDBDatabaseTest, Transaction) { + base::ScopedTempDir temp_directory; + ASSERT_TRUE(temp_directory.CreateUniqueTempDir()); + + const std::string key("key"); + std::string got_value; + std::string put_value; + SimpleComparator comparator; + + std::unique_ptr<LevelDBDatabase> leveldb; + LevelDBDatabase::Open(temp_directory.GetPath(), &comparator, &leveldb); + EXPECT_TRUE(leveldb); + + const std::string old_value("value"); + put_value = old_value; + leveldb::Status status = leveldb->Put(key, &put_value); + EXPECT_TRUE(status.ok()); + + scoped_refptr<LevelDBTransaction> transaction = + new LevelDBTransaction(leveldb.get()); + + const std::string new_value("new value"); + put_value = new_value; + status = leveldb->Put(key, &put_value); + EXPECT_TRUE(status.ok()); + + bool found = false; + status = transaction->Get(key, &got_value, &found); + EXPECT_TRUE(status.ok()); + EXPECT_TRUE(found); + EXPECT_EQ(comparator.Compare(got_value, old_value), 0); + + found = false; + status = leveldb->Get(key, &got_value, &found); + EXPECT_TRUE(status.ok()); + EXPECT_TRUE(found); + EXPECT_EQ(comparator.Compare(got_value, new_value), 0); + + const std::string added_key("added key"); + const std::string added_value("added value"); + put_value = added_value; + status = leveldb->Put(added_key, &put_value); + EXPECT_TRUE(status.ok()); + + status = leveldb->Get(added_key, &got_value, &found); + EXPECT_TRUE(status.ok()); + EXPECT_TRUE(found); + EXPECT_EQ(comparator.Compare(got_value, added_value), 0); + + status = transaction->Get(added_key, &got_value, &found); + EXPECT_TRUE(status.ok()); + EXPECT_FALSE(found); + + const std::string another_key("another key"); + const std::string another_value("another value"); + put_value = another_value; + transaction->Put(another_key, &put_value); + + status = transaction->Get(another_key, &got_value, &found); + EXPECT_TRUE(status.ok()); + EXPECT_TRUE(found); + EXPECT_EQ(comparator.Compare(got_value, another_value), 0); +} + +TEST(LevelDBDatabaseTest, TransactionIterator) { + base::ScopedTempDir temp_directory; + ASSERT_TRUE(temp_directory.CreateUniqueTempDir()); + + const std::string key1("key1"); + const std::string value1("value1"); + const std::string key2("key2"); + const std::string value2("value2"); + std::string put_value; + SimpleComparator comparator; + + std::unique_ptr<LevelDBDatabase> leveldb; + LevelDBDatabase::Open(temp_directory.GetPath(), &comparator, &leveldb); + EXPECT_TRUE(leveldb); + + put_value = value1; + leveldb::Status s = leveldb->Put(key1, &put_value); + EXPECT_TRUE(s.ok()); + put_value = value2; + s = leveldb->Put(key2, &put_value); + EXPECT_TRUE(s.ok()); + + scoped_refptr<LevelDBTransaction> transaction = + new LevelDBTransaction(leveldb.get()); + + s = leveldb->Remove(key2); + EXPECT_TRUE(s.ok()); + + std::unique_ptr<LevelDBIterator> it = transaction->CreateIterator(); + + it->Seek(std::string()); + + EXPECT_TRUE(it->IsValid()); + EXPECT_EQ(comparator.Compare(it->Key(), key1), 0); + EXPECT_EQ(comparator.Compare(it->Value(), value1), 0); + + it->Next(); + + EXPECT_TRUE(it->IsValid()); + EXPECT_EQ(comparator.Compare(it->Key(), key2), 0); + EXPECT_EQ(comparator.Compare(it->Value(), value2), 0); + + it->Next(); + + EXPECT_FALSE(it->IsValid()); +} + +TEST(LevelDBDatabaseTest, TransactionCommitTest) { + base::ScopedTempDir temp_directory; + ASSERT_TRUE(temp_directory.CreateUniqueTempDir()); + + const std::string key1("key1"); + const std::string key2("key2"); + const std::string value1("value1"); + const std::string value2("value2"); + const std::string value3("value3"); + + std::string put_value; + std::string got_value; + SimpleComparator comparator; + bool found; + + std::unique_ptr<LevelDBDatabase> leveldb; + LevelDBDatabase::Open(temp_directory.GetPath(), &comparator, &leveldb); + EXPECT_TRUE(leveldb); + + scoped_refptr<LevelDBTransaction> transaction = + new LevelDBTransaction(leveldb.get()); + + put_value = value1; + transaction->Put(key1, &put_value); + + put_value = value2; + transaction->Put(key2, &put_value); + + put_value = value3; + transaction->Put(key2, &put_value); + + leveldb::Status status = transaction->Commit(); + EXPECT_TRUE(status.ok()); + + status = leveldb->Get(key1, &got_value, &found); + EXPECT_TRUE(status.ok()); + EXPECT_TRUE(found); + EXPECT_EQ(value1, got_value); + + status = leveldb->Get(key2, &got_value, &found); + EXPECT_TRUE(status.ok()); + EXPECT_TRUE(found); + EXPECT_EQ(value3, got_value); +} + TEST(LevelDB, Locking) { base::ScopedTempDir temp_directory; ASSERT_TRUE(temp_directory.CreateUniqueTempDir());
diff --git a/content/test/BUILD.gn b/content/test/BUILD.gn index b8cab5f..db2a88e 100644 --- a/content/test/BUILD.gn +++ b/content/test/BUILD.gn
@@ -1117,7 +1117,6 @@ "../browser/indexed_db/indexed_db_quota_client_unittest.cc", "../browser/indexed_db/indexed_db_transaction_unittest.cc", "../browser/indexed_db/indexed_db_unittest.cc", - "../browser/indexed_db/leveldb/leveldb_transaction_unittest.cc", "../browser/indexed_db/leveldb/leveldb_unittest.cc", "../browser/indexed_db/leveldb/mock_leveldb_factory.cc", "../browser/indexed_db/leveldb/mock_leveldb_factory.h",
diff --git a/ios/chrome/browser/tabs/tab_model.h b/ios/chrome/browser/tabs/tab_model.h index 766eeeb..fa528de 100644 --- a/ios/chrome/browser/tabs/tab_model.h +++ b/ios/chrome/browser/tabs/tab_model.h
@@ -242,6 +242,7 @@ // Notifies observers that the given |tab| was changed. - (void)notifyTabChanged:(Tab*)tab; + // Notifies observers that the snapshot for the given |tab| changed was changed // to |image|. - (void)notifyTabSnapshotChanged:(Tab*)tab withImage:(UIImage*)image;
diff --git a/ios/chrome/browser/tabs/tab_model.mm b/ios/chrome/browser/tabs/tab_model.mm index 65d1835..732ce15 100644 --- a/ios/chrome/browser/tabs/tab_model.mm +++ b/ios/chrome/browser/tabs/tab_model.mm
@@ -117,6 +117,23 @@ base::Unretained(web_state_list))); } +// Internal helper function returning the opener for a given Tab by +// checking the associated Tab tabId (should be removed once the opener +// is passed to the insertTab:atIndex: and replaceTab:withTab: methods). +Tab* GetOpenerForTab(id<NSFastEnumeration> tabs, Tab* tab) { + NSString* opener_id = + [tab navigationManager]->GetSessionController().openerId; + if (!opener_id) + return nullptr; + + for (Tab* currentTab in tabs) { + if ([opener_id isEqualToString:currentTab.tabId]) + return currentTab; + } + + return nullptr; +} + } // anonymous namespace @interface TabModelWebStateProxyFactory : NSObject<WebStateProxyFactory> @@ -217,6 +234,12 @@ - (BOOL)restoreSessionWindow:(SessionWindowIOS*)window persistState:(BOOL)persistState; +// Helper method to insert |tab| at the given |index| recording |parentTab| as +// the opener. Broadcasts the proper notifications about the change. The +// receiver should be set as the parentTabModel for |tab|; this method doesn't +// check that. +- (void)insertTab:(Tab*)tab atIndex:(NSUInteger)index opener:(Tab*)parentTab; + @end @implementation TabModel @@ -405,68 +428,43 @@ } - (Tab*)nextTabWithOpener:(Tab*)tab afterTab:(Tab*)afterTab { - NSUInteger startIndex = NSNotFound; - // Start looking after |afterTab|. If it's not found, start looking after - // |tab|. If it's not found either, bail. + int startIndex = WebStateList::kInvalidIndex; if (afterTab) - startIndex = [self indexOfTab:afterTab]; - if (startIndex == NSNotFound) - startIndex = [self indexOfTab:tab]; - if (startIndex == NSNotFound) + startIndex = _webStateList.GetIndexOfWebState(afterTab.webState); + + if (startIndex == WebStateList::kInvalidIndex) + startIndex = _webStateList.GetIndexOfWebState(tab.webState); + + const int index = _webStateList.GetIndexOfNextWebStateOpenedBy( + tab.webState, startIndex, false); + if (index == WebStateList::kInvalidIndex) return nil; - NSString* parentID = tab.tabId; - for (NSUInteger i = startIndex + 1; i < self.count; ++i) { - Tab* current = [self tabAtIndex:i]; - DCHECK([current navigationManager]); - CRWSessionController* sessionController = - [current navigationManager]->GetSessionController(); - if ([sessionController.openerId isEqualToString:parentID]) - return current; - } - return nil; + + DCHECK_GE(index, 0); + return [self tabAtIndex:static_cast<NSUInteger>(index)]; } - (Tab*)lastTabWithOpener:(Tab*)tab { - NSUInteger startIndex = [self indexOfTab:tab]; - if (startIndex == NSNotFound) + int startIndex = _webStateList.GetIndexOfWebState(tab.webState); + if (startIndex == WebStateList::kInvalidIndex) return nil; - // There is at least one tab in the model, because otherwise the above check - // would have returned. - NSString* parentID = tab.tabId; - DCHECK([tab navigationManager]); - NSInteger parentNavIndex = [tab navigationManager]->GetCurrentItemIndex(); - Tab* match = nil; - // Find the last tab in the first matching 'group'. A 'group' is a set of - // tabs whose opener's id and opener's navigation index match. The navigation - // index is used in addition to the session id to detect navigations changes - // within the same session. - for (NSUInteger i = startIndex + 1; i < self.count; ++i) { - Tab* tabToCheck = [self tabAtIndex:i]; - DCHECK([tabToCheck navigationManager]); - CRWSessionController* sessionController = - [tabToCheck navigationManager]->GetSessionController(); - if ([sessionController.openerId isEqualToString:parentID] && - sessionController.openerNavigationIndex == parentNavIndex) { - match = tabToCheck; - } else if (match) { - break; - } - } - return match; + const int index = _webStateList.GetIndexOfLastWebStateOpenedBy( + tab.webState, startIndex, true); + if (index == WebStateList::kInvalidIndex) + return nil; + + DCHECK_GE(index, 0); + return [self tabAtIndex:static_cast<NSUInteger>(index)]; } - (Tab*)openerOfTab:(Tab*)tab { - if (![tab navigationManager]) + int index = _webStateList.GetIndexOfWebState(tab.webState); + if (index == WebStateList::kInvalidIndex) return nil; - NSString* openerId = [tab navigationManager]->GetSessionController().openerId; - if (!openerId.length) // Short-circuit if opener is empty. - return nil; - for (Tab* iteratedTab in self) { - if ([iteratedTab.tabId isEqualToString:openerId]) - return iteratedTab; - } - return nil; + + web::WebState* opener = _webStateList.GetOpenerOfWebStateAt(index); + return opener ? LegacyTabHelper::GetTabForWebState(opener) : nil; } - (Tab*)insertOrUpdateTabWithURL:(const GURL&)URL @@ -553,13 +551,14 @@ return tab; } -- (void)insertTab:(Tab*)tab atIndex:(NSUInteger)index { +- (void)insertTab:(Tab*)tab atIndex:(NSUInteger)index opener:(Tab*)parentTab { DCHECK(tab); DCHECK(![_tabRetainer containsObject:tab]); DCHECK_LE(index, static_cast<NSUInteger>(INT_MAX)); [_tabRetainer addObject:tab]; - _webStateList.InsertWebState(static_cast<int>(index), tab.webState); + _webStateList.InsertWebState(static_cast<int>(index), tab.webState, + parentTab.webState); // Persist the session due to a new tab being inserted. If this is a // background tab (will not become active), saving now will capture the @@ -570,6 +569,10 @@ ++_newTabCount; } +- (void)insertTab:(Tab*)tab atIndex:(NSUInteger)index { + [self insertTab:tab atIndex:index opener:GetOpenerForTab(self, tab)]; +} + - (void)moveTab:(Tab*)tab toIndex:(NSUInteger)toIndex { DCHECK([_tabRetainer containsObject:tab]); DCHECK_LE(toIndex, static_cast<NSUInteger>(INT_MAX)); @@ -590,7 +593,8 @@ [_tabRetainer addObject:newTab]; [newTab setParentTabModel:self]; - _webStateList.ReplaceWebStateAt(index, newTab.webState); + _webStateList.ReplaceWebStateAt(index, newTab.webState, + GetOpenerForTab(self, newTab).webState); if (self.currentTab == oldTab) [self changeSelectedTabFrom:nil to:newTab persistState:NO]; @@ -889,7 +893,7 @@ // TODO(crbug.com/661988): Make this work. } - [self insertTab:tab atIndex:index]; + [self insertTab:tab atIndex:index opener:parentTab]; if (!inBackground && _tabUsageRecorder) _tabUsageRecorder->TabCreatedForSelection(tab); @@ -986,20 +990,43 @@ scoped_refptr<web::CertificatePolicyCache> policyCache = web::BrowserState::GetCertificatePolicyCache(_browserState); + base::scoped_nsobject<NSMutableArray<Tab*>> restoredTabs( + [[NSMutableArray alloc] initWithCapacity:sessions.count]); + + // Recreate all the restored Tabs and add them to the WebStateList without + // any opener-opened relationship (as the n-th restored Tab opener may be + // at an index larger than n). Then in a second pass fix the openers. for (CRWSessionStorage* session in sessions) { std::unique_ptr<web::WebState> webState = web::WebState::Create(params, session); - DCHECK_EQ(webState->GetBrowserState(), _browserState); - Tab* tab = - [self insertTabWithWebState:std::move(webState) atIndex:self.count]; - tab.webController.usePlaceholderOverlay = YES; + base::scoped_nsobject<Tab> tab( + [[Tab alloc] initWithWebState:std::move(webState) model:self]); + [tab webController].webUsageEnabled = webUsageEnabled_; + [tab webController].usePlaceholderOverlay = YES; // Restore the CertificatePolicyCache (note that webState is invalid after // passing it via move semantic to -insertTabWithWebState:atIndex:). - UpdateCertificatePolicyCacheFromWebState(policyCache, tab.webState); + UpdateCertificatePolicyCacheFromWebState(policyCache, [tab webState]); + [self insertTab:tab atIndex:self.count opener:nil]; + [restoredTabs addObject:tab.get()]; } + + DCHECK_EQ(sessions.count, [restoredTabs count]); DCHECK_GT(_webStateList.count(), oldCount); + // Fix openers now that all Tabs have been restored. Only look for an opener + // Tab in the newly restored Tabs and not in the already open Tabs. + for (int index = oldCount; index < _webStateList.count(); ++index) { + DCHECK_GE(index, oldCount); + NSUInteger tabIndex = static_cast<NSUInteger>(index - oldCount); + Tab* tab = [restoredTabs objectAtIndex:tabIndex]; + Tab* opener = GetOpenerForTab(restoredTabs.get(), tab); + if (opener) { + DCHECK(opener.webState); + _webStateList.SetOpenerOfWebStateAt(index, opener.webState); + } + } + // Update the selected tab if there was a selected Tab in the saved session. if (window.selectedIndex != NSNotFound) { NSUInteger selectedIndex = window.selectedIndex + oldCount; @@ -1107,7 +1134,7 @@ params.transition_type = ui::PAGE_TRANSITION_TYPED; [[tab webController] loadWithParams:params]; [tab webController].webUsageEnabled = webUsageEnabled_; - [self insertTab:tab atIndex:index]; + [self insertTab:tab atIndex:index opener:parentTab]; return tab; }
diff --git a/ios/chrome/browser/tabs/tab_model_unittest.mm b/ios/chrome/browser/tabs/tab_model_unittest.mm index c28266e..5c06b91 100644 --- a/ios/chrome/browser/tabs/tab_model_unittest.mm +++ b/ios/chrome/browser/tabs/tab_model_unittest.mm
@@ -838,11 +838,24 @@ browserState:chrome_browser_state.get()]); [model addTabWithURL:kURL referrer:kReferrer windowName:@"window 1"]; - [model addTabWithURL:kURL referrer:kReferrer windowName:@"window 2"]; - [model addTabWithURL:kURL referrer:kReferrer windowName:@"window 3"]; + [model insertTabWithURL:kURL + referrer:kReferrer + windowName:@"window 3" + opener:[model tabAtIndex:0] + atIndex:[model count]]; + [model insertTabWithURL:kURL + referrer:kReferrer + windowName:@"window 3" + opener:[model tabAtIndex:1] + atIndex:0]; ASSERT_EQ(3U, [model count]); - model.get().currentTab = [model tabAtIndex:1]; + [model setCurrentTab:[model tabAtIndex:1]]; + + EXPECT_EQ(nil, [model openerOfTab:[model tabAtIndex:1]]); + EXPECT_EQ([model tabAtIndex:1], [model openerOfTab:[model tabAtIndex:2]]); + EXPECT_EQ([model tabAtIndex:2], [model openerOfTab:[model tabAtIndex:0]]); + // Force state to flush to disk on the main thread so it can be immediately // tested below. SessionWindowIOS* window = [model windowForSavingSession]; @@ -866,7 +879,13 @@ initWithSessionWindow:sessionWindow sessionService:test_service browserState:chrome_browser_state.get()]); - EXPECT_EQ(model.get().currentTab, [model tabAtIndex:1]); + ASSERT_EQ(3u, [model count]); + + EXPECT_EQ([model tabAtIndex:1], [model currentTab]); + EXPECT_EQ(nil, [model openerOfTab:[model tabAtIndex:1]]); + EXPECT_EQ([model tabAtIndex:1], [model openerOfTab:[model tabAtIndex:2]]); + EXPECT_EQ([model tabAtIndex:2], [model openerOfTab:[model tabAtIndex:0]]); + [model browserStateDestroyed]; model.reset();
diff --git a/ios/public/provider/chrome/browser/chrome_browser_provider.h b/ios/public/provider/chrome/browser/chrome_browser_provider.h index dc7b9992..bd849e2 100644 --- a/ios/public/provider/chrome/browser/chrome_browser_provider.h +++ b/ios/public/provider/chrome/browser/chrome_browser_provider.h
@@ -22,6 +22,10 @@ class UserFeedbackProvider; class VoiceSearchProvider; +namespace base { +class CommandLine; +} + namespace web { class WebState; } @@ -64,6 +68,11 @@ ChromeBrowserProvider(); virtual ~ChromeBrowserProvider(); + // Appends additional command-line flags. Called before web startup. + virtual void AppendSwitchesFromExperimentalSettings( + NSUserDefaults* experimental_settings, + base::CommandLine* command_line) const; + // This is called after web startup. virtual void Initialize() const;
diff --git a/ios/public/provider/chrome/browser/chrome_browser_provider.mm b/ios/public/provider/chrome/browser/chrome_browser_provider.mm index 4b04c6a..906ac74b 100644 --- a/ios/public/provider/chrome/browser/chrome_browser_provider.mm +++ b/ios/public/provider/chrome/browser/chrome_browser_provider.mm
@@ -29,6 +29,10 @@ ChromeBrowserProvider::~ChromeBrowserProvider() {} +void ChromeBrowserProvider::AppendSwitchesFromExperimentalSettings( + NSUserDefaults* experimental_settings, + base::CommandLine* command_line) const {} + void ChromeBrowserProvider::Initialize() const {} SigninErrorProvider* ChromeBrowserProvider::GetSigninErrorProvider() {
diff --git a/ios/shared/chrome/browser/tabs/web_state_list.h b/ios/shared/chrome/browser/tabs/web_state_list.h index 5964e01..3916076 100644 --- a/ios/shared/chrome/browser/tabs/web_state_list.h +++ b/ios/shared/chrome/browser/tabs/web_state_list.h
@@ -47,16 +47,45 @@ // WebState is not in the model. int GetIndexOfWebState(const web::WebState* web_state) const; - // Inserts the specified WebState at the specified index. - void InsertWebState(int index, web::WebState* web_state); + // Returns the WebState that opened the WebState at the specified index or + // null if there is no opener on record. + web::WebState* GetOpenerOfWebStateAt(int index) const; + + // Sets the opener for WebState at the specified index. The |opener| must be + // in the WebStateList. + void SetOpenerOfWebStateAt(int index, web::WebState* opener); + + // Returns the index of the next WebState in the sequence of WebStates opened + // from the specified WebState after |start_index|, or kInvalidIndex if there + // are no such WebState. If |use_group| is true, the opener's navigation index + // is used to detect navigation changes within the same session. + int GetIndexOfNextWebStateOpenedBy(const web::WebState* opener, + int start_index, + bool use_group) const; + + // Returns the index of the last WebState in the sequence of WebStates opened + // from the specified WebState after |start_index|, or kInvalidIndex if there + // are no such WebState. If |use_group| is true, the opener's navigation index + // is used to detect navigation changes within the same session. + int GetIndexOfLastWebStateOpenedBy(const web::WebState* opener, + int start_index, + bool use_group) const; + + // Inserts the specified WebState at the specified index with an optional + // opener (null if there is no opener). + void InsertWebState(int index, + web::WebState* web_state, + web::WebState* opener); // Moves the WebState at the specified index to another index. void MoveWebStateAt(int from_index, int to_index); // Replaces the WebState at the specified index with new WebState. Returns // the old WebState at that index to the caller (abandon ownership of the - // returned WebState). - web::WebState* ReplaceWebStateAt(int index, web::WebState* web_state); + // returned WebState). An optional opener for the new WebState may be passed. + web::WebState* ReplaceWebStateAt(int index, + web::WebState* web_state, + web::WebState* opener); // Detaches the WebState at the specified index. void DetachWebStateAt(int index); @@ -71,6 +100,20 @@ static const int kInvalidIndex = -1; private: + // Sets the opener of any WebState that reference the WebState at the + // specified index to null. + void ClearOpenersReferencing(int index); + + // Returns the index of the |n|-th WebState (with n > 0) in the sequence of + // WebStates opened from the specified WebState after |start_index|, or + // kInvalidIndex if there are no such WebState. If |use_group| is true, the + // opener's navigation index is used to detect navigation changes within the + // same session. + int GetIndexOfNthWebStateOpenedBy(const web::WebState* opener, + int start_index, + bool use_group, + int n) const; + class WebStateWrapper; const WebStateOwnership web_state_ownership_; std::vector<std::unique_ptr<WebStateWrapper>> web_state_wrappers_;
diff --git a/ios/shared/chrome/browser/tabs/web_state_list.mm b/ios/shared/chrome/browser/tabs/web_state_list.mm index cb85ed4..283ddee 100644 --- a/ios/shared/chrome/browser/tabs/web_state_list.mm +++ b/ios/shared/chrome/browser/tabs/web_state_list.mm
@@ -4,9 +4,12 @@ #import "ios/shared/chrome/browser/tabs/web_state_list.h" +#include <utility> + #include "base/logging.h" #include "base/memory/ptr_util.h" #import "ios/shared/chrome/browser/tabs/web_state_list_observer.h" +#import "ios/web/public/navigation_manager.h" #import "ios/web/public/web_state/web_state.h" #if !defined(__has_feature) || !__has_feature(objc_arc) @@ -22,10 +25,27 @@ ~WebStateWrapper(); web::WebState* web_state() const { return web_state_; } - void set_web_state(web::WebState* web_state) { web_state_ = web_state; } + web::WebState* opener() const { return opener_; } + + // Replaces the wrapped WebState (and clear associated state) and returns the + // old WebState after forfeiting ownership. + web::WebState* ReplaceWebState(web::WebState* web_state); + + // Sets the opener for the wrapped WebState and record the opener navigation + // index to allow detecting navigation changes during the same session. + void SetOpener(web::WebState* opener); + + // Returns whether |opener| spawned the wrapped WebState. If |use_group| is + // true, also use the opener navigation index to detect navigation changes + // during the same session. + bool WasOpenedBy(const web::WebState* opener, + int opener_navigation_index, + bool use_group) const; private: web::WebState* web_state_; + web::WebState* opener_ = nullptr; + int opener_last_committed_index_; const bool has_web_state_ownership_; DISALLOW_COPY_AND_ASSIGN(WebStateWrapper); @@ -33,13 +53,45 @@ WebStateList::WebStateWrapper::WebStateWrapper(web::WebState* web_state, bool assume_ownership) - : web_state_(web_state), has_web_state_ownership_(assume_ownership) {} + : web_state_(web_state), has_web_state_ownership_(assume_ownership) { + DCHECK(web_state_); +} WebStateList::WebStateWrapper::~WebStateWrapper() { if (has_web_state_ownership_) delete web_state_; } +web::WebState* WebStateList::WebStateWrapper::ReplaceWebState( + web::WebState* web_state) { + DCHECK(web_state); + DCHECK_NE(web_state, web_state_); + std::swap(web_state, web_state_); + opener_ = nullptr; + return web_state; +} + +void WebStateList::WebStateWrapper::SetOpener(web::WebState* opener) { + opener_ = opener; + if (opener_) { + opener_last_committed_index_ = + opener_->GetNavigationManager()->GetLastCommittedItemIndex(); + } +} + +bool WebStateList::WebStateWrapper::WasOpenedBy(const web::WebState* opener, + int opener_navigation_index, + bool use_group) const { + DCHECK(opener); + if (opener_ != opener) + return false; + + if (!use_group) + return true; + + return opener_last_committed_index_ == opener_navigation_index; +} + WebStateList::WebStateList(WebStateOwnership ownership) : web_state_ownership_(ownership) {} @@ -62,13 +114,41 @@ return kInvalidIndex; } -void WebStateList::InsertWebState(int index, web::WebState* web_state) { +web::WebState* WebStateList::GetOpenerOfWebStateAt(int index) const { + DCHECK(ContainsIndex(index)); + return web_state_wrappers_[index]->opener(); +} + +void WebStateList::SetOpenerOfWebStateAt(int index, web::WebState* opener) { + DCHECK(ContainsIndex(index)); + DCHECK(ContainsIndex(GetIndexOfWebState(opener))); + web_state_wrappers_[index]->SetOpener(opener); +} + +int WebStateList::GetIndexOfNextWebStateOpenedBy(const web::WebState* opener, + int start_index, + bool use_group) const { + return GetIndexOfNthWebStateOpenedBy(opener, start_index, use_group, 1); +} + +int WebStateList::GetIndexOfLastWebStateOpenedBy(const web::WebState* opener, + int start_index, + bool use_group) const { + return GetIndexOfNthWebStateOpenedBy(opener, start_index, use_group, INT_MAX); +} + +void WebStateList::InsertWebState(int index, + web::WebState* web_state, + web::WebState* opener) { DCHECK(ContainsIndex(index) || index == count()); web_state_wrappers_.insert( web_state_wrappers_.begin() + index, base::MakeUnique<WebStateWrapper>(web_state, web_state_ownership_ == WebStateOwned)); + if (opener) + SetOpenerOfWebStateAt(index, opener); + for (auto& observer : observers_) observer.WebStateInsertedAt(this, web_state, index); } @@ -91,10 +171,16 @@ } web::WebState* WebStateList::ReplaceWebStateAt(int index, - web::WebState* web_state) { + web::WebState* web_state, + web::WebState* opener) { DCHECK(ContainsIndex(index)); - web::WebState* old_web_state = web_state_wrappers_[index]->web_state(); - web_state_wrappers_[index]->set_web_state(web_state); + ClearOpenersReferencing(index); + + auto& web_state_wrapper = web_state_wrappers_[index]; + web::WebState* old_web_state = web_state_wrapper->ReplaceWebState(web_state); + + if (opener && opener != old_web_state) + SetOpenerOfWebStateAt(index, opener); for (auto& observer : observers_) observer.WebStateReplacedAt(this, old_web_state, web_state, index); @@ -104,6 +190,8 @@ void WebStateList::DetachWebStateAt(int index) { DCHECK(ContainsIndex(index)); + ClearOpenersReferencing(index); + web::WebState* web_state = web_state_wrappers_[index]->web_state(); web_state_wrappers_.erase(web_state_wrappers_.begin() + index); @@ -119,5 +207,38 @@ observers_.RemoveObserver(observer); } +void WebStateList::ClearOpenersReferencing(int index) { + web::WebState* old_web_state = web_state_wrappers_[index]->web_state(); + for (auto& web_state_wrapper : web_state_wrappers_) { + if (web_state_wrapper->opener() == old_web_state) + web_state_wrapper->SetOpener(nullptr); + } +} + +int WebStateList::GetIndexOfNthWebStateOpenedBy(const web::WebState* opener, + int start_index, + bool use_group, + int n) const { + DCHECK_GT(n, 0); + if (!opener || !ContainsIndex(start_index) || start_index == INT_MAX) + return kInvalidIndex; + + const int opener_navigation_index = + use_group ? opener->GetNavigationManager()->GetCurrentItemIndex() : -1; + + int found_index = kInvalidIndex; + for (int index = start_index + 1; index < count() && n; ++index) { + if (web_state_wrappers_[index]->WasOpenedBy(opener, opener_navigation_index, + use_group)) { + found_index = index; + --n; + } else if (found_index != kInvalidIndex) { + return found_index; + } + } + + return found_index; +} + // static const int WebStateList::kInvalidIndex;
diff --git a/ios/shared/chrome/browser/tabs/web_state_list_fast_enumeration_helper_unittest.mm b/ios/shared/chrome/browser/tabs/web_state_list_fast_enumeration_helper_unittest.mm index 6f17ea5..58fb791 100644 --- a/ios/shared/chrome/browser/tabs/web_state_list_fast_enumeration_helper_unittest.mm +++ b/ios/shared/chrome/browser/tabs/web_state_list_fast_enumeration_helper_unittest.mm
@@ -67,7 +67,7 @@ test_web_state->SetCurrentURL(GURL(urls[index])); web_state_list_.InsertWebState(web_state_list_.count(), - test_web_state.release()); + test_web_state.release(), nullptr); } }
diff --git a/ios/shared/chrome/browser/tabs/web_state_list_unittest.mm b/ios/shared/chrome/browser/tabs/web_state_list_unittest.mm index 1c0ed21..2ed665f 100644 --- a/ios/shared/chrome/browser/tabs/web_state_list_unittest.mm +++ b/ios/shared/chrome/browser/tabs/web_state_list_unittest.mm
@@ -8,6 +8,7 @@ #include "base/memory/ptr_util.h" #include "base/supports_user_data.h" #import "ios/shared/chrome/browser/tabs/web_state_list_observer.h" +#import "ios/web/public/test/fakes/test_navigation_manager.h" #import "ios/web/public/test/fakes/test_web_state.h" #include "testing/gtest/include/gtest/gtest.h" #include "testing/platform_test.h" @@ -97,6 +98,39 @@ DISALLOW_COPY_AND_ASSIGN(WebStateListTestObserver); }; + +// A fake NavigationManager used to test opener-opened relationship in the +// WebStateList. +class FakeNavigationManer : public web::TestNavigationManager { + public: + FakeNavigationManer() = default; + + // web::NavigationManager implementation. + int GetCurrentItemIndex() const override { return current_item_index_; } + + int GetLastCommittedItemIndex() const override { return current_item_index_; } + + bool CanGoBack() const override { return current_item_index_ > 0; } + + bool CanGoForward() const override { return current_item_index_ < INT_MAX; } + + void GoBack() override { + DCHECK(CanGoBack()); + --current_item_index_; + } + + void GoForward() override { + DCHECK(CanGoForward()); + ++current_item_index_; + } + + void GoToIndex(int index) override { current_item_index_ = index; } + + int current_item_index_ = 0; + + DISALLOW_COPY_AND_ASSIGN(FakeNavigationManer); +}; + } // namespace class WebStateListTest : public PlatformTest { @@ -117,6 +151,8 @@ web::WebState* CreateWebState(const char* url) { auto test_web_state = base::MakeUnique<web::TestWebState>(); test_web_state->SetCurrentURL(GURL(url)); + test_web_state->SetNavigationManager( + base::MakeUnique<FakeNavigationManer>()); return test_web_state.release(); } @@ -128,7 +164,7 @@ EXPECT_EQ(0, web_state_list_.count()); EXPECT_TRUE(web_state_list_.empty()); - web_state_list_.InsertWebState(0, CreateWebState(kURL0)); + web_state_list_.InsertWebState(0, CreateWebState(kURL0), nullptr); EXPECT_TRUE(observer_.web_state_inserted_called()); EXPECT_EQ(1, web_state_list_.count()); @@ -136,7 +172,7 @@ } TEST_F(WebStateListTest, InsertUrlSingle) { - web_state_list_.InsertWebState(0, CreateWebState(kURL0)); + web_state_list_.InsertWebState(0, CreateWebState(kURL0), nullptr); EXPECT_TRUE(observer_.web_state_inserted_called()); EXPECT_EQ(1, web_state_list_.count()); @@ -144,9 +180,9 @@ } TEST_F(WebStateListTest, InsertUrlMultiple) { - web_state_list_.InsertWebState(0, CreateWebState(kURL0)); - web_state_list_.InsertWebState(0, CreateWebState(kURL1)); - web_state_list_.InsertWebState(1, CreateWebState(kURL2)); + web_state_list_.InsertWebState(0, CreateWebState(kURL0), nullptr); + web_state_list_.InsertWebState(0, CreateWebState(kURL1), nullptr); + web_state_list_.InsertWebState(1, CreateWebState(kURL2), nullptr); EXPECT_TRUE(observer_.web_state_inserted_called()); EXPECT_EQ(3, web_state_list_.count()); @@ -156,9 +192,9 @@ } TEST_F(WebStateListTest, MoveWebStateAtRightByOne) { - web_state_list_.InsertWebState(0, CreateWebState(kURL0)); - web_state_list_.InsertWebState(1, CreateWebState(kURL1)); - web_state_list_.InsertWebState(2, CreateWebState(kURL2)); + web_state_list_.InsertWebState(0, CreateWebState(kURL0), nullptr); + web_state_list_.InsertWebState(1, CreateWebState(kURL1), nullptr); + web_state_list_.InsertWebState(2, CreateWebState(kURL2), nullptr); // Sanity check before closing WebState. EXPECT_EQ(3, web_state_list_.count()); @@ -177,9 +213,9 @@ } TEST_F(WebStateListTest, MoveWebStateAtRightByMoreThanOne) { - web_state_list_.InsertWebState(0, CreateWebState(kURL0)); - web_state_list_.InsertWebState(1, CreateWebState(kURL1)); - web_state_list_.InsertWebState(2, CreateWebState(kURL2)); + web_state_list_.InsertWebState(0, CreateWebState(kURL0), nullptr); + web_state_list_.InsertWebState(1, CreateWebState(kURL1), nullptr); + web_state_list_.InsertWebState(2, CreateWebState(kURL2), nullptr); // Sanity check before closing WebState. EXPECT_EQ(3, web_state_list_.count()); @@ -198,9 +234,9 @@ } TEST_F(WebStateListTest, MoveWebStateAtLeftByOne) { - web_state_list_.InsertWebState(0, CreateWebState(kURL0)); - web_state_list_.InsertWebState(1, CreateWebState(kURL1)); - web_state_list_.InsertWebState(2, CreateWebState(kURL2)); + web_state_list_.InsertWebState(0, CreateWebState(kURL0), nullptr); + web_state_list_.InsertWebState(1, CreateWebState(kURL1), nullptr); + web_state_list_.InsertWebState(2, CreateWebState(kURL2), nullptr); // Sanity check before closing WebState. EXPECT_EQ(3, web_state_list_.count()); @@ -219,9 +255,9 @@ } TEST_F(WebStateListTest, MoveWebStateAtLeftByMoreThanOne) { - web_state_list_.InsertWebState(0, CreateWebState(kURL0)); - web_state_list_.InsertWebState(1, CreateWebState(kURL1)); - web_state_list_.InsertWebState(2, CreateWebState(kURL2)); + web_state_list_.InsertWebState(0, CreateWebState(kURL0), nullptr); + web_state_list_.InsertWebState(1, CreateWebState(kURL1), nullptr); + web_state_list_.InsertWebState(2, CreateWebState(kURL2), nullptr); // Sanity check before closing WebState. EXPECT_EQ(3, web_state_list_.count()); @@ -240,9 +276,9 @@ } TEST_F(WebStateListTest, MoveWebStateAtSameIndex) { - web_state_list_.InsertWebState(0, CreateWebState(kURL0)); - web_state_list_.InsertWebState(1, CreateWebState(kURL1)); - web_state_list_.InsertWebState(2, CreateWebState(kURL2)); + web_state_list_.InsertWebState(0, CreateWebState(kURL0), nullptr); + web_state_list_.InsertWebState(1, CreateWebState(kURL1), nullptr); + web_state_list_.InsertWebState(2, CreateWebState(kURL2), nullptr); // Sanity check before closing WebState. EXPECT_EQ(3, web_state_list_.count()); @@ -261,8 +297,8 @@ } TEST_F(WebStateListTest, ReplaceWebStateAt) { - web_state_list_.InsertWebState(0, CreateWebState(kURL0)); - web_state_list_.InsertWebState(1, CreateWebState(kURL1)); + web_state_list_.InsertWebState(0, CreateWebState(kURL0), nullptr); + web_state_list_.InsertWebState(1, CreateWebState(kURL1), nullptr); // Sanity check before replacing WebState. EXPECT_EQ(2, web_state_list_.count()); @@ -271,7 +307,7 @@ observer_.ResetStatistics(); std::unique_ptr<web::WebState> old_web_state( - web_state_list_.ReplaceWebStateAt(1, CreateWebState(kURL2))); + web_state_list_.ReplaceWebStateAt(1, CreateWebState(kURL2), nullptr)); EXPECT_TRUE(observer_.web_state_replaced_called()); EXPECT_EQ(2, web_state_list_.count()); @@ -281,9 +317,9 @@ } TEST_F(WebStateListTest, DetachWebStateAtIndexBegining) { - web_state_list_.InsertWebState(0, CreateWebState(kURL0)); - web_state_list_.InsertWebState(1, CreateWebState(kURL1)); - web_state_list_.InsertWebState(2, CreateWebState(kURL2)); + web_state_list_.InsertWebState(0, CreateWebState(kURL0), nullptr); + web_state_list_.InsertWebState(1, CreateWebState(kURL1), nullptr); + web_state_list_.InsertWebState(2, CreateWebState(kURL2), nullptr); // Sanity check before closing WebState. EXPECT_EQ(3, web_state_list_.count()); @@ -301,9 +337,9 @@ } TEST_F(WebStateListTest, DetachWebStateAtIndexMiddle) { - web_state_list_.InsertWebState(0, CreateWebState(kURL0)); - web_state_list_.InsertWebState(1, CreateWebState(kURL1)); - web_state_list_.InsertWebState(2, CreateWebState(kURL2)); + web_state_list_.InsertWebState(0, CreateWebState(kURL0), nullptr); + web_state_list_.InsertWebState(1, CreateWebState(kURL1), nullptr); + web_state_list_.InsertWebState(2, CreateWebState(kURL2), nullptr); // Sanity check before closing WebState. EXPECT_EQ(3, web_state_list_.count()); @@ -321,9 +357,9 @@ } TEST_F(WebStateListTest, DetachWebStateAtIndexLast) { - web_state_list_.InsertWebState(0, CreateWebState(kURL0)); - web_state_list_.InsertWebState(1, CreateWebState(kURL1)); - web_state_list_.InsertWebState(2, CreateWebState(kURL2)); + web_state_list_.InsertWebState(0, CreateWebState(kURL0), nullptr); + web_state_list_.InsertWebState(1, CreateWebState(kURL1), nullptr); + web_state_list_.InsertWebState(2, CreateWebState(kURL2), nullptr); // Sanity check before closing WebState. EXPECT_EQ(3, web_state_list_.count()); @@ -349,7 +385,7 @@ auto web_state_list = base::MakeUnique<WebStateList>(WebStateList::WebStateBorrowed); - web_state_list->InsertWebState(0, test_web_state.get()); + web_state_list->InsertWebState(0, test_web_state.get(), nullptr); EXPECT_FALSE(web_state_was_killed); web_state_list.reset(); @@ -365,9 +401,133 @@ auto web_state_list = base::MakeUnique<WebStateList>(WebStateList::WebStateOwned); - web_state_list->InsertWebState(0, test_web_state.release()); + web_state_list->InsertWebState(0, test_web_state.release(), nullptr); EXPECT_FALSE(web_state_was_killed); web_state_list.reset(); EXPECT_TRUE(web_state_was_killed); } + +TEST_F(WebStateListTest, OpenersEmptyList) { + EXPECT_TRUE(web_state_list_.empty()); + + EXPECT_EQ(WebStateList::kInvalidIndex, + web_state_list_.GetIndexOfNextWebStateOpenedBy( + nullptr, WebStateList::kInvalidIndex, false)); + EXPECT_EQ(WebStateList::kInvalidIndex, + web_state_list_.GetIndexOfLastWebStateOpenedBy( + nullptr, WebStateList::kInvalidIndex, false)); + + EXPECT_EQ(WebStateList::kInvalidIndex, + web_state_list_.GetIndexOfNextWebStateOpenedBy( + nullptr, WebStateList::kInvalidIndex, true)); + EXPECT_EQ(WebStateList::kInvalidIndex, + web_state_list_.GetIndexOfLastWebStateOpenedBy( + nullptr, WebStateList::kInvalidIndex, true)); +} + +TEST_F(WebStateListTest, OpenersNothingOpened) { + web_state_list_.InsertWebState(0, CreateWebState(kURL0), nullptr); + web_state_list_.InsertWebState(1, CreateWebState(kURL1), nullptr); + web_state_list_.InsertWebState(2, CreateWebState(kURL2), nullptr); + + for (int index = 0; index < web_state_list_.count(); ++index) { + web::WebState* opener = web_state_list_.GetWebStateAt(index); + EXPECT_EQ( + WebStateList::kInvalidIndex, + web_state_list_.GetIndexOfNextWebStateOpenedBy(opener, index, false)); + EXPECT_EQ( + WebStateList::kInvalidIndex, + web_state_list_.GetIndexOfLastWebStateOpenedBy(opener, index, false)); + + EXPECT_EQ( + WebStateList::kInvalidIndex, + web_state_list_.GetIndexOfNextWebStateOpenedBy(opener, index, true)); + EXPECT_EQ( + WebStateList::kInvalidIndex, + web_state_list_.GetIndexOfLastWebStateOpenedBy(opener, index, true)); + } +} + +TEST_F(WebStateListTest, OpenersChildsAfterOpener) { + web_state_list_.InsertWebState(0, CreateWebState(kURL0), nullptr); + web::WebState* opener = web_state_list_.GetWebStateAt(0); + + web_state_list_.InsertWebState(1, CreateWebState(kURL1), opener); + web_state_list_.InsertWebState(2, CreateWebState(kURL2), opener); + + const int start_index = web_state_list_.GetIndexOfWebState(opener); + EXPECT_EQ(1, + web_state_list_.GetIndexOfNextWebStateOpenedBy(opener, start_index, + false)); + EXPECT_EQ(2, + web_state_list_.GetIndexOfLastWebStateOpenedBy(opener, start_index, + false)); + + EXPECT_EQ(1, + web_state_list_.GetIndexOfNextWebStateOpenedBy(opener, start_index, + true)); + EXPECT_EQ(2, + web_state_list_.GetIndexOfLastWebStateOpenedBy(opener, start_index, + true)); + + // Simulate a navigation on the opener, results should not change if not + // using groups, but should now be kInvalidIndex otherwise. + opener->GetNavigationManager()->GoForward(); + + EXPECT_EQ(1, + web_state_list_.GetIndexOfNextWebStateOpenedBy(opener, start_index, + false)); + EXPECT_EQ(2, + web_state_list_.GetIndexOfLastWebStateOpenedBy(opener, start_index, + false)); + + EXPECT_EQ(WebStateList::kInvalidIndex, + web_state_list_.GetIndexOfNextWebStateOpenedBy(opener, start_index, + true)); + EXPECT_EQ(WebStateList::kInvalidIndex, + web_state_list_.GetIndexOfLastWebStateOpenedBy(opener, start_index, + true)); + + // Add a new WebState with the same opener. It should be considered the next + // WebState if groups are considered and the last independently on whether + // groups are used or not. + web_state_list_.InsertWebState(3, CreateWebState(kURL2), opener); + + EXPECT_EQ(1, + web_state_list_.GetIndexOfNextWebStateOpenedBy(opener, start_index, + false)); + EXPECT_EQ(3, + web_state_list_.GetIndexOfLastWebStateOpenedBy(opener, start_index, + false)); + + EXPECT_EQ(3, + web_state_list_.GetIndexOfNextWebStateOpenedBy(opener, start_index, + true)); + EXPECT_EQ(3, + web_state_list_.GetIndexOfLastWebStateOpenedBy(opener, start_index, + true)); +} + +TEST_F(WebStateListTest, OpenersChildsBeforeOpener) { + web_state_list_.InsertWebState(0, CreateWebState(kURL0), nullptr); + web::WebState* opener = web_state_list_.GetWebStateAt(0); + + web_state_list_.InsertWebState(0, CreateWebState(kURL1), opener); + web_state_list_.InsertWebState(1, CreateWebState(kURL2), opener); + + const int start_index = web_state_list_.GetIndexOfWebState(opener); + EXPECT_EQ(WebStateList::kInvalidIndex, + web_state_list_.GetIndexOfNextWebStateOpenedBy(opener, start_index, + false)); + EXPECT_EQ(WebStateList::kInvalidIndex, + web_state_list_.GetIndexOfLastWebStateOpenedBy(opener, start_index, + false)); + + EXPECT_EQ(WebStateList::kInvalidIndex, + web_state_list_.GetIndexOfNextWebStateOpenedBy(opener, start_index, + true)); + EXPECT_EQ(WebStateList::kInvalidIndex, + web_state_list_.GetIndexOfLastWebStateOpenedBy(opener, start_index, + true)); +}
diff --git a/services/ui/ws/BUILD.gn b/services/ui/ws/BUILD.gn index 56bcc46..76368748 100644 --- a/services/ui/ws/BUILD.gn +++ b/services/ui/ws/BUILD.gn
@@ -213,6 +213,7 @@ "event_dispatcher_unittest.cc", "event_matcher_unittest.cc", "focus_controller_unittest.cc", + "frame_generator_unittest.cc", "server_window_compositor_frame_sink_manager_test_api.cc", "server_window_compositor_frame_sink_manager_test_api.h", "server_window_drawn_tracker_unittest.cc",
diff --git a/services/ui/ws/frame_generator.cc b/services/ui/ws/frame_generator.cc index 505ad37..9dc197d 100644 --- a/services/ui/ws/frame_generator.cc +++ b/services/ui/ws/frame_generator.cc
@@ -23,10 +23,27 @@ namespace ws { FrameGenerator::FrameGenerator(FrameGeneratorDelegate* delegate, - ServerWindow* root_window, - gfx::AcceleratedWidget widget) - : delegate_(delegate), root_window_(root_window), binding_(this) { + ServerWindow* root_window) + : delegate_(delegate), + root_window_(root_window), + binding_(this) { DCHECK(delegate_); +} + +void FrameGenerator::SetDeviceScaleFactor(float device_scale_factor) { + if (device_scale_factor_ == device_scale_factor) + return; + device_scale_factor_ = device_scale_factor; + if (compositor_frame_sink_) + compositor_frame_sink_->SetNeedsBeginFrame(true); +} + +FrameGenerator::~FrameGenerator() { + compositor_frame_sink_.reset(); +} + +void FrameGenerator::OnAcceleratedWidgetAvailable( + gfx::AcceleratedWidget widget) { DCHECK_NE(gfx::kNullAcceleratedWidget, widget); cc::mojom::MojoCompositorFrameSinkAssociatedRequest sink_request = mojo::MakeRequest(&compositor_frame_sink_); @@ -37,15 +54,6 @@ std::move(display_request)); } -FrameGenerator::~FrameGenerator() = default; - -void FrameGenerator::SetDeviceScaleFactor(float device_scale_factor) { - if (device_scale_factor_ == device_scale_factor) - return; - device_scale_factor_ = device_scale_factor; - compositor_frame_sink_->SetNeedsBeginFrame(true); -} - void FrameGenerator::OnSurfaceCreated(const cc::SurfaceInfo& surface_info) { DCHECK(surface_info.id().is_valid()); @@ -71,21 +79,24 @@ // TODO(fsamuel): We should add a trace for generating a top level frame. cc::CompositorFrame frame(GenerateCompositorFrame(root_window_->bounds())); - gfx::Size frame_size = last_submitted_frame_size_; - if (!frame.render_pass_list.empty()) - frame_size = frame.render_pass_list.back()->output_rect.size(); + if (compositor_frame_sink_) { + gfx::Size frame_size = last_submitted_frame_size_; + if (!frame.render_pass_list.empty()) + frame_size = frame.render_pass_list[0]->output_rect.size(); - if (!local_surface_id_.is_valid() || - frame_size != last_submitted_frame_size_) { - local_surface_id_ = id_allocator_.GenerateId(); - display_private_->ResizeDisplay(frame_size); + if (!local_surface_id_.is_valid() || + frame_size != last_submitted_frame_size_) { + local_surface_id_ = id_allocator_.GenerateId(); + display_private_->ResizeDisplay(frame_size); + } + + display_private_->SetLocalSurfaceId(local_surface_id_, + device_scale_factor_); + compositor_frame_sink_->SubmitCompositorFrame(local_surface_id_, + std::move(frame)); + compositor_frame_sink_->SetNeedsBeginFrame(false); + last_submitted_frame_size_ = frame_size; } - - display_private_->SetLocalSurfaceId(local_surface_id_, device_scale_factor_); - compositor_frame_sink_->SubmitCompositorFrame(local_surface_id_, - std::move(frame)); - compositor_frame_sink_->SetNeedsBeginFrame(false); - last_submitted_frame_size_ = frame_size; } void FrameGenerator::ReclaimResources(
diff --git a/services/ui/ws/frame_generator.h b/services/ui/ws/frame_generator.h index e8ac31c..bfad392a 100644 --- a/services/ui/ws/frame_generator.h +++ b/services/ui/ws/frame_generator.h
@@ -39,13 +39,14 @@ // submitting CompositorFrames to the owned CompositorFrameSink. class FrameGenerator : public cc::mojom::MojoCompositorFrameSinkClient { public: - FrameGenerator(FrameGeneratorDelegate* delegate, - ServerWindow* root_window, - gfx::AcceleratedWidget widget); + FrameGenerator(FrameGeneratorDelegate* delegate, ServerWindow* root_window); ~FrameGenerator() override; void SetDeviceScaleFactor(float device_scale_factor); + // Schedules a redraw for the provided region. + void OnAcceleratedWidgetAvailable(gfx::AcceleratedWidget widget); + // Updates the WindowManager's SurfaceInfo. void OnSurfaceCreated(const cc::SurfaceInfo& surface_info);
diff --git a/services/ui/ws/frame_generator_unittest.cc b/services/ui/ws/frame_generator_unittest.cc new file mode 100644 index 0000000..114924c --- /dev/null +++ b/services/ui/ws/frame_generator_unittest.cc
@@ -0,0 +1,119 @@ +// Copyright 2016 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "services/ui/ws/frame_generator.h" + +#include <memory> + +#include "base/memory/ptr_util.h" +#include "base/test/test_message_loop.h" +#include "cc/quads/render_pass.h" +#include "cc/quads/shared_quad_state.h" +#include "services/ui/ws/ids.h" +#include "services/ui/ws/platform_display_init_params.h" +#include "services/ui/ws/server_window.h" +#include "services/ui/ws/server_window_compositor_frame_sink_manager.h" +#include "services/ui/ws/test_server_window_delegate.h" +#include "services/ui/ws/test_utils.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace ui { +namespace ws { +namespace test { +namespace { + +// Typical id for the display root ServerWindow. +constexpr WindowId kRootDisplayId(0, 2); +const base::UnguessableToken kArbitraryToken = base::UnguessableToken::Create(); + +// Makes the window visible and creates the default surface for it. +void InitWindow(ServerWindow* window) { + window->SetVisible(true); + ServerWindowCompositorFrameSinkManager* compositor_frame_sink_manager = + window->GetOrCreateCompositorFrameSinkManager(); + compositor_frame_sink_manager->SetLatestSurfaceInfo(cc::SurfaceInfo( + cc::SurfaceId(cc::FrameSinkId(WindowIdToTransportId(window->id()), 0), + cc::LocalSurfaceId(1u, kArbitraryToken)), + 1.0f, gfx::Size(100, 100))); +} + +} // namespace + +class FrameGeneratorTest : public testing::Test { + public: + FrameGeneratorTest() + : root_window_(base::MakeUnique<ServerWindow>(&window_delegate_, + kRootDisplayId)) {} + ~FrameGeneratorTest() override {} + + // Calls DrawWindow() on |frame_generator_| + void DrawWindow(cc::RenderPass* pass); + + ServerWindow* root_window() { return root_window_.get(); } + + TestServerWindowDelegate* test_window_delegate() { return &window_delegate_; } + + private: + // testing::Test: + void SetUp() override; + void TearDown() override; + + std::unique_ptr<FrameGenerator> frame_generator_; + std::unique_ptr<TestFrameGeneratorDelegate> frame_generator_delegate_; + TestServerWindowDelegate window_delegate_; + std::unique_ptr<ServerWindow> root_window_; + + // Needed so that Mojo classes can be initialized for ServerWindow use. + base::TestMessageLoop message_loop_; + + DISALLOW_COPY_AND_ASSIGN(FrameGeneratorTest); +}; + +void FrameGeneratorTest::DrawWindow(cc::RenderPass* pass) { + cc::SurfaceId surface_id(cc::FrameSinkId(5, 5), + cc::LocalSurfaceId(1u, kArbitraryToken)); + frame_generator_->window_manager_surface_info_ = + cc::SurfaceInfo(surface_id, 2, gfx::Size(2, 2)); + frame_generator_->DrawWindow(pass); +} + +void FrameGeneratorTest::SetUp() { + testing::Test::SetUp(); + frame_generator_delegate_ = base::MakeUnique<TestFrameGeneratorDelegate>(); + PlatformDisplayInitParams init_params; + frame_generator_ = base::MakeUnique<FrameGenerator>( + frame_generator_delegate_.get(), root_window_.get()); + frame_generator_->SetDeviceScaleFactor( + init_params.metrics.device_scale_factor); + InitWindow(root_window()); +} + +void FrameGeneratorTest::TearDown() { + frame_generator_.reset(); + frame_generator_delegate_.reset(); +} + +// Tests correctness of the SharedQuadStateList generated by +// FrameGenerator::DrawWindow(). +TEST_F(FrameGeneratorTest, DrawWindow) { + ServerWindow child_window(test_window_delegate(), WindowId(1, 1)); + root_window()->Add(&child_window); + InitWindow(&child_window); + const float child_opacity = .4f; + child_window.SetOpacity(child_opacity); + + std::unique_ptr<cc::RenderPass> render_pass = cc::RenderPass::Create(); + DrawWindow(render_pass.get()); + cc::SharedQuadStateList* quad_state_list = + &render_pass->shared_quad_state_list; + + EXPECT_EQ(1u, quad_state_list->size()); + cc::SharedQuadState* root_sqs = quad_state_list->back(); + // Opacity should always be 1. + EXPECT_EQ(1.0f, root_sqs->opacity); +} + +} // namespace test +} // namespace ws +} // namespace ui
diff --git a/services/ui/ws/platform_display_default.cc b/services/ui/ws/platform_display_default.cc index 6d32827f0..6f8c8d6 100644 --- a/services/ui/ws/platform_display_default.cc +++ b/services/ui/ws/platform_display_default.cc
@@ -35,10 +35,11 @@ #if !defined(OS_ANDROID) image_cursors_(new ImageCursors), #endif + frame_generator_(new FrameGenerator(this, init_params.root_window)), metrics_(init_params.metrics), - widget_(gfx::kNullAcceleratedWidget), - root_window_(init_params.root_window), - init_device_scale_factor_(init_params.metrics.device_scale_factor) { + widget_(gfx::kNullAcceleratedWidget) { + frame_generator_->SetDeviceScaleFactor( + init_params.metrics.device_scale_factor); } PlatformDisplayDefault::~PlatformDisplayDefault() { @@ -147,8 +148,7 @@ } metrics_ = metrics; - if (frame_generator_) - frame_generator_->SetDeviceScaleFactor(metrics_.device_scale_factor); + frame_generator_->SetDeviceScaleFactor(metrics_.device_scale_factor); return true; } @@ -178,8 +178,7 @@ } void PlatformDisplayDefault::OnDamageRect(const gfx::Rect& damaged_region) { - if (frame_generator_) - frame_generator_->OnWindowDamaged(); + frame_generator_->OnWindowDamaged(); } void PlatformDisplayDefault::DispatchEvent(ui::Event* event) { @@ -248,9 +247,7 @@ DCHECK_EQ(gfx::kNullAcceleratedWidget, widget_); widget_ = widget; delegate_->OnAcceleratedWidgetAvailable(); - frame_generator_ = - base::MakeUnique<FrameGenerator>(this, root_window_, widget_); - frame_generator_->SetDeviceScaleFactor(init_device_scale_factor_); + frame_generator_->OnAcceleratedWidgetAvailable(widget); } void PlatformDisplayDefault::OnAcceleratedWidgetDestroyed() {
diff --git a/services/ui/ws/platform_display_default.h b/services/ui/ws/platform_display_default.h index 54d36d8..ca1b7d5 100644 --- a/services/ui/ws/platform_display_default.h +++ b/services/ui/ws/platform_display_default.h
@@ -26,7 +26,7 @@ // FrameGenerator for Chrome OS. class PlatformDisplayDefault : public PlatformDisplay, public ui::PlatformWindowDelegate, - public FrameGeneratorDelegate { + private FrameGeneratorDelegate { public: explicit PlatformDisplayDefault(const PlatformDisplayInitParams& init_params); ~PlatformDisplayDefault() override; @@ -83,8 +83,6 @@ display::ViewportMetrics metrics_; std::unique_ptr<ui::PlatformWindow> platform_window_; gfx::AcceleratedWidget widget_; - ServerWindow* root_window_; - float init_device_scale_factor_; DISALLOW_COPY_AND_ASSIGN(PlatformDisplayDefault); };
diff --git a/third_party/WebKit/Source/core/editing/BUILD.gn b/third_party/WebKit/Source/core/editing/BUILD.gn index a570fab3..46d172fc 100644 --- a/third_party/WebKit/Source/core/editing/BUILD.gn +++ b/third_party/WebKit/Source/core/editing/BUILD.gn
@@ -259,6 +259,8 @@ "iterators/TextSearcherICUTest.cpp", "markers/DocumentMarkerControllerTest.cpp", "serializers/StyledMarkupSerializerTest.cpp", + "spellcheck/SpellCheckTestBase.cpp", + "spellcheck/SpellCheckTestBase.h", "spellcheck/SpellCheckerTest.cpp", "state_machines/BackspaceStateMachineTest.cpp", "state_machines/BackwardCodePointStateMachineTest.cpp",
diff --git a/third_party/WebKit/Source/core/editing/DOMSelection.cpp b/third_party/WebKit/Source/core/editing/DOMSelection.cpp index b3c2e3b..4540f0f6 100644 --- a/third_party/WebKit/Source/core/editing/DOMSelection.cpp +++ b/third_party/WebKit/Source/core/editing/DOMSelection.cpp
@@ -204,7 +204,10 @@ return 0; if (documentCachedRange()) return 1; - if (frame()->selection().isNone()) + if (frame() + ->selection() + .computeVisibleSelectionInDOMTreeDeprecated() + .isNone()) return 0; // Any selection can be adjusted to Range for Document. if (isSelectionOfDocument()) @@ -463,7 +466,11 @@ LOG(FATAL) << "Selection has a cached Range, but anchorPosition is null. start=" << range->startContainer() << " end=" << range->endContainer(); - } else if (frame() && !frame()->selection().isNone()) { + } else if (frame() && + !frame() + ->selection() + .computeVisibleSelectionInDOMTreeDeprecated() + .isNone()) { LOG(FATAL) << "FrameSelection is not none, but anchorPosition is null."; } } @@ -649,7 +656,7 @@ FrameSelection& selection = frame()->selection(); - if (selection.isNone()) + if (selection.computeVisibleSelectionInDOMTreeDeprecated().isNone()) return; // TODO(xiaochengh): The use of updateStyleAndLayoutIgnorePendingStylesheets
diff --git a/third_party/WebKit/Source/core/editing/EditingTestBase.cpp b/third_party/WebKit/Source/core/editing/EditingTestBase.cpp index f41ad2a..78cc0cc 100644 --- a/third_party/WebKit/Source/core/editing/EditingTestBase.cpp +++ b/third_party/WebKit/Source/core/editing/EditingTestBase.cpp
@@ -33,6 +33,11 @@ m_dummyPageHolder = DummyPageHolder::create(IntSize(800, 600)); } +void EditingTestBase::setupPageWithClients(Page::PageClients* clients) { + DCHECK(!m_dummyPageHolder) << "Page should be set up only once"; + m_dummyPageHolder = DummyPageHolder::create(IntSize(800, 600), clients); +} + ShadowRoot* EditingTestBase::createShadowRootForElementWithIDAndSetInnerHTML( TreeScope& scope, const char* hostElementID,
diff --git a/third_party/WebKit/Source/core/editing/EditingTestBase.h b/third_party/WebKit/Source/core/editing/EditingTestBase.h index b45f04c353..924313af 100644 --- a/third_party/WebKit/Source/core/editing/EditingTestBase.h +++ b/third_party/WebKit/Source/core/editing/EditingTestBase.h
@@ -5,15 +5,15 @@ #ifndef EditingTestBase_h #define EditingTestBase_h -#include "core/editing/Position.h" -#include "wtf/Forward.h" #include <gtest/gtest.h> #include <memory> #include <string> +#include "core/editing/Position.h" +#include "core/testing/DummyPageHolder.h" +#include "wtf/Forward.h" namespace blink { -class DummyPageHolder; class FrameSelection; class LocalFrame; @@ -26,6 +26,8 @@ void SetUp() override; + void setupPageWithClients(Page::PageClients*); + Document& document() const; DummyPageHolder& dummyPageHolder() const { return *m_dummyPageHolder; } LocalFrame& frame() const;
diff --git a/third_party/WebKit/Source/core/editing/Editor.cpp b/third_party/WebKit/Source/core/editing/Editor.cpp index 8db85385..147b10f 100644 --- a/third_party/WebKit/Source/core/editing/Editor.cpp +++ b/third_party/WebKit/Source/core/editing/Editor.cpp
@@ -410,7 +410,7 @@ DeleteMode deleteMode, InputEvent::InputType inputType, const Position& referenceMovePosition) { - if (frame().selection().isNone()) + if (frame().selection().computeVisibleSelectionInDOMTreeDeprecated().isNone()) return; const bool kMergeBlocksAfterDelete = true; @@ -605,7 +605,10 @@ bool matchStyle, InputEvent::InputType inputType) { DCHECK(!frame().document()->needsLayoutTreeUpdate()); - if (frame().selection().isNone() || + if (frame() + .selection() + .computeVisibleSelectionInDOMTreeDeprecated() + .isNone() || !frame() .selection() .computeVisibleSelectionInDOMTreeDeprecated() @@ -796,7 +799,11 @@ void Editor::applyParagraphStyle(StylePropertySet* style, InputEvent::InputType inputType) { - if (frame().selection().isNone() || !style) + if (frame() + .selection() + .computeVisibleSelectionInDOMTreeDeprecated() + .isNone() || + !style) return; DCHECK(frame().document()); ApplyStyleCommand::create(*frame().document(), EditingStyle::create(style),
diff --git a/third_party/WebKit/Source/core/editing/FrameSelection.cpp b/third_party/WebKit/Source/core/editing/FrameSelection.cpp index e63d68b..e64e78d 100644 --- a/third_party/WebKit/Source/core/editing/FrameSelection.cpp +++ b/third_party/WebKit/Source/core/editing/FrameSelection.cpp
@@ -829,7 +829,7 @@ } void FrameSelection::setFocusedNodeIfNeeded() { - if (isNone() || !isFocused()) + if (computeVisibleSelectionInDOMTreeDeprecated().isNone() || !isFocused()) return; if (Element* target = @@ -1002,7 +1002,8 @@ // entire WebView is editable or designMode is on for this document). Document* document = m_frame->document(); - if (!isNone() || !(blink::hasEditableStyle(*document))) + if (!computeVisibleSelectionInDOMTreeDeprecated().isNone() || + !(blink::hasEditableStyle(*document))) return; Element* documentElement = document->documentElement(); @@ -1103,7 +1104,7 @@ } void FrameSelection::moveRangeSelectionExtent(const IntPoint& contentsPoint) { - if (isNone()) + if (computeVisibleSelectionInDOMTreeDeprecated().isNone()) return; VisibleSelection newSelection =
diff --git a/third_party/WebKit/Source/core/editing/FrameSelection.h b/third_party/WebKit/Source/core/editing/FrameSelection.h index 2e8a12d..f672dce6 100644 --- a/third_party/WebKit/Source/core/editing/FrameSelection.h +++ b/third_party/WebKit/Source/core/editing/FrameSelection.h
@@ -190,10 +190,6 @@ void didChangeFocus(); const SelectionInDOMTree& selectionInDOMTree() const; - // TODO(yosin): We should rename |isNone()| to |isVisibleNone()|. - bool isNone() const { - return computeVisibleSelectionInDOMTreeDeprecated().isNone(); - } bool isInPasswordField() const; bool isDirectional() const { return selectionInDOMTree().isDirectional(); }
diff --git a/third_party/WebKit/Source/core/editing/FrameSelectionTest.cpp b/third_party/WebKit/Source/core/editing/FrameSelectionTest.cpp index 09d6169c..e284266e 100644 --- a/third_party/WebKit/Source/core/editing/FrameSelectionTest.cpp +++ b/third_party/WebKit/Source/core/editing/FrameSelectionTest.cpp
@@ -63,7 +63,8 @@ SelectionInDOMTree::Builder() .setBaseAndExtent(Position(text, 0), Position(text, 5)) .build()); - EXPECT_FALSE(selection().isNone()); + EXPECT_FALSE( + selection().computeVisibleSelectionInDOMTreeDeprecated().isNone()); } TEST_F(FrameSelectionTest, PaintCaretShouldNotLayout) { @@ -208,9 +209,10 @@ Element* select = document().createElement("select"); document().replaceChild(select, document().documentElement()); selection().selectAll(); - EXPECT_TRUE(selection().isNone()) << "Nothing should be selected if the " - "content of the documentElement is not " - "selctable."; + EXPECT_TRUE(selection().computeVisibleSelectionInDOMTreeDeprecated().isNone()) + << "Nothing should be selected if the " + "content of the documentElement is not " + "selctable."; } TEST_F(FrameSelectionTest, SelectAllPreservesHandle) {
diff --git a/third_party/WebKit/Source/core/editing/InputMethodController.cpp b/third_party/WebKit/Source/core/editing/InputMethodController.cpp index 844581bd..6b23c57f 100644 --- a/third_party/WebKit/Source/core/editing/InputMethodController.cpp +++ b/third_party/WebKit/Source/core/editing/InputMethodController.cpp
@@ -326,7 +326,7 @@ // Select the text that will be deleted or replaced. selectComposition(); - if (frame().selection().isNone()) + if (frame().selection().computeVisibleSelectionInDOMTreeDeprecated().isNone()) return false; if (!isAvailable()) @@ -447,7 +447,7 @@ Editor::RevealSelectionScope revealSelectionScope(&editor()); - if (frame().selection().isNone()) + if (frame().selection().computeVisibleSelectionInDOMTreeDeprecated().isNone()) return; clear(); @@ -507,7 +507,7 @@ selectComposition(); - if (frame().selection().isNone()) + if (frame().selection().computeVisibleSelectionInDOMTreeDeprecated().isNone()) return; Element* target = document().focusedElement();
diff --git a/third_party/WebKit/Source/core/editing/commands/EditorCommand.cpp b/third_party/WebKit/Source/core/editing/commands/EditorCommand.cpp index b4cff0c..21ce048f 100644 --- a/third_party/WebKit/Source/core/editing/commands/EditorCommand.cpp +++ b/third_party/WebKit/Source/core/editing/commands/EditorCommand.cpp
@@ -1914,7 +1914,9 @@ // We should update selection to canonicalize with current layout and style, // before accessing |FrameSelection::selection()|. frame.selection().updateIfNeeded(); - return !frame.selection().isNone() && + return !frame.selection() + .computeVisibleSelectionInDOMTreeDeprecated() + .isNone() && frame.selection() .computeVisibleSelectionInDOMTreeDeprecated() .isContentRichlyEditable() &&
diff --git a/third_party/WebKit/Source/core/editing/spellcheck/SpellCheckTestBase.cpp b/third_party/WebKit/Source/core/editing/spellcheck/SpellCheckTestBase.cpp new file mode 100644 index 0000000..b8fbc49 --- /dev/null +++ b/third_party/WebKit/Source/core/editing/spellcheck/SpellCheckTestBase.cpp
@@ -0,0 +1,17 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "core/editing/spellcheck/SpellCheckTestBase.h" + +namespace blink { + +void SpellCheckTestBase::SetUp() { + Page::PageClients pageClients; + fillWithEmptyClients(pageClients); + m_spellCheckerClient = WTF::wrapUnique(new DummySpellCheckerClient); + pageClients.spellCheckerClient = m_spellCheckerClient.get(); + setupPageWithClients(&pageClients); +} + +} // namespace blink
diff --git a/third_party/WebKit/Source/core/editing/spellcheck/SpellCheckTestBase.h b/third_party/WebKit/Source/core/editing/spellcheck/SpellCheckTestBase.h new file mode 100644 index 0000000..96635f19 --- /dev/null +++ b/third_party/WebKit/Source/core/editing/spellcheck/SpellCheckTestBase.h
@@ -0,0 +1,38 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef SpellCheckTestBase_h +#define SpellCheckTestBase_h + +#include "core/editing/EditingTestBase.h" +#include "core/loader/EmptyClients.h" +#include "core/testing/DummyPageHolder.h" + +namespace blink { + +class SpellCheckTestBase : public EditingTestBase { + protected: + void SetUp() override; + + private: + class DummySpellCheckerClient : public EmptySpellCheckerClient { + public: + virtual ~DummySpellCheckerClient() {} + + bool isSpellCheckingEnabled() override { return true; } + + TextCheckerClient& textChecker() override { + return m_emptyTextCheckerClient; + } + + private: + EmptyTextCheckerClient m_emptyTextCheckerClient; + }; + + std::unique_ptr<DummySpellCheckerClient> m_spellCheckerClient; +}; + +} // namespace blink + +#endif // SpellCheckTestBase_h
diff --git a/third_party/WebKit/Source/core/editing/spellcheck/SpellCheckerTest.cpp b/third_party/WebKit/Source/core/editing/spellcheck/SpellCheckerTest.cpp index 3ebd340c..f0499597 100644 --- a/third_party/WebKit/Source/core/editing/spellcheck/SpellCheckerTest.cpp +++ b/third_party/WebKit/Source/core/editing/spellcheck/SpellCheckerTest.cpp
@@ -4,17 +4,16 @@ #include "core/editing/spellcheck/SpellChecker.h" -#include "core/editing/EditingTestBase.h" #include "core/editing/Editor.h" +#include "core/editing/spellcheck/SpellCheckTestBase.h" #include "core/frame/FrameView.h" #include "core/frame/LocalFrame.h" #include "core/frame/Settings.h" #include "core/html/HTMLInputElement.h" -#include "core/testing/DummyPageHolder.h" namespace blink { -class SpellCheckerTest : public EditingTestBase { +class SpellCheckerTest : public SpellCheckTestBase { protected: int layoutCount() const { return page().frameView().layoutCount(); } DummyPageHolder& page() const { return dummyPageHolder(); } @@ -59,10 +58,10 @@ SelectionInDOMTree::Builder().collapse(newPosition).build()); ASSERT_EQ(3u, input->selectionStart()); - Persistent<SpellChecker> spellChecker(SpellChecker::create(page().frame())); + EXPECT_TRUE(frame().spellChecker().isSpellCheckingEnabled()); forceLayout(); int startCount = layoutCount(); - spellChecker->respondToChangedSelection( + frame().spellChecker().respondToChangedSelection( oldSelection.start(), FrameSelection::CloseTyping | FrameSelection::ClearTypingStyle); EXPECT_EQ(startCount, layoutCount());
diff --git a/third_party/WebKit/Source/core/input/EventHandler.cpp b/third_party/WebKit/Source/core/input/EventHandler.cpp index 0d9a229..b8f758b 100644 --- a/third_party/WebKit/Source/core/input/EventHandler.cpp +++ b/third_party/WebKit/Source/core/input/EventHandler.cpp
@@ -555,7 +555,10 @@ if (m_mouseEventManager->mousePressed() && selectionController().mouseDownMayStartSelect() && !m_mouseEventManager->mouseDownMayStartDrag() && - !m_frame->selection().isNone() && !m_capturingMouseEventsNode) { + !m_frame->selection() + .computeVisibleSelectionInDOMTreeDeprecated() + .isNone() && + !m_capturingMouseEventsNode) { return iBeam; }
diff --git a/third_party/WebKit/Source/core/input/EventHandlerTest.cpp b/third_party/WebKit/Source/core/input/EventHandlerTest.cpp index 08bd48fa..b6574da 100644 --- a/third_party/WebKit/Source/core/input/EventHandlerTest.cpp +++ b/third_party/WebKit/Source/core/input/EventHandlerTest.cpp
@@ -382,7 +382,8 @@ TapEventBuilder singleTapEvent(IntPoint(700, 700), 1); document().frame()->eventHandler().handleGestureEvent(singleTapEvent); - ASSERT_TRUE(selection().isNone()); + ASSERT_TRUE( + selection().computeVisibleSelectionInDOMTreeDeprecated().isNone()); ASSERT_FALSE(selection().isHandleVisible()); }
diff --git a/third_party/WebKit/Source/core/page/DragController.cpp b/third_party/WebKit/Source/core/page/DragController.cpp index 8e933ff..052f0448 100644 --- a/third_party/WebKit/Source/core/page/DragController.cpp +++ b/third_party/WebKit/Source/core/page/DragController.cpp
@@ -465,7 +465,9 @@ Range*& range, const IntPoint& point) { frame->selection().setSelection(dragCaret); - if (frame->selection().isNone()) { + if (frame->selection() + .computeVisibleSelectionInDOMTreeDeprecated() + .isNone()) { // TODO(editing-dev): The use of // updateStyleAndLayoutIgnorePendingStylesheets // needs to be audited. See http://crbug.com/590369 for more details. @@ -480,7 +482,9 @@ dragCaret = frame->selection().computeVisibleSelectionInDOMTreeDeprecated(); range = createRange(dragCaret.toNormalizedEphemeralRange()); } - return !frame->selection().isNone() && + return !frame->selection() + .computeVisibleSelectionInDOMTreeDeprecated() + .isNone() && frame->selection() .computeVisibleSelectionInDOMTreeDeprecated() .isContentEditable();
diff --git a/third_party/WebKit/Source/modules/webshare/OWNERS b/third_party/WebKit/Source/modules/webshare/OWNERS new file mode 100644 index 0000000..ae44f9d5 --- /dev/null +++ b/third_party/WebKit/Source/modules/webshare/OWNERS
@@ -0,0 +1,2 @@ +mgiuca@chromium.org +sammc@chromium.org
diff --git a/third_party/WebKit/Source/platform/newwtf/README.md b/third_party/WebKit/Source/platform/newwtf/README.md deleted file mode 100644 index 8ab2fac..0000000 --- a/third_party/WebKit/Source/platform/newwtf/README.md +++ /dev/null
@@ -1,25 +0,0 @@ -# platform/newwtf (will become platform/wtf eventually) - -This is a temporary location where all files under Source/wtf/ will be moved -eventually. For details about WTF relocation project, see -[the proposal](https://docs.google.com/document/d/1shS1IZe__auYxjm9FhPbTY2P01FbNTMYRh-nen5gkDo/edit?usp=sharing) -and -[the design doc](https://docs.google.com/document/d/1JK26H-1-cD9-s9QLvEfY55H2kgSxRFNPLfjs049Us5w/edit?usp=sharing). -You can see -[bug 691465](https://bugs.chromium.org/p/chromium/issues/detail?id=691465) -for the project's status. - -During the project, files in wtf/ are moved to platform/newwtf/ incrementally, -and redirection headers will be placed in wtf/. So nothing should break in the -meantime. You can continue including WTF headers like `#include "wtf/Xxx.h"` -till the next announcement in blink-dev. - -Why "new" wtf? We initially named this directory platform/wtf/, but it turned -out this would cause a warning on win-clang due to MSVC-specific #include -search order. Basically, we can't have "platform/wtf/Foo.h" and "wtf/Foo.h" at -the same time, since `#include "wtf/Foo.h"` may imply platform/wtf/Foo.h -depending on the context. We don't want to turn off this warning Blink-wide, -and that's why we have platform/newwtf/. - -platform/newwtf/ will be renamed to platform/wtf/ after we get rid of wtf/ -completely.
diff --git a/third_party/WebKit/Source/platform/newwtf/DEPS b/third_party/WebKit/Source/platform/wtf/DEPS similarity index 96% rename from third_party/WebKit/Source/platform/newwtf/DEPS rename to third_party/WebKit/Source/platform/wtf/DEPS index 0c1df8d..255ca52 100644 --- a/third_party/WebKit/Source/platform/newwtf/DEPS +++ b/third_party/WebKit/Source/platform/wtf/DEPS
@@ -23,6 +23,6 @@ # To avoid recursive dependency, we impose a blanket ban on using other # platform files. Think carefully if you want to relax this restriction. "-platform", - "+platform/newwtf", + "+platform/wtf", "-v8", ]
diff --git a/third_party/WebKit/Source/platform/newwtf/OWNERS b/third_party/WebKit/Source/platform/wtf/OWNERS similarity index 100% rename from third_party/WebKit/Source/platform/newwtf/OWNERS rename to third_party/WebKit/Source/platform/wtf/OWNERS
diff --git a/third_party/WebKit/Source/platform/wtf/README.md b/third_party/WebKit/Source/platform/wtf/README.md new file mode 100644 index 0000000..e61a5014 --- /dev/null +++ b/third_party/WebKit/Source/platform/wtf/README.md
@@ -0,0 +1,14 @@ +# platform/wtf + +This is the location where all the files under Source/wtf will be moved +eventually. See +[the proposal](https://docs.google.com/document/d/1shS1IZe__auYxjm9FhPbTY2P01FbNTMYRh-nen5gkDo/edit?usp=sharing) +and +[the design doc](https://docs.google.com/document/d/1JK26H-1-cD9-s9QLvEfY55H2kgSxRFNPLfjs049Us5w/edit?usp=sharing) +regarding the relocation project. For the project's progress, see +[bug 691465](https://bugs.chromium.org/p/chromium/issues/detail?id=691465). + +During the project, files in wtf/ are moved to platform/wtf incrementally, and +redirection headers will be placed in wtf/. So nothing should break in the +meantime. You can continue including WTF headers like `#include "wtf/Xxx.h"` +till the next announce in blink-dev.
diff --git a/third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp b/third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp index 740bf93e..8b3bdea 100644 --- a/third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp +++ b/third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp
@@ -548,7 +548,7 @@ return false; FrameSelection& selection = localFrame->selection(); - if (selection.isNone()) + if (selection.computeVisibleSelectionInDOMTreeDeprecated().isNone()) return false; // TODO(xiaochengh): The use of updateStyleAndLayoutIgnorePendingStylesheets
diff --git a/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp b/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp index b11fc7b0..93281bf 100644 --- a/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp +++ b/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
@@ -1149,7 +1149,7 @@ bool WebLocalFrameImpl::selectWordAroundCaret() { TRACE_EVENT0("blink", "WebLocalFrameImpl::selectWordAroundCaret"); FrameSelection& selection = frame()->selection(); - if (selection.isNone() || + if (selection.computeVisibleSelectionInDOMTreeDeprecated().isNone() || selection.computeVisibleSelectionInDOMTreeDeprecated().isRange()) return false;
diff --git a/third_party/WebKit/Source/web/WebViewImpl.cpp b/third_party/WebKit/Source/web/WebViewImpl.cpp index 64e2af8a..6ab6f18 100644 --- a/third_party/WebKit/Source/web/WebViewImpl.cpp +++ b/third_party/WebKit/Source/web/WebViewImpl.cpp
@@ -2356,7 +2356,8 @@ if (!localFrame) return false; FrameSelection& selection = localFrame->selection(); - if (!selection.isAvailable() || selection.isNone()) { + if (!selection.isAvailable() || + selection.computeVisibleSelectionInDOMTreeDeprecated().isNone()) { // plugins/mouse-capture-inside-shadow.html reaches here. return false; }
diff --git a/third_party/WebKit/Source/wtf/DEPS b/third_party/WebKit/Source/wtf/DEPS index ce89b4a..49c624e5 100644 --- a/third_party/WebKit/Source/wtf/DEPS +++ b/third_party/WebKit/Source/wtf/DEPS
@@ -20,7 +20,7 @@ "+base/time/time.h", "+base/tuple.h", "+build", - # Allow redirection to platform/newwtf. See crbug.com/691465. - "+platform/newwtf", + # Allow redirection to platform/wtf. See crbug.com/691465. + "+platform/wtf", "-v8", ]
diff --git a/third_party/WebKit/Source/wtf/README.md b/third_party/WebKit/Source/wtf/README.md index a65a111..980a7bac 100644 --- a/third_party/WebKit/Source/wtf/README.md +++ b/third_party/WebKit/Source/wtf/README.md
@@ -1,4 +1,4 @@ # WTF -- Web Template Framework -The contents in this directory are being moved to platform/newwtf. See -[platform/newwtf/README.md](../platform/wtf/README.md) for details. +The contents in this directory are being moved to platform/wtf. See +[platform/wtf/README.md](../platform/wtf/README.md) for details.
diff --git a/third_party/WebKit/Source/wtf/Vector.h b/third_party/WebKit/Source/wtf/Vector.h index 77f86ed..c3ab13f 100644 --- a/third_party/WebKit/Source/wtf/Vector.h +++ b/third_party/WebKit/Source/wtf/Vector.h
@@ -64,6 +64,10 @@ // Bunch of traits for Vector are defined here, with which you can customize // Vector's behavior. In most cases the default traits are appropriate, so you // usually don't have to specialize those traits by yourself. +// +// The behavior of the implementation below can be controlled by VectorTraits. +// If you want to change the behavior of your type, take a look at VectorTraits +// (defined in VectorTraits.h), too. template <bool needsDestruction, typename T> struct VectorDestructor; @@ -288,8 +292,9 @@ }; // A collection of all the traits used by Vector. This is basically an -// implementation detail of Vector, and you should specialize individual traits -// defined above, if you want to customize Vector's behavior. +// implementation detail of Vector, and you probably don't want to change this. +// If you want to customize Vector's behavior, you should specialize +// VectorTraits instead (defined in VectorTraits.h). template <typename T> struct VectorTypeOperations { STATIC_ONLY(VectorTypeOperations); @@ -821,6 +826,108 @@ // Vector // +// Vector is a container that works just like std::vector. WTF's Vector has +// several extra functionalities: inline buffer, behavior customization via +// traits, and Oilpan support. Those are explained in the sections below. +// +// Vector is the most basic container, which stores its element in a contiguous +// buffer. The buffer is expanded automatically when necessary. The elements +// are automatically moved to the new buffer. This event is called a +// reallocation. A reallocation takes O(N)-time (N = number of elements), but +// its occurrences are rare, so its time cost should not be significant, +// compared to the time cost of other operations to the vector. +// +// Time complexity of key operations is as follows: +// +// * Indexed access -- O(1) +// * Insertion or removal of an element at the end -- amortized O(1) +// * Other insertion or removal -- O(N) +// * Swapping with another vector -- O(1) +// +// 1. Iterator invalidation semantics +// +// Vector provides STL-compatible iterators and reverse iterators. Iterators +// are _invalidated_ on certain occasions. Reading an invalidated iterator +// causes undefined behavior. +// +// Iterators are invalidated on the following situations: +// +// * When a reallocation happens on a vector, all the iterators for that +// vector will be invalidated. +// * Some member functions invalidate part of the existing iterators for +// the vector; see comments on the individual functions. +// * [Oilpan only] Heap compaction invalidates all the iterators for any +// HeapVectors. This means you can only store an iterator on stack, as +// a local variable. +// +// In this context, pointers or references to an element of a Vector are +// essentially equivalent to iterators, in that they also become invalid +// whenever corresponding iterators are invalidated. +// +// 2. Inline buffer +// +// Vectors may have an _inline buffer_. An inline buffer is a storage area +// that is contained in the vector itself, along with other metadata like +// m_size. It is used as a storage space when the vector's elements fit in +// that space. If the inline buffer becomes full and further space is +// necessary, an out-of-line buffer is allocated in the heap, and it will +// take over the role of the inline buffer. +// +// The existence of an inline buffer is indicated by non-zero |inlineCapacity| +// template argument. The value represents the number of elements that can be +// stored in the inline buffer. Zero |inlineCapacity| means the vector has no +// inline buffer. +// +// An inline buffer increases the size of the Vector instances, and, in trade +// for that, it gives you several performance benefits, as long as the number +// of elements do not exceed |inlineCapacity|: +// +// * No heap allocation will be made. +// * Memory locality will improve. +// +// Generally, having an inline buffer is useful for vectors that (1) are +// frequently accessed or modified, and (2) contain only a few elements at +// most. +// +// 3. Behavior customization +// +// You usually do not need to customize Vector's behavior, since the default +// behavior is appropriate for normal usage. The behavior is controlled by +// VectorTypeOperations traits template above. Read VectorTypeOperations +// and VectorTraits if you want to change the behavior for your types (i.e. +// if you really want faster vector operations). +// +// The default traits basically do the following: +// +// * Skip constructor call and fill zeros with memset for simple types; +// * Skip destructor call for simple types; +// * Copy or move by memcpy for simple types; and +// * Customize the comparisons for smart pointer types, so you can look +// up a std::unique_ptr<T> element with a raw pointer, for instance. +// +// 4. Oilpan +// +// If you want to store garbage collected objects in Vector, (1) use HeapVector +// (defined in HeapAllocator.h) instead of Vector, and (2) make sure your +// garbage-collected type is wrapped with Member, like: +// +// HeapVector<Member<Node>> nodes; +// +// Unlike normal garbage-collected objects, a HeapVector object itself is +// NOT a garbage-collected object, but its backing buffer is allocated in +// Oilpan heap, and it may still carry garbage-collected objects. +// +// Even though a HeapVector object is not garbage-collected, you still need +// to trace it, if you stored it in your class. Also, you can allocate it +// as a local variable. This is useful when you want to build a vector locally +// and put it in an on-heap vector with swap(). +// +// Also, heap compaction, which may happen at any time when Blink code is not +// running (i.e. Blink code does not appear in the call stack), may invalidate +// existing iterators for any HeapVectors. So, essentially, you should always +// allocate an iterator on stack (as a local variable), and you should not +// store iterators in another heap object. + template <typename T, size_t inlineCapacity = 0, typename Allocator = PartitionAllocator>
diff --git a/third_party/WebKit/public/platform/modules/webshare/OWNERS b/third_party/WebKit/public/platform/modules/webshare/OWNERS index 5e4aaef..691beec 100644 --- a/third_party/WebKit/public/platform/modules/webshare/OWNERS +++ b/third_party/WebKit/public/platform/modules/webshare/OWNERS
@@ -1,3 +1,6 @@ +mgiuca@chromium.org +sammc@chromium.org + # Changes to Mojo interfaces require a security review to avoid # introducing new sandbox escapes. per-file *.mojom=set noparent
diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index 7809606..6bf2c91 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/tools/metrics/histograms/histograms.xml
@@ -118557,6 +118557,9 @@ <suffix name="externalrequestforced" label="Forced prerender regardless of network."/> <suffix name="Instant" label="Instant search prerender."/> + <suffix name="none" + label="No origin; in the case of prefetch TTFCP this is when no + prefetch was involved."/> <suffix name="omnibox" label="Triggered from the omnibox."/> <suffix name="wash" label="Multiple sources could have triggered."/> <suffix name="web" label="Link triggered prerender."/>
diff --git a/ui/file_manager/PRESUBMIT.py b/ui/file_manager/PRESUBMIT.py index c6783c4..82825ea4fb 100644 --- a/ui/file_manager/PRESUBMIT.py +++ b/ui/file_manager/PRESUBMIT.py
@@ -9,3 +9,16 @@ 'master.tryserver.chromium.linux:closure_compilation', ], 'Automatically added optional Closure bots to run on CQ.') + + +def CheckChangeOnUpload(input_api, output_api): + return _CommonChecks(input_api, output_api) + + +def CheckChangeOnCommit(input_api, output_api): + return _CommonChecks(input_api, output_api) + + +def _CommonChecks(input_api, output_api): + return input_api.canned_checks.CheckPatchFormatted(input_api, output_api, + check_js=True)
diff --git a/ui/file_manager/compiled_resources2.gyp b/ui/file_manager/compiled_resources2.gyp index 170c22f..0b865462 100644 --- a/ui/file_manager/compiled_resources2.gyp +++ b/ui/file_manager/compiled_resources2.gyp
@@ -15,6 +15,7 @@ 'file_manager/foreground/js/compiled_resources2.gyp:*', 'file_manager/foreground/js/metadata/compiled_resources2.gyp:*', 'gallery/js/compiled_resources2.gyp:*', + 'gallery/js/image_editor/compiled_resources2.gyp:*', 'image_loader/compiled_resources2.gyp:*', 'video_player/js/cast/compiled_resources2.gyp:*', 'video_player/js/compiled_resources2.gyp:*',
diff --git a/ui/file_manager/externs/compiled_resources2.gyp b/ui/file_manager/externs/compiled_resources2.gyp index ebfed7e..77d9dcf 100644 --- a/ui/file_manager/externs/compiled_resources2.gyp +++ b/ui/file_manager/externs/compiled_resources2.gyp
@@ -84,6 +84,10 @@ 'includes': ['../../../third_party/closure_compiler/include_js.gypi'], }, { + 'target_name': 'gallery_event', + 'includes': ['../../../third_party/closure_compiler/include_js.gypi'], + }, + { 'target_name': 'gallery_foreground', 'includes': ['../../../third_party/closure_compiler/include_js.gypi'], },
diff --git a/ui/file_manager/externs/gallery_event.js b/ui/file_manager/externs/gallery_event.js new file mode 100644 index 0000000..2c262ef --- /dev/null +++ b/ui/file_manager/externs/gallery_event.js
@@ -0,0 +1,20 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + + +/** + * @constructor + * @extends {Event} + */ +var GalleryEvent = function() {}; + +/** + * @type {Entry} + */ +GalleryEvent.prototype.oldEntry; + +/** + * @type {GalleryItem} + */ +GalleryEvent.prototype.item;
diff --git a/ui/file_manager/file_manager/foreground/js/compiled_resources2.gyp b/ui/file_manager/file_manager/foreground/js/compiled_resources2.gyp index c38db4dc..a4cc5dc 100644 --- a/ui/file_manager/file_manager/foreground/js/compiled_resources2.gyp +++ b/ui/file_manager/file_manager/foreground/js/compiled_resources2.gyp
@@ -183,10 +183,16 @@ # 'target_name': 'task_controller', # 'includes': ['../../../compile_js2.gypi'], # }, -# { -# 'target_name': 'thumbnail_loader', -# 'includes': ['../../../compile_js2.gypi'], -# }, + { + 'target_name': 'thumbnail_loader', + 'dependencies': [ + '../../../image_loader/compiled_resources2.gyp:image_loader_client', + '../../common/js/compiled_resources2.gyp:file_type', + '../../common/js/compiled_resources2.gyp:util', + '<(DEPTH)/ui/webui/resources/js/compiled_resources2.gyp:assert', + ], + 'includes': ['../../../compile_js2.gypi'], + }, # { # 'target_name': 'toolbar_controller', # 'includes': ['../../../compile_js2.gypi'],
diff --git a/ui/file_manager/gallery/js/compiled_resources.gyp b/ui/file_manager/gallery/js/compiled_resources.gyp index 2d498007..d7b996fd 100644 --- a/ui/file_manager/gallery/js/compiled_resources.gyp +++ b/ui/file_manager/gallery/js/compiled_resources.gyp
@@ -124,6 +124,7 @@ '../../file_manager/foreground/js/volume_manager_wrapper.js', '../../image_loader/image_loader_client.js', './image_editor/image_util.js', + './image_editor/image_loader.js', './image_editor/viewport.js', './image_editor/image_buffer.js', './image_editor/image_view.js',
diff --git a/ui/file_manager/gallery/js/compiled_resources2.gyp b/ui/file_manager/gallery/js/compiled_resources2.gyp index ce84252..1f7eb26 100644 --- a/ui/file_manager/gallery/js/compiled_resources2.gyp +++ b/ui/file_manager/gallery/js/compiled_resources2.gyp
@@ -1,6 +1,7 @@ # Copyright 2016 The Chromium Authors. All rights reserved. # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. +# TODO(oka): Compile all the targets. { 'targets': [ # { @@ -15,44 +16,97 @@ # 'target_name': 'entry_list_watcher', # 'includes': ['../../compile_js2.gypi'], # }, -# { -# 'target_name': 'error_banner', -# 'includes': ['../../compile_js2.gypi'], -# }, + { + 'target_name': 'error_banner', + 'dependencies': [ + '../../file_manager/common/js/compiled_resources2.gyp:util', + ], + 'includes': ['../../compile_js2.gypi'], + }, # { # 'target_name': 'gallery', # 'includes': ['../../compile_js2.gypi'], # }, # { # 'target_name': 'gallery_data_model', +# 'dependencies': [ +# 'thumbnail_mode', +# '../../file_manager/common/js/compiled_resources2.gyp:util', +# '<(DEPTH)/ui/webui/resources/js/compiled_resources2.gyp:assert', +# 'gallery_item', +# '<(DEPTH)/ui/webui/resources/js/cr/ui/compiled_resources2.gyp:array_data_model', +# ], # 'includes': ['../../compile_js2.gypi'], # }, -# { -# 'target_name': 'gallery_item', -# 'includes': ['../../compile_js2.gypi'], -# }, + { + 'target_name': 'gallery_item', + 'dependencies': [ + '../../file_manager/common/js/compiled_resources2.gyp:util', + '../../file_manager/foreground/js/compiled_resources2.gyp:volume_manager_wrapper', + '../../file_manager/foreground/js/metadata/compiled_resources2.gyp:metadata_model', + '../../file_manager/foreground/js/metadata/compiled_resources2.gyp:thumbnail_model', + 'gallery_util', + 'image_editor/compiled_resources2.gyp:image_encoder', + 'image_editor/compiled_resources2.gyp:image_util', + ], + 'includes': ['../../compile_js2.gypi'], + }, # { # 'target_name': 'gallery_scripts', # 'includes': ['../../compile_js2.gypi'], # }, -# { -# 'target_name': 'gallery_util', -# 'includes': ['../../compile_js2.gypi'], -# }, -# { -# 'target_name': 'metadata_worker', -# 'includes': ['../../compile_js2.gypi'], -# }, -# { -# 'target_name': 'ribbon', -# 'includes': ['../../compile_js2.gypi'], -# }, + { + 'target_name': 'gallery_util', + 'dependencies': [ + '../../file_manager/common/js/compiled_resources2.gyp:file_type', + '../../file_manager/common/js/compiled_resources2.gyp:util', + '../../file_manager/common/js/compiled_resources2.gyp:volume_manager_common', + '../../file_manager/foreground/js/compiled_resources2.gyp:volume_manager_wrapper', + ], + 'includes': ['../../compile_js2.gypi'], + }, + { + 'target_name': 'metadata_worker', + 'includes': ['../../compile_js2.gypi'], + }, + { + 'target_name': 'ribbon', + 'dependencies': [ + '../../externs/compiled_resources2.gyp:gallery_event', + '../../file_manager/foreground/js/compiled_resources2.gyp:thumbnail_loader', + '../../file_manager/foreground/js/metadata/compiled_resources2.gyp:thumbnail_model', + '<(DEPTH)/ui/webui/resources/js/cr/ui/compiled_resources2.gyp:array_data_model', + '<(DEPTH)/ui/webui/resources/js/cr/ui/compiled_resources2.gyp:list_selection_model', + 'gallery_item', + 'image_editor/compiled_resources2.gyp:image_util', + ], + 'includes': ['../../compile_js2.gypi'], + }, # { # 'target_name': 'slide_mode', +# 'dependencies': [ +# '../../file_manager/common/js/compiled_resources2.gyp:metrics', +# '../../file_manager/common/js/compiled_resources2.gyp:util', +# '<(EXTERNS_GYP):chrome_extensions', +# 'gallery_item', +# 'image_editor/compiled_resources2.gyp:image_editor', +# 'image_editor/compiled_resources2.gyp:image_util', +# 'image_editor/compiled_resources2.gyp:image_view', +# 'image_editor/compiled_resources2.gyp:viewport', +# ], # 'includes': ['../../compile_js2.gypi'], # }, # { # 'target_name': 'thumbnail_mode', +# 'dependencies': [ +# '../../file_manager/foreground/js/compiled_resources2.gyp:thumbnail_loader', +# '../../file_manager/foreground/js/metadata/compiled_resources2.gyp:thumbnail_model', +# '<(DEPTH)/ui/webui/resources/js/cr/ui/compiled_resources2.gyp:list_selection_model', +# 'error_banner', +# 'gallery_data_model', +# 'gallery_item', +# 'image_editor/compiled_resources2.gyp:image_editor', +# ], # 'includes': ['../../compile_js2.gypi'], # }, ],
diff --git a/ui/file_manager/gallery/js/gallery.js b/ui/file_manager/gallery/js/gallery.js index 4f986de4..71aaac3f 100644 --- a/ui/file_manager/gallery/js/gallery.js +++ b/ui/file_manager/gallery/js/gallery.js
@@ -228,13 +228,6 @@ } /** - * Tools fade-out timeout in milliseconds. - * @const - * @type {number} - */ -Gallery.FADE_TIMEOUT = 2000; - -/** * First time tools fade-out timeout in milliseconds. * @const * @type {number} @@ -250,14 +243,6 @@ Gallery.MOSAIC_BACKGROUND_INIT_DELAY = 1000; /** - * Types of metadata Gallery uses (to query the metadata cache). - * @const - * @type {!Array<string>} - */ -Gallery.PREFETCH_PROPERTY_NAMES = - ['imageWidth', 'imageHeight', 'imageRotation', 'size', 'present']; - -/** * Modes in Gallery. * @enum {string} */ @@ -395,7 +380,7 @@ var item = items[index]; var entry = item.getEntry(); var metadataPromise = self.metadataModel_.get([entry], - Gallery.PREFETCH_PROPERTY_NAMES); + GalleryItem.PREFETCH_PROPERTY_NAMES); var thumbnailPromise = thumbnailModel.get([entry]); return Promise.all([metadataPromise, thumbnailPromise]).then( function(metadataLists) {
diff --git a/ui/file_manager/gallery/js/gallery_data_model.js b/ui/file_manager/gallery/js/gallery_data_model.js index ad2b4e5..85ac647 100644 --- a/ui/file_manager/gallery/js/gallery_data_model.js +++ b/ui/file_manager/gallery/js/gallery_data_model.js
@@ -84,7 +84,7 @@ if (!util.isSameEntry(oldEntry, item.getEntry())) { Promise.all([ this.metadataModel_.get( - [oldEntry], Gallery.PREFETCH_PROPERTY_NAMES), + [oldEntry], GalleryItem.PREFETCH_PROPERTY_NAMES), new ThumbnailModel(this.metadataModel_).get([oldEntry]) ]).then(function(itemLists) { // New entry is added and the item now tracks it.
diff --git a/ui/file_manager/gallery/js/gallery_item.js b/ui/file_manager/gallery/js/gallery_item.js index 358bb93f..75aa5e7 100644 --- a/ui/file_manager/gallery/js/gallery_item.js +++ b/ui/file_manager/gallery/js/gallery_item.js
@@ -60,6 +60,14 @@ }; /** + * Types of metadata Gallery uses (to query the metadata cache). + * @const + * @type {!Array<string>} + */ +GalleryItem.PREFETCH_PROPERTY_NAMES = + ['imageWidth', 'imageHeight', 'imageRotation', 'size', 'present']; + +/** * @return {!FileEntry} Image entry. */ GalleryItem.prototype.getEntry = function() { return this.entry_; }; @@ -261,7 +269,7 @@ */ GalleryItem.prototype.saveToFile = function( volumeManager, metadataModel, fallbackDir, canvas, overwrite, callback) { - ImageUtil.metrics.startInterval(ImageUtil.getMetricName('SaveTime')); + metrics.startInterval(ImageUtil.getMetricName('SaveTime')); var saveResultRecorded = false; Promise.all([this.getEntryToWrite_(overwrite, fallbackDir, volumeManager), @@ -304,9 +312,9 @@ locationInfo = this.locationInfo_; } - ImageUtil.metrics.recordEnum(ImageUtil.getMetricName('SaveResult'), 1, 2); + metrics.recordEnum(ImageUtil.getMetricName('SaveResult'), 1, 2); saveResultRecorded = true; - ImageUtil.metrics.recordInterval(ImageUtil.getMetricName('SaveTime')); + metrics.recordInterval(ImageUtil.getMetricName('SaveTime')); this.entry_ = fileEntry; this.locationInfo_ = locationInfo; @@ -314,7 +322,7 @@ // Updates the metadata. metadataModel.notifyEntriesChanged([this.entry_]); Promise.all([ - metadataModel.get([this.entry_], Gallery.PREFETCH_PROPERTY_NAMES), + metadataModel.get([this.entry_], GalleryItem.PREFETCH_PROPERTY_NAMES), new ThumbnailModel(metadataModel).get([this.entry_]) ]).then(function(metadataLists) { this.metadataItem_ = metadataLists[0][0]; @@ -328,7 +336,7 @@ console.error('Error saving from gallery', this.entry_.name, error); if (!saveResultRecorded) - ImageUtil.metrics.recordEnum(ImageUtil.getMetricName('SaveResult'), 0, 2); + metrics.recordEnum(ImageUtil.getMetricName('SaveResult'), 0, 2); callback(false); }.bind(this));
diff --git a/ui/file_manager/gallery/js/gallery_item_unittest.js b/ui/file_manager/gallery/js/gallery_item_unittest.js index 8ce4bff..a96d9335 100644 --- a/ui/file_manager/gallery/js/gallery_item_unittest.js +++ b/ui/file_manager/gallery/js/gallery_item_unittest.js
@@ -3,15 +3,15 @@ // found in the LICENSE file. /** - * Mock of ImageUtil. + * Mock of ImageUtil and metrics. */ var ImageUtil = { getMetricName: function() {}, - metrics: { - recordEnum: function() {}, - recordInterval: function() {}, - startInterval: function() {} - } +}; +var metrics = { + recordEnum: function() {}, + recordInterval: function() {}, + startInterval: function() {} }; /**
diff --git a/ui/file_manager/gallery/js/gallery_scripts.js b/ui/file_manager/gallery/js/gallery_scripts.js index 2873e7f..98fa45ba 100644 --- a/ui/file_manager/gallery/js/gallery_scripts.js +++ b/ui/file_manager/gallery/js/gallery_scripts.js
@@ -65,6 +65,7 @@ // <include src="image_editor/image_util.js"> // <include src="image_editor/viewport.js"> // <include src="image_editor/image_buffer.js"> +// <include src="image_editor/image_loader.js"> // <include src="image_editor/image_view.js"> // <include src="image_editor/commands.js"> // <include src="image_editor/image_editor.js"> @@ -88,7 +89,7 @@ // Exports window.ImageUtil = ImageUtil; -window.ImageUtil.metrics = metrics; +window.metrics = metrics; window.Gallery = Gallery; window.reload = reload; // will be called by the background.
diff --git a/ui/file_manager/gallery/js/image_editor/compiled_resources2.gyp b/ui/file_manager/gallery/js/image_editor/compiled_resources2.gyp index 255bb3dc..d681fa1 100644 --- a/ui/file_manager/gallery/js/image_editor/compiled_resources2.gyp +++ b/ui/file_manager/gallery/js/image_editor/compiled_resources2.gyp
@@ -1,55 +1,122 @@ # Copyright 2016 The Chromium Authors. All rights reserved. # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. +# TODO(oka): Compile all the targets. { 'targets': [ # { # 'target_name': 'commands', +# 'dependencies': [ +# 'image_util', +# '../../../file_manager/foreground/elements/compiled_resources2.gyp:files_toast', +# 'image_editor', +# ], # 'includes': ['../../../compile_js2.gypi'], # }, -# { -# 'target_name': 'exif_encoder', -# 'includes': ['../../../compile_js2.gypi'], -# }, -# { -# 'target_name': 'filter', -# 'includes': ['../../../compile_js2.gypi'], -# }, + { + 'target_name': 'exif_encoder', + 'dependencies': [ + '../../../externs/compiled_resources2.gyp:exif_entry', + '../../../file_manager/foreground/js/metadata/compiled_resources2.gyp:exif_constants', + '../../../file_manager/foreground/js/metadata/compiled_resources2.gyp:metadata_item', + 'image_encoder', + ], + 'includes': ['../../../compile_js2.gypi'], + }, + { + 'target_name': 'filter', + 'dependencies': [ + 'image_util', + ], + 'includes': ['../../../compile_js2.gypi'], + }, # { # 'target_name': 'image_adjust', +# 'dependencies': [ +# '<(DEPTH)/ui/webui/resources/js/compiled_resources2.gyp:assert', +# 'commands', +# 'image_editor', +# ], # 'includes': ['../../../compile_js2.gypi'], # }, -# { -# 'target_name': 'image_buffer', -# 'includes': ['../../../compile_js2.gypi'], -# }, + { + 'target_name': 'image_buffer', + 'includes': ['../../../compile_js2.gypi'], + }, # { # 'target_name': 'image_editor', +# 'dependencies': [ +# 'viewport', +# 'image_view', +# 'commands', +# '../../../file_manager/common/js/compiled_resources2.gyp:util', +# 'image_buffer', +# 'image_util', +# '<(DEPTH)/ui/webui/resources/js/cr/compiled_resources2.gyp:event_target', +# '../../../file_manager/foreground/elements/compiled_resources2.gyp:files_tooltip', +# '../../../file_manager/common/js/compiled_resources2.gyp:util', +# '../compiled_resources2.gyp:gallery_util', +# ], # 'includes': ['../../../compile_js2.gypi'], # }, -# { -# 'target_name': 'image_encoder', -# 'includes': ['../../../compile_js2.gypi'], -# }, + { + 'target_name': 'image_encoder', + 'dependencies': [ + '../../../file_manager/foreground/js/metadata/compiled_resources2.gyp:metadata_item', + '<(DEPTH)/ui/webui/resources/js/compiled_resources2.gyp:assert', + 'image_util', + ], + 'includes': ['../../../compile_js2.gypi'], + }, + { + 'target_name': 'image_loader', + 'dependencies': [ + '../../../file_manager/common/js/compiled_resources2.gyp:file_type', + '../../../file_manager/common/js/compiled_resources2.gyp:metrics_base', + '../../../file_manager/common/js/compiled_resources2.gyp:util', + '../../../file_manager/foreground/js/metadata/compiled_resources2.gyp:metadata_model', + '../../../image_loader/compiled_resources2.gyp:image_loader_client', + '../compiled_resources2.gyp:gallery_item', + 'image_util', + ], + 'includes': ['../../../compile_js2.gypi'], + }, # { # 'target_name': 'image_transform', # 'includes': ['../../../compile_js2.gypi'], # }, -# { -# 'target_name': 'image_util', -# 'includes': ['../../../compile_js2.gypi'], -# }, -# { -# 'target_name': 'image_view', -# 'includes': ['../../../compile_js2.gypi'], -# }, + { + 'target_name': 'image_util', + 'dependencies': [ + '<(DEPTH)/ui/webui/resources/js/compiled_resources2.gyp:assert', + ], + 'includes': ['../../../compile_js2.gypi'], + }, + { + 'target_name': 'image_view', + 'dependencies': [ + '../../../file_manager/common/js/compiled_resources2.gyp:metrics', + '../../../file_manager/foreground/js/compiled_resources2.gyp:thumbnail_loader', + '../compiled_resources2.gyp:gallery_item', + '<(DEPTH)/ui/webui/resources/js/compiled_resources2.gyp:assert', + 'image_buffer', + 'image_loader', + 'image_util', + 'viewport', + ], + 'includes': ['../../../compile_js2.gypi'], + }, # { # 'target_name': 'test_util', # 'includes': ['../../../compile_js2.gypi'], # }, -# { -# 'target_name': 'viewport', -# 'includes': ['../../../compile_js2.gypi'], -# }, + { + 'target_name': 'viewport', + 'dependencies': [ + '<(DEPTH)/ui/webui/resources/js/cr/compiled_resources2.gyp:event_target', + 'image_util', + ], + 'includes': ['../../../compile_js2.gypi'], + }, ], }
diff --git a/ui/file_manager/gallery/js/image_editor/image_editor.js b/ui/file_manager/gallery/js/image_editor/image_editor.js index c1397f18..30da9fe3 100644 --- a/ui/file_manager/gallery/js/image_editor/image_editor.js +++ b/ui/file_manager/gallery/js/image_editor/image_editor.js
@@ -182,7 +182,7 @@ * @param {string} name Action name. */ ImageEditor.prototype.recordToolUse = function(name) { - ImageUtil.metrics.recordEnum( + metrics.recordEnum( ImageUtil.getMetricName('Tool'), name, this.actionNames_); };
diff --git a/ui/file_manager/gallery/js/image_editor/image_loader.js b/ui/file_manager/gallery/js/image_editor/image_loader.js new file mode 100644 index 0000000..fe9eb232 --- /dev/null +++ b/ui/file_manager/gallery/js/image_editor/image_loader.js
@@ -0,0 +1,265 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +/** + * ImageLoader loads an image from a given Entry into a canvas in two steps: + * 1. Loads the image into an HTMLImageElement. + * 2. Copies pixels from HTMLImageElement to HTMLCanvasElement. This is done + * stripe-by-stripe to avoid freezing up the UI. The transform is taken into + * account. + * + * @param {!HTMLDocument} document Owner document. + * @param {!MetadataModel} metadataModel + * @constructor + * @struct + */ +ImageUtil.ImageLoader = function(document, metadataModel) { + this.document_ = document; + + /** + * @private {!MetadataModel} + * @const + */ + this.metadataModel_ = metadataModel; + + this.generation_ = 0; + + /** + * @type {number} + * @private + */ + this.timeout_ = 0; + + /** + * @type {?function(!HTMLCanvasElement, string=)} + * @private + */ + this.callback_ = null; + + /** + * @type {FileEntry} + * @private + */ + this.entry_ = null; +}; + +/** + * Loads an image. + * TODO(mtomasz): Simplify, or even get rid of this class and merge with the + * ThumbnaiLoader class. + * + * @param {!GalleryItem} item Item representing the image to be loaded. + * @param {function(!HTMLCanvasElement, string=)} callback Callback to be + * called when loaded. The second optional argument is an error identifier. + * @param {number=} opt_delay Load delay in milliseconds, useful to let the + * animations play out before the computation heavy image loading starts. + */ +ImageUtil.ImageLoader.prototype.load = function(item, callback, opt_delay) { + var entry = item.getEntry(); + + this.cancel(); + this.entry_ = entry; + this.callback_ = callback; + + var targetImage = assertInstanceof(this.document_.createElement('img'), + HTMLImageElement); + // The transform fetcher is not cancellable so we need a generation counter. + var generation = ++this.generation_; + + /** + * @param {!HTMLImageElement} image Image to be transformed. + * @param {Object=} opt_transform Transformation. + */ + var onTransform = function(image, opt_transform) { + if (generation === this.generation_) { + this.convertImage_(image, opt_transform); + } + }; + onTransform = onTransform.bind(this); + + /** + * @param {string=} opt_error Error. + */ + var onError = function(opt_error) { + targetImage.onerror = null; + targetImage.onload = null; + var tmpCallback = this.callback_; + this.callback_ = null; + var emptyCanvas = assertInstanceof(this.document_.createElement('canvas'), + HTMLCanvasElement); + emptyCanvas.width = 0; + emptyCanvas.height = 0; + tmpCallback(emptyCanvas, opt_error); + }; + onError = onError.bind(this); + + var loadImage = function(url) { + if (generation !== this.generation_) + return; + + metrics.startInterval(ImageUtil.getMetricName('LoadTime')); + this.timeout_ = 0; + + targetImage.onload = function() { + targetImage.onerror = null; + targetImage.onload = null; + if (generation !== this.generation_) + return; + this.metadataModel_.get([entry], ['contentImageTransform']).then( + function(metadataItems) { + onTransform(targetImage, metadataItems[0].contentImageTransform); + }.bind(this)); + }.bind(this); + + // The error callback has an optional error argument, which in case of a + // general error should not be specified + targetImage.onerror = onError.bind(null, 'GALLERY_IMAGE_ERROR'); + + targetImage.src = url; + }.bind(this); + + // Loads the image. If already loaded, then forces a reload. + var startLoad = function() { + if (generation !== this.generation_) + return; + + // Obtain target URL. + if (FileType.isRaw(entry)) { + var timestamp = + item.getMetadataItem() && + item.getMetadataItem().modificationTime && + item.getMetadataItem().modificationTime.getTime(); + ImageLoaderClient.getInstance().load(entry.toURL(), function(result) { + if (generation !== this.generation_) + return; + if (result.status === 'success') + loadImage(result.data); + else + onError('GALLERY_IMAGE_ERROR'); + }.bind(this), { + cache: true, + timestamp: timestamp, + priority: 0 // Use highest priority to show main image. + }); + return; + } + + // Load the image directly. The query parameter is workaround for + // crbug.com/379678, which force to update the contents of the image. + loadImage(entry.toURL() + '?nocache=' + Date.now()); + }.bind(this); + + if (opt_delay) { + this.timeout_ = setTimeout(startLoad, opt_delay); + } else { + startLoad(); + } +}; + +/** + * @return {boolean} True if an image is loading. + */ +ImageUtil.ImageLoader.prototype.isBusy = function() { + return !!this.callback_; +}; + +/** + * @param {Entry} entry Image entry. + * @return {boolean} True if loader loads this image. + */ +ImageUtil.ImageLoader.prototype.isLoading = function(entry) { + return this.isBusy() && util.isSameEntry(this.entry_, entry); +}; + +/** + * @param {function(!HTMLCanvasElement, string=)} callback To be called when the + * image loaded. + */ +ImageUtil.ImageLoader.prototype.setCallback = function(callback) { + this.callback_ = callback; +}; + +/** + * Stops loading image. + */ +ImageUtil.ImageLoader.prototype.cancel = function() { + if (!this.callback_) + return; + this.callback_ = null; + if (this.timeout_) { + clearTimeout(this.timeout_); + this.timeout_ = 0; + } + this.generation_++; // Silence the transform fetcher if it is in progress. +}; + +/** + * @param {!HTMLImageElement} image Image to be transformed. + * @param {!Object} transform transformation description to apply to the image. + * @private + */ +ImageUtil.ImageLoader.prototype.convertImage_ = function(image, transform) { + if (!transform || + (transform.rotate90 === 0 && + transform.scaleX === 1 && + transform.scaleY === 1)) { + setTimeout(this.callback_, 0, image); + this.callback_ = null; + return; + } + var canvas = this.document_.createElement('canvas'); + + if (transform.rotate90 & 1) { // Rotated +/-90deg, swap the dimensions. + canvas.width = image.height; + canvas.height = image.width; + } else { + canvas.width = image.width; + canvas.height = image.height; + } + + var context = canvas.getContext('2d'); + context.save(); + context.translate(canvas.width / 2, canvas.height / 2); + context.rotate(transform.rotate90 * Math.PI / 2); + context.scale(transform.scaleX, transform.scaleY); + + var stripCount = Math.ceil(image.width * image.height / (1 << 21)); + var step = Math.max(16, Math.ceil(image.height / stripCount)) & 0xFFFFF0; + + this.copyStrip_(context, image, 0, step); +}; + +/** + * @param {!CanvasRenderingContext2D} context Context to draw. + * @param {!HTMLImageElement} image Image to draw. + * @param {number} firstRow Number of the first pixel row to draw. + * @param {number} rowCount Count of pixel rows to draw. + * @private + */ +ImageUtil.ImageLoader.prototype.copyStrip_ = function( + context, image, firstRow, rowCount) { + var lastRow = Math.min(firstRow + rowCount, image.height); + + context.drawImage( + image, 0, firstRow, image.width, lastRow - firstRow, + -image.width / 2, firstRow - image.height / 2, + image.width, lastRow - firstRow); + + if (lastRow === image.height) { + context.restore(); + if (this.entry_.toURL().substr(0, 5) !== 'data:') { // Ignore data urls. + metrics.recordInterval(ImageUtil.getMetricName('LoadTime')); + } + setTimeout(this.callback_, 0, context.canvas); + this.callback_ = null; + } else { + var self = this; + this.timeout_ = setTimeout( + function() { + self.timeout_ = 0; + self.copyStrip_(context, image, lastRow, rowCount); + }, 0); + } +}; +
diff --git a/ui/file_manager/gallery/js/image_editor/image_util.js b/ui/file_manager/gallery/js/image_editor/image_util.js index 2218bdc..3f2dded 100644 --- a/ui/file_manager/gallery/js/image_editor/image_util.js +++ b/ui/file_manager/gallery/js/image_editor/image_util.js
@@ -420,267 +420,6 @@ }; /** - * ImageLoader loads an image from a given Entry into a canvas in two steps: - * 1. Loads the image into an HTMLImageElement. - * 2. Copies pixels from HTMLImageElement to HTMLCanvasElement. This is done - * stripe-by-stripe to avoid freezing up the UI. The transform is taken into - * account. - * - * @param {!HTMLDocument} document Owner document. - * @param {!MetadataModel} metadataModel - * @constructor - * @struct - */ -ImageUtil.ImageLoader = function(document, metadataModel) { - this.document_ = document; - - /** - * @private {!MetadataModel} - * @const - */ - this.metadataModel_ = metadataModel; - - this.generation_ = 0; - - /** - * @type {number} - * @private - */ - this.timeout_ = 0; - - /** - * @type {?function(!HTMLCanvasElement, string=)} - * @private - */ - this.callback_ = null; - - /** - * @type {FileEntry} - * @private - */ - this.entry_ = null; -}; - -/** - * Loads an image. - * TODO(mtomasz): Simplify, or even get rid of this class and merge with the - * ThumbnaiLoader class. - * - * @param {!GalleryItem} item Item representing the image to be loaded. - * @param {function(!HTMLCanvasElement, string=)} callback Callback to be - * called when loaded. The second optional argument is an error identifier. - * @param {number=} opt_delay Load delay in milliseconds, useful to let the - * animations play out before the computation heavy image loading starts. - */ -ImageUtil.ImageLoader.prototype.load = function(item, callback, opt_delay) { - var entry = item.getEntry(); - - this.cancel(); - this.entry_ = entry; - this.callback_ = callback; - - var targetImage = assertInstanceof(this.document_.createElement('img'), - HTMLImageElement); - // The transform fetcher is not cancellable so we need a generation counter. - var generation = ++this.generation_; - - /** - * @param {!HTMLImageElement} image Image to be transformed. - * @param {Object=} opt_transform Transformation. - */ - var onTransform = function(image, opt_transform) { - if (generation === this.generation_) { - this.convertImage_(image, opt_transform); - } - }; - onTransform = onTransform.bind(this); - - /** - * @param {string=} opt_error Error. - */ - var onError = function(opt_error) { - targetImage.onerror = null; - targetImage.onload = null; - var tmpCallback = this.callback_; - this.callback_ = null; - var emptyCanvas = assertInstanceof(this.document_.createElement('canvas'), - HTMLCanvasElement); - emptyCanvas.width = 0; - emptyCanvas.height = 0; - tmpCallback(emptyCanvas, opt_error); - }; - onError = onError.bind(this); - - var loadImage = function(url) { - if (generation !== this.generation_) - return; - - ImageUtil.metrics.startInterval(ImageUtil.getMetricName('LoadTime')); - this.timeout_ = 0; - - targetImage.onload = function() { - targetImage.onerror = null; - targetImage.onload = null; - if (generation !== this.generation_) - return; - this.metadataModel_.get([entry], ['contentImageTransform']).then( - function(metadataItems) { - onTransform(targetImage, metadataItems[0].contentImageTransform); - }.bind(this)); - }.bind(this); - - // The error callback has an optional error argument, which in case of a - // general error should not be specified - targetImage.onerror = onError.bind(null, 'GALLERY_IMAGE_ERROR'); - - targetImage.src = url; - }.bind(this); - - // Loads the image. If already loaded, then forces a reload. - var startLoad = function() { - if (generation !== this.generation_) - return; - - // Obtain target URL. - if (FileType.isRaw(entry)) { - var timestamp = - item.getMetadataItem() && - item.getMetadataItem().modificationTime && - item.getMetadataItem().modificationTime.getTime(); - ImageLoaderClient.getInstance().load(entry.toURL(), function(result) { - if (generation !== this.generation_) - return; - if (result.status === 'success') - loadImage(result.data); - else - onError('GALLERY_IMAGE_ERROR'); - }.bind(this), { - cache: true, - timestamp: timestamp, - priority: 0 // Use highest priority to show main image. - }); - return; - } - - // Load the image directly. The query parameter is workaround for - // crbug.com/379678, which force to update the contents of the image. - loadImage(entry.toURL() + '?nocache=' + Date.now()); - }.bind(this); - - if (opt_delay) { - this.timeout_ = setTimeout(startLoad, opt_delay); - } else { - startLoad(); - } -}; - -/** - * @return {boolean} True if an image is loading. - */ -ImageUtil.ImageLoader.prototype.isBusy = function() { - return !!this.callback_; -}; - -/** - * @param {Entry} entry Image entry. - * @return {boolean} True if loader loads this image. - */ -ImageUtil.ImageLoader.prototype.isLoading = function(entry) { - return this.isBusy() && util.isSameEntry(this.entry_, entry); -}; - -/** - * @param {function(!HTMLCanvasElement, string=)} callback To be called when the - * image loaded. - */ -ImageUtil.ImageLoader.prototype.setCallback = function(callback) { - this.callback_ = callback; -}; - -/** - * Stops loading image. - */ -ImageUtil.ImageLoader.prototype.cancel = function() { - if (!this.callback_) - return; - this.callback_ = null; - if (this.timeout_) { - clearTimeout(this.timeout_); - this.timeout_ = 0; - } - this.generation_++; // Silence the transform fetcher if it is in progress. -}; - -/** - * @param {!HTMLImageElement} image Image to be transformed. - * @param {!Object} transform transformation description to apply to the image. - * @private - */ -ImageUtil.ImageLoader.prototype.convertImage_ = function(image, transform) { - if (!transform || - (transform.rotate90 === 0 && - transform.scaleX === 1 && - transform.scaleY === 1)) { - setTimeout(this.callback_, 0, image); - this.callback_ = null; - return; - } - var canvas = this.document_.createElement('canvas'); - - if (transform.rotate90 & 1) { // Rotated +/-90deg, swap the dimensions. - canvas.width = image.height; - canvas.height = image.width; - } else { - canvas.width = image.width; - canvas.height = image.height; - } - - var context = canvas.getContext('2d'); - context.save(); - context.translate(canvas.width / 2, canvas.height / 2); - context.rotate(transform.rotate90 * Math.PI / 2); - context.scale(transform.scaleX, transform.scaleY); - - var stripCount = Math.ceil(image.width * image.height / (1 << 21)); - var step = Math.max(16, Math.ceil(image.height / stripCount)) & 0xFFFFF0; - - this.copyStrip_(context, image, 0, step); -}; - -/** - * @param {!CanvasRenderingContext2D} context Context to draw. - * @param {!HTMLImageElement} image Image to draw. - * @param {number} firstRow Number of the first pixel row to draw. - * @param {number} rowCount Count of pixel rows to draw. - * @private - */ -ImageUtil.ImageLoader.prototype.copyStrip_ = function( - context, image, firstRow, rowCount) { - var lastRow = Math.min(firstRow + rowCount, image.height); - - context.drawImage( - image, 0, firstRow, image.width, lastRow - firstRow, - -image.width / 2, firstRow - image.height / 2, - image.width, lastRow - firstRow); - - if (lastRow === image.height) { - context.restore(); - if (this.entry_.toURL().substr(0, 5) !== 'data:') { // Ignore data urls. - ImageUtil.metrics.recordInterval(ImageUtil.getMetricName('LoadTime')); - } - setTimeout(this.callback_, 0, context.canvas); - this.callback_ = null; - } else { - var self = this; - this.timeout_ = setTimeout( - function() { - self.timeout_ = 0; - self.copyStrip_(context, image, lastRow, rowCount); - }, 0); - } -}; - -/** * @param {!HTMLElement} element To remove children from. */ ImageUtil.removeChildren = function(element) { @@ -712,12 +451,6 @@ }; /** - * Metrics (from metrics.js) itnitialized by the File Manager from owner frame. - * @type {Object} - */ -ImageUtil.metrics = null; - -/** * @param {string} name Local name. * @return {string} Full name. */
diff --git a/ui/file_manager/gallery/js/image_editor/image_view.js b/ui/file_manager/gallery/js/image_editor/image_view.js index f97c899..0c556cb7 100644 --- a/ui/file_manager/gallery/js/image_editor/image_view.js +++ b/ui/file_manager/gallery/js/image_editor/image_view.js
@@ -360,7 +360,7 @@ this.lastLoadTime_ = time; } - ImageUtil.metrics.startInterval(ImageUtil.getMetricName('DisplayTime')); + metrics.startInterval(ImageUtil.getMetricName('DisplayTime')); var self = this; @@ -469,9 +469,9 @@ if (loadType !== ImageView.LoadType.ERROR && loadType !== ImageView.LoadType.CACHED_SCREEN) { - ImageUtil.metrics.recordInterval(ImageUtil.getMetricName('DisplayTime')); + metrics.recordInterval(ImageUtil.getMetricName('DisplayTime')); } - ImageUtil.metrics.recordEnum(ImageUtil.getMetricName('LoadMode'), + metrics.recordEnum(ImageUtil.getMetricName('LoadMode'), loadType, Object.keys(ImageView.LoadType).length); if (loadType === ImageView.LoadType.ERROR &&
diff --git a/ui/file_manager/gallery/js/slide_mode.js b/ui/file_manager/gallery/js/slide_mode.js index 12e1885..75df0b37 100644 --- a/ui/file_manager/gallery/js/slide_mode.js +++ b/ui/file_manager/gallery/js/slide_mode.js
@@ -1039,7 +1039,7 @@ this.errorBanner_.show('GALLERY_IMAGE_OFFLINE'); } - ImageUtil.metrics.recordUserAction(ImageUtil.getMetricName('View')); + metrics.recordUserAction(ImageUtil.getMetricName('View')); var toMillions = function(number) { return Math.round(number / (1000 * 1000)); @@ -1047,19 +1047,19 @@ var metadata = item.getMetadataItem(); if (metadata) { - ImageUtil.metrics.recordSmallCount(ImageUtil.getMetricName('Size.MB'), + metrics.recordSmallCount(ImageUtil.getMetricName('Size.MB'), toMillions(metadata.size)); } var image = this.imageView_.getImage(); - ImageUtil.metrics.recordSmallCount(ImageUtil.getMetricName('Size.MPix'), + metrics.recordSmallCount(ImageUtil.getMetricName('Size.MPix'), toMillions(image.width * image.height)); var extIndex = entry.name.lastIndexOf('.'); var ext = extIndex < 0 ? '' : entry.name.substr(extIndex + 1).toLowerCase(); if (ext === 'jpeg') ext = 'jpg'; - ImageUtil.metrics.recordEnum( + metrics.recordEnum( ImageUtil.getMetricName('FileType'), ext, ImageUtil.FILE_TYPES); // Enable or disable buttons for editing and printing. @@ -1318,7 +1318,7 @@ // Record UMA for the first edit. if (this.imageView_.getContentRevision() === 1) - ImageUtil.metrics.recordUserAction(ImageUtil.getMetricName('Edit')); + metrics.recordUserAction(ImageUtil.getMetricName('Edit')); // Users can change overwrite original setting only if there is no undo // stack and item is original and writable.
diff --git a/ui/webui/resources/js/cr/ui/compiled_resources2.gyp b/ui/webui/resources/js/cr/ui/compiled_resources2.gyp index 130b899..baa48b2 100644 --- a/ui/webui/resources/js/cr/ui/compiled_resources2.gyp +++ b/ui/webui/resources/js/cr/ui/compiled_resources2.gyp
@@ -66,6 +66,14 @@ 'includes': ['../../../../../../third_party/closure_compiler/compile_js2.gypi'], }, { + 'target_name': 'list_selection_model', + 'dependencies': [ + '../../compiled_resources2.gyp:cr', + '../compiled_resources2.gyp:event_target', + ], + 'includes': ['../../../../../../third_party/closure_compiler/compile_js2.gypi'], + }, + { 'target_name': 'menu_button', 'dependencies': [ '../../compiled_resources2.gyp:assert',