diff --git a/DEPS b/DEPS index 97dd5d4..601db6c 100644 --- a/DEPS +++ b/DEPS
@@ -44,7 +44,7 @@ # Three lines of non-changing comments so that # the commit queue can handle CLs rolling V8 # and whatever else without interference from each other. - 'v8_revision': '68d75c82961b0cddb8fdd97e7ffe991d5f2ec215', + 'v8_revision': '07efdd707852ef8b9aca212417e824a13652f8cb', # Three lines of non-changing comments so that # the commit queue can handle CLs rolling swarming_client # and whatever else without interference from each other.
diff --git a/chrome/app/bookmarks_strings.grdp b/chrome/app/bookmarks_strings.grdp index 31d520ee..a3a7135f 100644 --- a/chrome/app/bookmarks_strings.grdp +++ b/chrome/app/bookmarks_strings.grdp
@@ -439,6 +439,7 @@ </message> <message name="IDS_MD_BOOKMARK_MANAGER_TOAST_ITEMS_DELETED" desc="Label displayed in toast popup message when two or more items are deleted."> {COUNT, plural, + =1 {1 bookmark deleted} other {# bookmarks deleted}} </message> <message name="IDS_MD_BOOKMARK_MANAGER_TOAST_URL_COPIED" desc="Label displayed in toast popup message when a URL is copied."> @@ -449,6 +450,7 @@ </message> <message name="IDS_MD_BOOKMARK_MANAGER_TOAST_ITEMS_COPIED" desc="Label displayed in a toast popup message when two or more bookmark items are copied"> {COUNT, plural, + =1 {1 item copied} other {# items copied}} </message> <!-- End of material design Bookmarks Manager strings. -->
diff --git a/chrome/browser/chromeos/arc/arc_service_launcher.cc b/chrome/browser/chromeos/arc/arc_service_launcher.cc index 91e2224..940d8a4 100644 --- a/chrome/browser/chromeos/arc/arc_service_launcher.cc +++ b/chrome/browser/chromeos/arc/arc_service_launcher.cc
@@ -144,6 +144,7 @@ ArcDownloadsWatcherService::GetForBrowserContext(profile); ArcEnterpriseReportingService::GetForBrowserContext(profile); ArcFileSystemMounter::GetForBrowserContext(profile); + ArcFileSystemOperationRunner::GetForBrowserContext(profile); ArcImeService::GetForBrowserContext(profile); ArcIntentHelperBridge::GetForBrowserContext(profile); ArcMetricsService::GetForBrowserContext(profile); @@ -165,10 +166,6 @@ ArcWallpaperService::GetForBrowserContext(profile); GpuArcVideoServiceHost::GetForBrowserContext(profile); - arc_service_manager_->AddService( - base::MakeUnique<ArcFileSystemOperationRunner>( - arc_service_manager_->arc_bridge_service(), profile)); - // Kiosk apps should be run only for kiosk sessions. if (user_manager::UserManager::Get()->IsLoggedInAsArcKioskApp()) { // ArcKioskAppService is BrowserContextKeyedService so that it's destroyed
diff --git a/chrome/browser/chromeos/arc/fileapi/arc_content_file_system_async_file_util_unittest.cc b/chrome/browser/chromeos/arc/fileapi/arc_content_file_system_async_file_util_unittest.cc index 9b95c908..f7702518 100644 --- a/chrome/browser/chromeos/arc/fileapi/arc_content_file_system_async_file_util_unittest.cc +++ b/chrome/browser/chromeos/arc/fileapi/arc_content_file_system_async_file_util_unittest.cc
@@ -15,9 +15,11 @@ #include "chrome/browser/chromeos/arc/fileapi/arc_content_file_system_url_util.h" #include "chrome/browser/chromeos/arc/fileapi/arc_file_system_operation_runner.h" #include "chrome/browser/chromeos/fileapi/external_file_url_util.h" +#include "chrome/test/base/testing_profile.h" #include "components/arc/arc_bridge_service.h" #include "components/arc/arc_service_manager.h" #include "components/arc/test/fake_file_system_instance.h" +#include "components/keyed_service/content/browser_context_keyed_service_factory.h" #include "content/public/test/test_browser_thread_bundle.h" #include "storage/browser/fileapi/file_system_url.h" #include "testing/gtest/include/gtest/gtest.h" @@ -33,6 +35,12 @@ constexpr char kData[] = "abcdef"; constexpr char kMimeType[] = "application/octet-stream"; +std::unique_ptr<KeyedService> CreateArcFileSystemOperationRunnerForTesting( + content::BrowserContext* context) { + return ArcFileSystemOperationRunner::CreateForTesting( + ArcServiceManager::Get()->arc_bridge_service()); +} + class ArcContentFileSystemAsyncFileUtilTest : public testing::Test { public: ArcContentFileSystemAsyncFileUtilTest() = default; @@ -43,11 +51,13 @@ File(kArcUrl, kData, kMimeType, File::Seekable::NO)); arc_service_manager_ = base::MakeUnique<ArcServiceManager>(); - arc_service_manager_->AddService( - ArcFileSystemOperationRunner::CreateForTesting( - arc_service_manager_->arc_bridge_service())); + arc_service_manager_->set_browser_context(&profile_); + ArcFileSystemOperationRunner::GetFactory()->SetTestingFactoryAndUse( + &profile_, &CreateArcFileSystemOperationRunnerForTesting); arc_service_manager_->arc_bridge_service()->file_system()->SetInstance( &fake_file_system_); + + async_file_util_ = base::MakeUnique<ArcContentFileSystemAsyncFileUtil>(); } protected: @@ -64,9 +74,10 @@ } content::TestBrowserThreadBundle thread_bundle_; + TestingProfile profile_; FakeFileSystemInstance fake_file_system_; std::unique_ptr<ArcServiceManager> arc_service_manager_; - ArcContentFileSystemAsyncFileUtil async_file_util_; + std::unique_ptr<ArcContentFileSystemAsyncFileUtil> async_file_util_; private: DISALLOW_COPY_AND_ASSIGN(ArcContentFileSystemAsyncFileUtilTest); @@ -78,7 +89,7 @@ GURL externalfile_url = ArcUrlToExternalFileUrl(GURL(kArcUrl)); base::RunLoop run_loop; - async_file_util_.GetFileInfo( + async_file_util_->GetFileInfo( std::unique_ptr<storage::FileSystemOperationContext>(), ExternalFileURLToFileSystemURL(externalfile_url), -1, // fields
diff --git a/chrome/browser/chromeos/arc/fileapi/arc_content_file_system_file_stream_reader_unittest.cc b/chrome/browser/chromeos/arc/fileapi/arc_content_file_system_file_stream_reader_unittest.cc index 9fd8cb5..be7b0cb 100644 --- a/chrome/browser/chromeos/arc/fileapi/arc_content_file_system_file_stream_reader_unittest.cc +++ b/chrome/browser/chromeos/arc/fileapi/arc_content_file_system_file_stream_reader_unittest.cc
@@ -13,9 +13,11 @@ #include "base/threading/thread_task_runner_handle.h" #include "chrome/browser/chromeos/arc/fileapi/arc_content_file_system_file_stream_reader.h" #include "chrome/browser/chromeos/arc/fileapi/arc_file_system_operation_runner.h" +#include "chrome/test/base/testing_profile.h" #include "components/arc/arc_bridge_service.h" #include "components/arc/arc_service_manager.h" #include "components/arc/test/fake_file_system_instance.h" +#include "components/keyed_service/content/browser_context_keyed_service_factory.h" #include "content/public/test/test_browser_thread_bundle.h" #include "net/base/io_buffer.h" #include "net/base/test_completion_callback.h" @@ -57,6 +59,12 @@ return true; } +std::unique_ptr<KeyedService> CreateFileSystemOperationRunnerForTesting( + content::BrowserContext* context) { + return ArcFileSystemOperationRunner::CreateForTesting( + ArcServiceManager::Get()->arc_bridge_service()); +} + class ArcContentFileSystemFileStreamReaderTest : public testing::Test { public: ArcContentFileSystemFileStreamReaderTest() = default; @@ -70,15 +78,16 @@ File(kArcUrlPipe, kData, kMimeType, File::Seekable::NO)); arc_service_manager_ = base::MakeUnique<ArcServiceManager>(); - arc_service_manager_->AddService( - ArcFileSystemOperationRunner::CreateForTesting( - arc_service_manager_->arc_bridge_service())); + arc_service_manager_->set_browser_context(&profile_); + ArcFileSystemOperationRunner::GetFactory()->SetTestingFactoryAndUse( + &profile_, &CreateFileSystemOperationRunnerForTesting); arc_service_manager_->arc_bridge_service()->file_system()->SetInstance( &fake_file_system_); } private: content::TestBrowserThreadBundle thread_bundle_; + TestingProfile profile_; FakeFileSystemInstance fake_file_system_; std::unique_ptr<ArcServiceManager> arc_service_manager_;
diff --git a/chrome/browser/chromeos/arc/fileapi/arc_documents_provider_root_unittest.cc b/chrome/browser/chromeos/arc/fileapi/arc_documents_provider_root_unittest.cc index e9653d19..b09d223 100644 --- a/chrome/browser/chromeos/arc/fileapi/arc_documents_provider_root_unittest.cc +++ b/chrome/browser/chromeos/arc/fileapi/arc_documents_provider_root_unittest.cc
@@ -14,10 +14,12 @@ #include "chrome/browser/chromeos/arc/fileapi/arc_documents_provider_root.h" #include "chrome/browser/chromeos/arc/fileapi/arc_documents_provider_util.h" #include "chrome/browser/chromeos/arc/fileapi/arc_file_system_operation_runner.h" +#include "chrome/test/base/testing_profile.h" #include "components/arc/arc_bridge_service.h" #include "components/arc/arc_service_manager.h" #include "components/arc/common/file_system.mojom.h" #include "components/arc/test/fake_file_system_instance.h" +#include "components/keyed_service/content/browser_context_keyed_service_factory.h" #include "content/public/test/test_browser_thread_bundle.h" #include "storage/browser/fileapi/watcher_manager.h" #include "storage/common/fileapi/directory_entry.h" @@ -106,6 +108,12 @@ static_cast<uint64_t>(info.creation_time.ToJavaTime())); } +std::unique_ptr<KeyedService> CreateFileSystemOperationRunnerForTesting( + content::BrowserContext* context) { + return ArcFileSystemOperationRunner::CreateForTesting( + ArcServiceManager::Get()->arc_bridge_service()); +} + class ArcDocumentsProviderRootTest : public testing::Test { public: ArcDocumentsProviderRootTest() = default; @@ -117,9 +125,9 @@ } arc_service_manager_ = base::MakeUnique<ArcServiceManager>(); - arc_service_manager_->AddService( - ArcFileSystemOperationRunner::CreateForTesting( - arc_service_manager_->arc_bridge_service())); + arc_service_manager_->set_browser_context(&profile_); + ArcFileSystemOperationRunner::GetFactory()->SetTestingFactoryAndUse( + &profile_, &CreateFileSystemOperationRunnerForTesting); arc_service_manager_->arc_bridge_service()->file_system()->SetInstance( &fake_file_system_); @@ -131,8 +139,15 @@ kRootSpec.document_id); } + void TearDown() override { + root_.reset(); + // Run all pending tasks before destroying testing profile. + base::RunLoop().RunUntilIdle(); + } + protected: content::TestBrowserThreadBundle thread_bundle_; + TestingProfile profile_; FakeFileSystemInstance fake_file_system_; std::unique_ptr<ArcServiceManager> arc_service_manager_; std::unique_ptr<ArcDocumentsProviderRoot> root_;
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 8b604a9..9d8eb84 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
@@ -9,20 +9,53 @@ #include "base/bind.h" #include "base/location.h" #include "base/memory/ptr_util.h" +#include "base/memory/singleton.h" #include "base/optional.h" #include "base/threading/thread_task_runner_handle.h" #include "chrome/browser/chromeos/arc/arc_util.h" +#include "chrome/browser/profiles/profile.h" #include "components/arc/arc_bridge_service.h" +#include "components/arc/arc_browser_context_keyed_service_factory_base.h" #include "content/public/browser/browser_thread.h" #include "url/gurl.h" using content::BrowserThread; namespace arc { +namespace { + +// Singleton factory for ArcFileSystemOperationRunner. +class ArcFileSystemOperationRunnerFactory + : public internal::ArcBrowserContextKeyedServiceFactoryBase< + ArcFileSystemOperationRunner, + ArcFileSystemOperationRunnerFactory> { + public: + // Factory name used by ArcBrowserContextKeyedServiceFactoryBase. + static constexpr const char* kName = "ArcFileSystemOperationRunnerFactory"; + + static ArcFileSystemOperationRunnerFactory* GetInstance() { + return base::Singleton<ArcFileSystemOperationRunnerFactory>::get(); + } + + private: + friend base::DefaultSingletonTraits<ArcFileSystemOperationRunnerFactory>; + ArcFileSystemOperationRunnerFactory() = default; + ~ArcFileSystemOperationRunnerFactory() override = default; +}; + +} // namespace // static -const char ArcFileSystemOperationRunner::kArcServiceName[] = - "arc::ArcFileSystemOperationRunner"; +ArcFileSystemOperationRunner* +ArcFileSystemOperationRunner::GetForBrowserContext( + content::BrowserContext* context) { + return ArcFileSystemOperationRunnerFactory::GetForBrowserContext(context); +} + +// static +BrowserContextKeyedServiceFactory* ArcFileSystemOperationRunner::GetFactory() { + return ArcFileSystemOperationRunnerFactory::GetInstance(); +} // static std::unique_ptr<ArcFileSystemOperationRunner> @@ -31,29 +64,31 @@ // We can't use base::MakeUnique() here because we are calling a private // constructor. return base::WrapUnique<ArcFileSystemOperationRunner>( - new ArcFileSystemOperationRunner(bridge_service, nullptr, false)); + new ArcFileSystemOperationRunner(nullptr, bridge_service, false)); } ArcFileSystemOperationRunner::ArcFileSystemOperationRunner( - ArcBridgeService* bridge_service, - const Profile* profile) - : ArcFileSystemOperationRunner(bridge_service, profile, true) { - DCHECK(profile); + content::BrowserContext* context, + ArcBridgeService* bridge_service) + : ArcFileSystemOperationRunner(Profile::FromBrowserContext(context), + bridge_service, + true) { + DCHECK(context); } ArcFileSystemOperationRunner::ArcFileSystemOperationRunner( + content::BrowserContext* context, ArcBridgeService* bridge_service, - const Profile* profile, bool set_should_defer_by_events) - : ArcService(bridge_service), - profile_(profile), + : context_(context), + arc_bridge_service_(bridge_service), set_should_defer_by_events_(set_should_defer_by_events), binding_(this), weak_ptr_factory_(this) { DCHECK_CURRENTLY_ON(BrowserThread::UI); // We need to observe FileSystemInstance even in unit tests to call Init(). - arc_bridge_service()->file_system()->AddObserver(this); + arc_bridge_service_->file_system()->AddObserver(this); // ArcSessionManager may not exist in unit tests. auto* arc_session_manager = ArcSessionManager::Get(); @@ -70,7 +105,12 @@ if (arc_session_manager) arc_session_manager->RemoveObserver(this); - arc_bridge_service()->file_system()->RemoveObserver(this); + // TODO(hidehiko): Currently, the lifetime of ArcBridgeService and + // BrowserContextKeyedService is not nested. + // If ArcServiceManager::Get() returns nullptr, it is already destructed, + // so do not touch it. + if (ArcServiceManager::Get()) + arc_bridge_service_->file_system()->RemoveObserver(this); // On destruction, deferred operations are discarded. } @@ -95,7 +135,7 @@ return; } auto* file_system_instance = ARC_GET_INSTANCE_FOR_METHOD( - arc_bridge_service()->file_system(), GetFileSize); + arc_bridge_service_->file_system(), GetFileSize); if (!file_system_instance) { base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, base::BindOnce(callback, -1)); @@ -115,7 +155,7 @@ return; } auto* file_system_instance = ARC_GET_INSTANCE_FOR_METHOD( - arc_bridge_service()->file_system(), GetMimeType); + arc_bridge_service_->file_system(), GetMimeType); if (!file_system_instance) { base::ThreadTaskRunnerHandle::Get()->PostTask( FROM_HERE, base::BindOnce(callback, base::nullopt)); @@ -135,7 +175,7 @@ return; } auto* file_system_instance = ARC_GET_INSTANCE_FOR_METHOD( - arc_bridge_service()->file_system(), OpenFileToRead); + arc_bridge_service_->file_system(), OpenFileToRead); if (!file_system_instance) { base::ThreadTaskRunnerHandle::Get()->PostTask( FROM_HERE, @@ -157,7 +197,7 @@ return; } auto* file_system_instance = ARC_GET_INSTANCE_FOR_METHOD( - arc_bridge_service()->file_system(), GetDocument); + arc_bridge_service_->file_system(), GetDocument); if (!file_system_instance) { base::ThreadTaskRunnerHandle::Get()->PostTask( FROM_HERE, @@ -180,7 +220,7 @@ return; } auto* file_system_instance = ARC_GET_INSTANCE_FOR_METHOD( - arc_bridge_service()->file_system(), GetChildDocuments); + arc_bridge_service_->file_system(), GetChildDocuments); if (!file_system_instance) { base::ThreadTaskRunnerHandle::Get()->PostTask( FROM_HERE, base::BindOnce(callback, base::nullopt)); @@ -204,7 +244,7 @@ return; } auto* file_system_instance = ARC_GET_INSTANCE_FOR_METHOD( - arc_bridge_service()->file_system(), AddWatcher); + arc_bridge_service_->file_system(), AddWatcher); if (!file_system_instance) { base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, base::BindOnce(callback, -1)); @@ -240,7 +280,7 @@ watcher_callbacks_.erase(iter); auto* file_system_instance = ARC_GET_INSTANCE_FOR_METHOD( - arc_bridge_service()->file_system(), AddWatcher); + arc_bridge_service_->file_system(), AddWatcher); if (!file_system_instance) { base::ThreadTaskRunnerHandle::Get()->PostTask( FROM_HERE, base::BindOnce(callback, false)); @@ -270,7 +310,7 @@ void ArcFileSystemOperationRunner::OnInstanceReady() { DCHECK_CURRENTLY_ON(BrowserThread::UI); auto* file_system_instance = - ARC_GET_INSTANCE_FOR_METHOD(arc_bridge_service()->file_system(), Init); + ARC_GET_INSTANCE_FOR_METHOD(arc_bridge_service_->file_system(), Init); if (file_system_instance) { mojom::FileSystemHostPtr host_proxy; binding_.Bind(mojo::MakeRequest(&host_proxy)); @@ -309,8 +349,9 @@ void ArcFileSystemOperationRunner::OnStateChanged() { DCHECK_CURRENTLY_ON(BrowserThread::UI); if (set_should_defer_by_events_) { - SetShouldDefer(IsArcPlayStoreEnabledForProfile(profile_) && - !arc_bridge_service()->file_system()->has_instance()); + SetShouldDefer(IsArcPlayStoreEnabledForProfile( + Profile::FromBrowserContext(context_)) && + !arc_bridge_service_->file_system()->has_instance()); } }
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 6f40f97..4db37091 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
@@ -20,11 +20,20 @@ #include "components/arc/arc_service.h" #include "components/arc/common/file_system.mojom.h" #include "components/arc/instance_holder.h" +#include "components/keyed_service/core/keyed_service.h" #include "mojo/public/cpp/bindings/binding.h" #include "storage/browser/fileapi/watcher_manager.h" +class BrowserContextKeyedServiceFactory; + +namespace content { +class BrowserContext; +} // namespace content + namespace arc { +class ArcBridgeService; + // Runs ARC file system operations. // // This is an abstraction layer on top of mojom::FileSystemInstance. All ARC @@ -49,7 +58,7 @@ // // All member functions must be called on the UI thread. class ArcFileSystemOperationRunner - : public ArcService, + : public KeyedService, public mojom::FileSystemHost, public ArcSessionManager::Observer, public InstanceHolder<mojom::FileSystemInstance>::Observer { @@ -78,8 +87,10 @@ virtual ~Observer() = default; }; - // For supporting ArcServiceManager::GetService<T>(). - static const char kArcServiceName[]; + // Returns singleton instance for the given BrowserContext, + // or nullptr if the browser |context| is not allowed to use ARC. + static ArcFileSystemOperationRunner* GetForBrowserContext( + content::BrowserContext* context); // Creates an instance suitable for unit tests. // This instance will run all operations immediately without deferring by @@ -88,10 +99,11 @@ static std::unique_ptr<ArcFileSystemOperationRunner> CreateForTesting( ArcBridgeService* bridge_service); - // The standard constructor. A production instance should be created by - // this constructor. - ArcFileSystemOperationRunner(ArcBridgeService* bridge_service, - const Profile* profile); + // Returns Factory instance for ArcFileSystemOperationRunner. + static BrowserContextKeyedServiceFactory* GetFactory(); + + ArcFileSystemOperationRunner(content::BrowserContext* context, + ArcBridgeService* bridge_service); ~ArcFileSystemOperationRunner() override; // Adds or removes observers. @@ -127,8 +139,8 @@ private: friend class ArcFileSystemOperationRunnerTest; - ArcFileSystemOperationRunner(ArcBridgeService* bridge_service, - const Profile* profile, + ArcFileSystemOperationRunner(content::BrowserContext* context, + ArcBridgeService* bridge_service, bool set_should_defer_by_events); void OnWatcherAdded(const WatcherCallback& watcher_callback, @@ -143,9 +155,9 @@ // deferring. void SetShouldDefer(bool should_defer); - // Profile this runner is associated with. This can be nullptr in - // unit tests. - const Profile* const profile_; + // Maybe nullptr in unittests. + content::BrowserContext* const context_; + ArcBridgeService* const arc_bridge_service_; // Owned by ArcServiceManager // Indicates if this instance should enable/disable deferring by events. // Usually true, but set to false in unit tests.
diff --git a/chrome/browser/chromeos/arc/fileapi/arc_file_system_operation_runner_unittest.cc b/chrome/browser/chromeos/arc/fileapi/arc_file_system_operation_runner_unittest.cc index c6aa025..355b081 100644 --- a/chrome/browser/chromeos/arc/fileapi/arc_file_system_operation_runner_unittest.cc +++ b/chrome/browser/chromeos/arc/fileapi/arc_file_system_operation_runner_unittest.cc
@@ -36,17 +36,14 @@ void SetUp() override { arc_service_manager_ = base::MakeUnique<ArcServiceManager>(); + runner_ = ArcFileSystemOperationRunner::CreateForTesting( + arc_service_manager_->arc_bridge_service()); arc_service_manager_->arc_bridge_service()->file_system()->SetInstance( &file_system_instance_); - arc_service_manager_->AddService( - ArcFileSystemOperationRunner::CreateForTesting( - arc_service_manager_->arc_bridge_service())); // Run the message loop until FileSystemInstance::Init() is called. base::RunLoop().RunUntilIdle(); ASSERT_TRUE(file_system_instance_.InitCalled()); - - runner_ = arc_service_manager_->GetService<ArcFileSystemOperationRunner>(); } protected: @@ -100,8 +97,7 @@ content::TestBrowserThreadBundle thread_bundle_; FakeFileSystemInstance file_system_instance_; std::unique_ptr<ArcServiceManager> arc_service_manager_; - // Owned by |arc_service_manager_|. - ArcFileSystemOperationRunner* runner_; + std::unique_ptr<ArcFileSystemOperationRunner> runner_; private: DISALLOW_COPY_AND_ASSIGN(ArcFileSystemOperationRunnerTest);
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 424894bfd..7a2d583 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
@@ -19,6 +19,13 @@ namespace { +// TODO(crbug.com/745648): Use correct BrowserContext. +ArcFileSystemOperationRunner* GetArcFileSystemOperationRunner() { + DCHECK_CURRENTLY_ON(BrowserThread::UI); + return ArcFileSystemOperationRunner::GetForBrowserContext( + ArcServiceManager::Get()->browser_context()); +} + template <typename T> void PostToIOThread(const base::Callback<void(T)>& callback, T result) { DCHECK_CURRENTLY_ON(BrowserThread::UI); @@ -30,8 +37,7 @@ void GetFileSizeOnUIThread(const GURL& url, const GetFileSizeCallback& callback) { DCHECK_CURRENTLY_ON(BrowserThread::UI); - auto* runner = - ArcServiceManager::GetGlobalService<ArcFileSystemOperationRunner>(); + auto* runner = GetArcFileSystemOperationRunner(); if (!runner) { DLOG(ERROR) << "ArcFileSystemOperationRunner unavailable. " << "File system operations are dropped."; @@ -44,8 +50,7 @@ void OpenFileToReadOnUIThread(const GURL& url, const OpenFileToReadCallback& callback) { DCHECK_CURRENTLY_ON(BrowserThread::UI); - auto* runner = - ArcServiceManager::GetGlobalService<ArcFileSystemOperationRunner>(); + auto* runner = GetArcFileSystemOperationRunner(); if (!runner) { DLOG(ERROR) << "ArcFileSystemOperationRunner unavailable. " << "File system operations are dropped."; @@ -59,8 +64,7 @@ const std::string& document_id, const GetDocumentCallback& callback) { DCHECK_CURRENTLY_ON(BrowserThread::UI); - auto* runner = - ArcServiceManager::GetGlobalService<ArcFileSystemOperationRunner>(); + auto* runner = GetArcFileSystemOperationRunner(); if (!runner) { DLOG(ERROR) << "ArcFileSystemOperationRunner unavailable. " << "File system operations are dropped."; @@ -74,8 +78,7 @@ const std::string& parent_document_id, const GetChildDocumentsCallback& callback) { DCHECK_CURRENTLY_ON(BrowserThread::UI); - auto* runner = - ArcServiceManager::GetGlobalService<ArcFileSystemOperationRunner>(); + auto* runner = GetArcFileSystemOperationRunner(); if (!runner) { DLOG(ERROR) << "ArcFileSystemOperationRunner unavailable. " << "File system operations are dropped."; @@ -90,8 +93,7 @@ const WatcherCallback& watcher_callback, const AddWatcherCallback& callback) { DCHECK_CURRENTLY_ON(BrowserThread::UI); - auto* runner = - ArcServiceManager::GetGlobalService<ArcFileSystemOperationRunner>(); + auto* runner = GetArcFileSystemOperationRunner(); if (!runner) { DLOG(ERROR) << "ArcFileSystemOperationRunner unavailable. " << "File system operations are dropped."; @@ -104,8 +106,7 @@ void RemoveWatcherOnUIThread(int64_t watcher_id, const RemoveWatcherCallback& callback) { DCHECK_CURRENTLY_ON(BrowserThread::UI); - auto* runner = - ArcServiceManager::GetGlobalService<ArcFileSystemOperationRunner>(); + auto* runner = GetArcFileSystemOperationRunner(); if (!runner) { DLOG(ERROR) << "ArcFileSystemOperationRunner unavailable. " << "File system operations are dropped."; @@ -117,8 +118,7 @@ void AddObserverOnUIThread(scoped_refptr<ObserverIOThreadWrapper> observer) { DCHECK_CURRENTLY_ON(BrowserThread::UI); - auto* runner = - ArcServiceManager::GetGlobalService<ArcFileSystemOperationRunner>(); + auto* runner = GetArcFileSystemOperationRunner(); if (!runner) { DLOG(ERROR) << "ArcFileSystemOperationRunner unavailable. " << "File system operations are dropped."; @@ -129,8 +129,7 @@ void RemoveObserverOnUIThread(scoped_refptr<ObserverIOThreadWrapper> observer) { DCHECK_CURRENTLY_ON(BrowserThread::UI); - auto* runner = - ArcServiceManager::GetGlobalService<ArcFileSystemOperationRunner>(); + auto* runner = GetArcFileSystemOperationRunner(); if (!runner) { DLOG(ERROR) << "ArcFileSystemOperationRunner unavailable. " << "File system operations are dropped.";
diff --git a/chrome/browser/chromeos/file_manager/filesystem_api_util.cc b/chrome/browser/chromeos/file_manager/filesystem_api_util.cc index 8a435cb..61731e9 100644 --- a/chrome/browser/chromeos/file_manager/filesystem_api_util.cc +++ b/chrome/browser/chromeos/file_manager/filesystem_api_util.cc
@@ -201,8 +201,8 @@ if (arc::IsArcAllowedForProfile(profile) && base::FilePath(arc::kContentFileSystemMountPointPath).IsParent(path)) { GURL arc_url = arc::PathToArcUrl(path); - auto* runner = arc::ArcServiceManager::GetGlobalService< - arc::ArcFileSystemOperationRunner>(); + auto* runner = + arc::ArcFileSystemOperationRunner::GetForBrowserContext(profile); if (!runner) { content::BrowserThread::PostTask( content::BrowserThread::UI, FROM_HERE,
diff --git a/chrome/browser/chromeos/net/tether_notification_presenter.cc b/chrome/browser/chromeos/net/tether_notification_presenter.cc index fa49f0a0..5f477a3 100644 --- a/chrome/browser/chromeos/net/tether_notification_presenter.cc +++ b/chrome/browser/chromeos/net/tether_notification_presenter.cc
@@ -74,14 +74,6 @@ "cros_tether_notification_ids.setup_required"; // static -constexpr const std::array<const char* const, 4> - TetherNotificationPresenter::kIdsWhichOpenSettingsOnClick{ - {TetherNotificationPresenter::kTetherNotifierId, - TetherNotificationPresenter::kActiveHostNotificationId, - TetherNotificationPresenter::kPotentialHotspotNotificationId, - TetherNotificationPresenter::kSetupRequiredNotificationId}}; - -// static std::unique_ptr<message_center::Notification> TetherNotificationPresenter::CreateNotificationWithMediumSignalStrengthIcon( const std::string& id, @@ -217,13 +209,7 @@ void TetherNotificationPresenter::OnNotificationClicked( const std::string& notification_id) { - auto* const* const it = std::find_if( - kIdsWhichOpenSettingsOnClick.begin(), kIdsWhichOpenSettingsOnClick.end(), - [notification_id](const char* id) { return notification_id == id; }); - if (it == kIdsWhichOpenSettingsOnClick.end()) { - return; - } - + PA_LOG(INFO) << "Notification with ID " << notification_id << " was clicked."; settings_ui_delegate_->ShowSettingsSubPageForProfile(profile_, kTetherSettingsSubpage); message_center_->RemoveNotification(notification_id, true /* by_user */);
diff --git a/chrome/browser/chromeos/net/tether_notification_presenter.h b/chrome/browser/chromeos/net/tether_notification_presenter.h index 743491b..e883f987c 100644 --- a/chrome/browser/chromeos/net/tether_notification_presenter.h +++ b/chrome/browser/chromeos/net/tether_notification_presenter.h
@@ -71,15 +71,11 @@ }; private: - // IDs associated with Tether notification types. static const char kTetherNotifierId[]; static const char kPotentialHotspotNotificationId[]; static const char kActiveHostNotificationId[]; static const char kSetupRequiredNotificationId[]; - // IDs of all notifications which, when clicked, open mobile data settings. - static const std::array<const char* const, 4> kIdsWhichOpenSettingsOnClick; - static std::unique_ptr<message_center::Notification> CreateNotificationWithMediumSignalStrengthIcon(const std::string& id, const base::string16& title,
diff --git a/chrome/browser/chromeos/net/tether_notification_presenter_unittest.cc b/chrome/browser/chromeos/net/tether_notification_presenter_unittest.cc index 2bb8dbee..a5cd9f8 100644 --- a/chrome/browser/chromeos/net/tether_notification_presenter_unittest.cc +++ b/chrome/browser/chromeos/net/tether_notification_presenter_unittest.cc
@@ -410,12 +410,6 @@ VerifySettingsNotOpened(); } -TEST_F(TetherNotificationPresenterTest, - TestDoesNotOpenSettingsWhenOtherNotificationClicked) { - test_message_center_->NotifyNotificationTapped("otherNotificationId"); - VerifySettingsNotOpened(); -} - } // namespace tether } // namespace chromeos
diff --git a/chrome/browser/offline_pages/prefetch/offline_prefetch_download_client.cc b/chrome/browser/offline_pages/prefetch/offline_prefetch_download_client.cc index 2eeeb74..d541f71 100644 --- a/chrome/browser/offline_pages/prefetch/offline_prefetch_download_client.cc +++ b/chrome/browser/offline_pages/prefetch/offline_prefetch_download_client.cc
@@ -19,8 +19,11 @@ OfflinePrefetchDownloadClient::~OfflinePrefetchDownloadClient() = default; void OfflinePrefetchDownloadClient::OnServiceInitialized( + bool state_lost, const std::vector<std::string>& outstanding_download_guids) { // TODO(jianli): Remove orphaned downloads. + // TODO(jianli, dtrainor): Handle service corruption and recovery if + // |state_lost| is |true|. PrefetchDownloader* downloader = GetPrefetchDownloader(); if (downloader) downloader->OnDownloadServiceReady();
diff --git a/chrome/browser/offline_pages/prefetch/offline_prefetch_download_client.h b/chrome/browser/offline_pages/prefetch/offline_prefetch_download_client.h index 197c22f..867723d 100644 --- a/chrome/browser/offline_pages/prefetch/offline_prefetch_download_client.h +++ b/chrome/browser/offline_pages/prefetch/offline_prefetch_download_client.h
@@ -24,6 +24,7 @@ private: // Overridden from Client: void OnServiceInitialized( + bool state_lost, const std::vector<std::string>& outstanding_download_guids) override; void OnServiceUnavailable() override; download::Client::ShouldDownload OnDownloadStarted(
diff --git a/chrome/browser/permissions/permission_request_manager.cc b/chrome/browser/permissions/permission_request_manager.cc index 8686773..678c50a 100644 --- a/chrome/browser/permissions/permission_request_manager.cc +++ b/chrome/browser/permissions/permission_request_manager.cc
@@ -194,19 +194,20 @@ continue; // We can simply erase the current entry in the request table if we aren't - // showing the dialog, or if we are showing it and it can accept the update. - bool can_erase = view_->CanAcceptRequestUpdate(); - if (can_erase) { - DeleteBubble(); + // showing the dialog (because we switched tabs with an active prompt), or + // if we are showing it and it can accept the update. + if (!view_ || view_->CanAcceptRequestUpdate()) { + // TODO(timloh): We should fix this at some point. + // Grouped (mic+camera) requests are currently never cancelled. + DCHECK_EQ(static_cast<size_t>(1), requests_.size()); + + if (view_) + DeleteBubble(); RequestFinishedIncludingDuplicates(*requests_iter); requests_.erase(requests_iter); - if (requests_.empty()) - DequeueRequestsAndShowBubble(); - else - ShowBubble(); - + DequeueRequestsAndShowBubble(); return; }
diff --git a/chrome/browser/permissions/permission_request_manager_unittest.cc b/chrome/browser/permissions/permission_request_manager_unittest.cc index 1d9f2207..4acacd9 100644 --- a/chrome/browser/permissions/permission_request_manager_unittest.cc +++ b/chrome/browser/permissions/permission_request_manager_unittest.cc
@@ -194,6 +194,17 @@ EXPECT_TRUE(request_camera_.granted()); } +TEST_F(PermissionRequestManagerTest, CancelAfterTabSwitch) { + manager_->AddRequest(&request1_); + manager_->DisplayPendingRequests(); + WaitForBubbleToBeShown(); + EXPECT_TRUE(prompt_factory_->is_visible()); + MockTabSwitchAway(); + EXPECT_FALSE(prompt_factory_->is_visible()); + manager_->CancelRequest(&request1_); + EXPECT_TRUE(request1_.finished()); +} + TEST_F(PermissionRequestManagerTest, NoRequests) { manager_->DisplayPendingRequests(); WaitForBubbleToBeShown();
diff --git a/chrome/browser/signin/easy_unlock_service_regular.cc b/chrome/browser/signin/easy_unlock_service_regular.cc index f94be93..9732c48e 100644 --- a/chrome/browser/signin/easy_unlock_service_regular.cc +++ b/chrome/browser/signin/easy_unlock_service_regular.cc
@@ -192,6 +192,12 @@ void EasyUnlockServiceRegular::LaunchSetup() { DCHECK_CURRENTLY_ON(content::BrowserThread::UI); #if defined(OS_CHROMEOS) + // TODO(tengs): To keep login working for existing EasyUnlock users, we need + // to explicitly disable login here for new users who set up EasyUnlock. + // After a sufficient number of releases, we should make the default value + // false. + pref_manager_->SetIsChromeOSLoginEnabled(false); + // Force the user to reauthenticate by showing a modal overlay (similar to the // lock screen). The password obtained from the reauth is cached for a short // period of time and used to create the cryptohome keys for sign-in.
diff --git a/chrome/browser/ui/views/ime_driver/input_method_bridge_chromeos.cc b/chrome/browser/ui/views/ime_driver/input_method_bridge_chromeos.cc index 991f1e63..9427fe30 100644 --- a/chrome/browser/ui/views/ime_driver/input_method_bridge_chromeos.cc +++ b/chrome/browser/ui/views/ime_driver/input_method_bridge_chromeos.cc
@@ -34,8 +34,7 @@ DCHECK(event->IsKeyEvent()); ui::KeyEvent* key_event = event->AsKeyEvent(); if (!key_event->is_char()) { - input_method_chromeos_->DispatchKeyEvent( - key_event, base::MakeUnique<base::Callback<void(bool)>>(callback)); + input_method_chromeos_->DispatchKeyEvent(key_event, std::move(callback)); } else { const bool handled = false; callback.Run(handled);
diff --git a/components/download/internal/controller_impl.cc b/components/download/internal/controller_impl.cc index 692080ba..408f0b6 100644 --- a/components/download/internal/controller_impl.cc +++ b/components/download/internal/controller_impl.cc
@@ -322,7 +322,11 @@ AttemptToFinalizeSetup(); } -void ControllerImpl::OnDriverHardRecoverComplete(bool success) {} +void ControllerImpl::OnDriverHardRecoverComplete(bool success) { + DCHECK(!startup_status_.driver_ok.has_value()); + startup_status_.driver_ok = success; + AttemptToFinalizeSetup(); +} void ControllerImpl::OnDownloadCreated(const DriverEntry& download) { if (controller_state_ != State::READY) @@ -404,14 +408,22 @@ AttemptToFinalizeSetup(); } +void ControllerImpl::OnFileMonitorHardRecoverComplete(bool success) { + DCHECK(!startup_status_.file_monitor_ok.has_value()); + startup_status_.file_monitor_ok = success; + AttemptToFinalizeSetup(); +} + void ControllerImpl::OnModelReady(bool success) { DCHECK(!startup_status_.model_ok.has_value()); startup_status_.model_ok = success; AttemptToFinalizeSetup(); } -void ControllerImpl::OnHardRecoverComplete(bool success) { - // TODO(dtrainor): Support recovery. +void ControllerImpl::OnModelHardRecoverComplete(bool success) { + DCHECK(!startup_status_.model_ok.has_value()); + startup_status_.model_ok = success; + AttemptToFinalizeSetup(); } void ControllerImpl::OnItemAdded(bool success, @@ -469,23 +481,23 @@ } void ControllerImpl::AttemptToFinalizeSetup() { - DCHECK_EQ(controller_state_, State::INITIALIZING); + DCHECK(controller_state_ == State::INITIALIZING || + controller_state_ == State::RECOVERING); if (!startup_status_.Complete()) return; - stats::LogControllerStartupStatus(startup_status_); + bool in_recovery = controller_state_ == State::RECOVERING; + + stats::LogControllerStartupStatus(in_recovery, startup_status_); if (!startup_status_.Ok()) { - // TODO(dtrainor): Recover here. Try to clean up any disk state and, if - // possible, any DownloadDriver data and continue with initialization? - controller_state_ = State::UNAVAILABLE; + if (in_recovery) { + HandleUnrecoverableSetup(); + NotifyServiceOfStartup(); + } else { + StartHardRecoveryAttempt(); + } - // If we cannot recover, notify Clients that the service is unavailable. - base::ThreadTaskRunnerHandle::Get()->PostTask( - FROM_HERE, base::Bind(&ControllerImpl::SendOnServiceUnavailable, - weak_ptr_factory_.GetWeakPtr())); - - NotifyServiceOfStartup(); return; } @@ -496,7 +508,7 @@ RemoveCleanupEligibleDownloads(); ResolveInitialRequestStates(); - NotifyClientsOfStartup(); + NotifyClientsOfStartup(in_recovery); controller_state_ = State::READY; @@ -509,8 +521,29 @@ ActivateMoreDownloads(); } +void ControllerImpl::HandleUnrecoverableSetup() { + controller_state_ = State::UNAVAILABLE; + + // If we cannot recover, notify Clients that the service is unavailable. + base::ThreadTaskRunnerHandle::Get()->PostTask( + FROM_HERE, base::Bind(&ControllerImpl::SendOnServiceUnavailable, + weak_ptr_factory_.GetWeakPtr())); +} + +void ControllerImpl::StartHardRecoveryAttempt() { + startup_status_.Reset(); + controller_state_ = State::RECOVERING; + + driver_->HardRecover(); + model_->HardRecover(); + file_monitor_->HardRecover( + base::Bind(&ControllerImpl::OnFileMonitorHardRecoverComplete, + weak_ptr_factory_.GetWeakPtr())); +} + void ControllerImpl::PollActiveDriverDownloads() { - DCHECK_EQ(controller_state_, State::INITIALIZING); + DCHECK(controller_state_ == State::INITIALIZING || + controller_state_ == State::RECOVERING); std::set<std::string> guids = driver_->GetActiveDownloads(); @@ -521,7 +554,8 @@ } void ControllerImpl::CancelOrphanedRequests() { - DCHECK_EQ(controller_state_, State::INITIALIZING); + DCHECK(controller_state_ == State::INITIALIZING || + controller_state_ == State::RECOVERING); auto entries = model_->PeekEntries(); @@ -544,7 +578,8 @@ } void ControllerImpl::CleanupUnknownFiles() { - DCHECK_EQ(controller_state_, State::INITIALIZING); + DCHECK(controller_state_ == State::INITIALIZING || + controller_state_ == State::RECOVERING); auto entries = model_->PeekEntries(); std::vector<DriverEntry> driver_entries; @@ -558,7 +593,8 @@ } void ControllerImpl::ResolveInitialRequestStates() { - DCHECK_EQ(controller_state_, State::INITIALIZING); + DCHECK(controller_state_ == State::INITIALIZING || + controller_state_ == State::RECOVERING); auto entries = model_->PeekEntries(); for (auto* entry : entries) { @@ -731,7 +767,7 @@ } } -void ControllerImpl::NotifyClientsOfStartup() { +void ControllerImpl::NotifyClientsOfStartup(bool state_lost) { std::set<Entry::State> ignored_states = {Entry::State::COMPLETE}; auto categorized = util::MapEntriesToClients( clients_->GetRegisteredClients(), model_->PeekEntries(), ignored_states); @@ -740,7 +776,7 @@ base::ThreadTaskRunnerHandle::Get()->PostTask( FROM_HERE, base::Bind(&ControllerImpl::SendOnServiceInitialized, weak_ptr_factory_.GetWeakPtr(), client_id, - categorized[client_id])); + state_lost, categorized[client_id])); } } @@ -938,10 +974,11 @@ void ControllerImpl::SendOnServiceInitialized( DownloadClient client_id, + bool state_lost, const std::vector<std::string>& guids) { auto* client = clients_->GetClient(client_id); DCHECK(client); - client->OnServiceInitialized(guids); + client->OnServiceInitialized(state_lost, guids); } void ControllerImpl::SendOnServiceUnavailable() {
diff --git a/components/download/internal/controller_impl.h b/components/download/internal/controller_impl.h index ac21583..2b644de 100644 --- a/components/download/internal/controller_impl.h +++ b/components/download/internal/controller_impl.h
@@ -80,7 +80,7 @@ // Model::Client implementation. void OnModelReady(bool success) override; - void OnHardRecoverComplete(bool success) override; + void OnModelHardRecoverComplete(bool success) override; void OnItemAdded(bool success, DownloadClient client, const std::string& guid) override; @@ -94,6 +94,9 @@ // Called when the file monitor and download file directory are initialized. void OnFileMonitorReady(bool success); + // Called when the file monitor finishes attempting to recover itself. + void OnFileMonitorHardRecoverComplete(bool success); + // DeviceStatusListener::Observer implementation. void OnDeviceStatusChanged(const DeviceStatus& device_status) override; @@ -101,6 +104,14 @@ // internal state initialization. void AttemptToFinalizeSetup(); + // Called when setup and recovery failed. Shuts down the service and notifies + // the Clients. + void HandleUnrecoverableSetup(); + + // If initialization failed, try to reset the state of all components and + // restart them. If that attempt fails the service will be unavailable. + void StartHardRecoveryAttempt(); + // Checks for all the currently active driver downloads. This lets us know // which ones are active that we haven't tracked. void PollActiveDriverDownloads(); @@ -127,7 +138,7 @@ // Notifies all Client in |clients_| that this controller is initialized and // lets them know which download requests we are aware of for their // DownloadClient. - void NotifyClientsOfStartup(); + void NotifyClientsOfStartup(bool state_lost); // Notifies the service that the startup has completed so that it can start // processing any pending requests. @@ -161,6 +172,7 @@ // Postable methods meant to just be pass throughs to Client APIs. This is // meant to help prevent reentrancy. void SendOnServiceInitialized(DownloadClient client_id, + bool state_lost, const std::vector<std::string>& guids); void SendOnServiceUnavailable(); void SendOnDownloadUpdated(DownloadClient client_id,
diff --git a/components/download/internal/controller_impl_unittest.cc b/components/download/internal/controller_impl_unittest.cc index 15e52b382..88dce235 100644 --- a/components/download/internal/controller_impl_unittest.cc +++ b/components/download/internal/controller_impl_unittest.cc
@@ -83,6 +83,9 @@ MockFileMonitor() = default; ~MockFileMonitor() override = default; + void TriggerInit(bool success); + void TriggerHardRecover(bool success); + void Initialize(const FileMonitor::InitCallback& callback) override; MOCK_METHOD2(DeleteUnknownFiles, void(const Model::EntryList&, const std::vector<DriverEntry>&)); @@ -91,11 +94,27 @@ const base::Closure&)); MOCK_METHOD2(DeleteFiles, void(const std::set<base::FilePath>&, stats::FileCleanupReason)); - MOCK_METHOD1(HardRecover, void(const FileMonitor::InitCallback&)); + void HardRecover(const FileMonitor::InitCallback&) override; + + private: + FileMonitor::InitCallback init_callback_; + FileMonitor::InitCallback recover_callback_; }; +void MockFileMonitor::TriggerInit(bool success) { + init_callback_.Run(success); +} + +void MockFileMonitor::TriggerHardRecover(bool success) { + recover_callback_.Run(success); +} + void MockFileMonitor::Initialize(const FileMonitor::InitCallback& callback) { - callback.Run(true); + init_callback_ = callback; +} + +void MockFileMonitor::HardRecover(const FileMonitor::InitCallback& callback) { + recover_callback_ = callback; } class DownloadServiceControllerImplTest : public testing::Test { @@ -208,8 +227,7 @@ TEST_F(DownloadServiceControllerImplTest, SuccessfulInitModelFirst) { base::HistogramTester histogram_tester; - - EXPECT_CALL(*client_, OnServiceInitialized(_)).Times(0); + EXPECT_CALL(*client_, OnServiceInitialized(false, _)).Times(0); EXPECT_EQ(controller_->GetState(), Controller::State::CREATED); InitializeController(); @@ -217,9 +235,10 @@ EXPECT_EQ(controller_->GetState(), Controller::State::INITIALIZING); store_->TriggerInit(true, base::MakeUnique<std::vector<Entry>>()); + file_monitor_->TriggerInit(true); EXPECT_EQ(controller_->GetState(), Controller::State::INITIALIZING); - EXPECT_CALL(*client_, OnServiceInitialized(_)).Times(1); + EXPECT_CALL(*client_, OnServiceInitialized(false, _)).Times(1); EXPECT_CALL(*scheduler_, Next(_, _)).Times(1); EXPECT_CALL(*scheduler_, Reschedule(_)).Times(1); @@ -229,13 +248,14 @@ task_runner_->RunUntilIdle(); histogram_tester.ExpectBucketCount( - "Download.Service.StartUpStatus", + "Download.Service.StartUpStatus.Initialization", static_cast<base::HistogramBase::Sample>(stats::StartUpResult::SUCCESS), 1); } TEST_F(DownloadServiceControllerImplTest, SuccessfulInitDriverFirst) { - EXPECT_CALL(*client_, OnServiceInitialized(_)).Times(0); + base::HistogramTester histogram_tester; + EXPECT_CALL(*client_, OnServiceInitialized(false, _)).Times(0); EXPECT_EQ(controller_->GetState(), Controller::State::CREATED); InitializeController(); @@ -246,15 +266,122 @@ EXPECT_FALSE(init_callback_called_); EXPECT_EQ(controller_->GetState(), Controller::State::INITIALIZING); - EXPECT_CALL(*client_, OnServiceInitialized(_)).Times(1); + EXPECT_CALL(*client_, OnServiceInitialized(false, _)).Times(1); EXPECT_CALL(*scheduler_, Next(_, _)).Times(1); EXPECT_CALL(*scheduler_, Reschedule(_)).Times(1); store_->TriggerInit(true, base::MakeUnique<std::vector<Entry>>()); + file_monitor_->TriggerInit(true); EXPECT_EQ(controller_->GetState(), Controller::State::READY); task_runner_->RunUntilIdle(); EXPECT_TRUE(init_callback_called_); + + histogram_tester.ExpectBucketCount( + "Download.Service.StartUpStatus.Initialization", + static_cast<base::HistogramBase::Sample>(stats::StartUpResult::SUCCESS), + 1); +} + +TEST_F(DownloadServiceControllerImplTest, HardRecoveryAfterFailedModel) { + base::HistogramTester histogram_tester; + EXPECT_CALL(*client_, OnServiceInitialized(true, _)).Times(0); + EXPECT_EQ(controller_->GetState(), Controller::State::CREATED); + + InitializeController(); + driver_->MakeReady(); + store_->TriggerInit(false, base::MakeUnique<std::vector<Entry>>()); + file_monitor_->TriggerInit(true); + + EXPECT_EQ(controller_->GetState(), Controller::State::RECOVERING); + driver_->TriggerHardRecoverComplete(true); + store_->TriggerHardRecover(true); + file_monitor_->TriggerHardRecover(true); + + EXPECT_CALL(*client_, OnServiceInitialized(true, _)).Times(1); + task_runner_->RunUntilIdle(); + histogram_tester.ExpectBucketCount( + "Download.Service.StartUpStatus.Initialization", + static_cast<base::HistogramBase::Sample>(stats::StartUpResult::FAILURE), + 1); + histogram_tester.ExpectBucketCount( + "Download.Service.StartUpStatus.Initialization", + static_cast<base::HistogramBase::Sample>( + stats::StartUpResult::FAILURE_REASON_MODEL), + 1); + histogram_tester.ExpectBucketCount( + "Download.Service.StartUpStatus.Recovery", + static_cast<base::HistogramBase::Sample>(stats::StartUpResult::SUCCESS), + 1); +} + +TEST_F(DownloadServiceControllerImplTest, HardRecoveryAfterFailedFileMonitor) { + base::HistogramTester histogram_tester; + EXPECT_CALL(*client_, OnServiceInitialized(true, _)).Times(0); + EXPECT_EQ(controller_->GetState(), Controller::State::CREATED); + + InitializeController(); + driver_->MakeReady(); + store_->TriggerInit(true, base::MakeUnique<std::vector<Entry>>()); + file_monitor_->TriggerInit(false); + + EXPECT_EQ(controller_->GetState(), Controller::State::RECOVERING); + driver_->TriggerHardRecoverComplete(true); + store_->TriggerHardRecover(true); + file_monitor_->TriggerHardRecover(true); + + EXPECT_CALL(*client_, OnServiceInitialized(true, _)).Times(1); + task_runner_->RunUntilIdle(); + histogram_tester.ExpectBucketCount( + "Download.Service.StartUpStatus.Initialization", + static_cast<base::HistogramBase::Sample>(stats::StartUpResult::FAILURE), + 1); + histogram_tester.ExpectBucketCount( + "Download.Service.StartUpStatus.Initialization", + static_cast<base::HistogramBase::Sample>( + stats::StartUpResult::FAILURE_REASON_FILE_MONITOR), + 1); + histogram_tester.ExpectBucketCount( + "Download.Service.StartUpStatus.Recovery", + static_cast<base::HistogramBase::Sample>(stats::StartUpResult::SUCCESS), + 1); +} + +TEST_F(DownloadServiceControllerImplTest, HardRecoveryFails) { + base::HistogramTester histogram_tester; + EXPECT_CALL(*client_, OnServiceInitialized(true, _)).Times(0); + EXPECT_EQ(controller_->GetState(), Controller::State::CREATED); + + InitializeController(); + driver_->MakeReady(); + store_->TriggerInit(false, base::MakeUnique<std::vector<Entry>>()); + file_monitor_->TriggerInit(true); + + EXPECT_EQ(controller_->GetState(), Controller::State::RECOVERING); + driver_->TriggerHardRecoverComplete(true); + store_->TriggerHardRecover(true); + file_monitor_->TriggerHardRecover(false); + + EXPECT_CALL(*client_, OnServiceUnavailable()).Times(1); + task_runner_->RunUntilIdle(); + histogram_tester.ExpectBucketCount( + "Download.Service.StartUpStatus.Initialization", + static_cast<base::HistogramBase::Sample>(stats::StartUpResult::FAILURE), + 1); + histogram_tester.ExpectBucketCount( + "Download.Service.StartUpStatus.Initialization", + static_cast<base::HistogramBase::Sample>( + stats::StartUpResult::FAILURE_REASON_MODEL), + 1); + histogram_tester.ExpectBucketCount( + "Download.Service.StartUpStatus.Recovery", + static_cast<base::HistogramBase::Sample>(stats::StartUpResult::FAILURE), + 1); + histogram_tester.ExpectBucketCount( + "Download.Service.StartUpStatus.Recovery", + static_cast<base::HistogramBase::Sample>( + stats::StartUpResult::FAILURE_REASON_FILE_MONITOR), + 1); } TEST_F(DownloadServiceControllerImplTest, SuccessfulInitWithExistingDownload) { @@ -266,15 +393,16 @@ std::vector<Entry> entries = {entry1, entry2, entry3}; std::vector<std::string> expected_guids = {entry1.guid, entry2.guid}; - EXPECT_CALL( - *client_, - OnServiceInitialized(testing::UnorderedElementsAreArray(expected_guids))); + EXPECT_CALL(*client_, + OnServiceInitialized( + false, testing::UnorderedElementsAreArray(expected_guids))); EXPECT_CALL(*scheduler_, Next(_, _)).Times(1); EXPECT_CALL(*scheduler_, Reschedule(_)).Times(1); InitializeController(); driver_->MakeReady(); store_->TriggerInit(true, base::MakeUnique<std::vector<Entry>>(entries)); + file_monitor_->TriggerInit(true); task_runner_->RunUntilIdle(); EXPECT_TRUE(init_callback_called_); @@ -301,6 +429,7 @@ InitializeController(); driver_->MakeReady(); store_->TriggerInit(true, base::MakeUnique<std::vector<Entry>>(entries)); + file_monitor_->TriggerInit(true); task_runner_->RunUntilIdle(); } @@ -318,46 +447,25 @@ InitializeController(); driver_->MakeReady(); store_->TriggerInit(true, base::MakeUnique<std::vector<Entry>>(entries)); + file_monitor_->TriggerInit(true); controller_->OnStartScheduledTask(DownloadTaskType::CLEANUP_TASK, base::Bind(&NotifyTaskFinished)); task_runner_->RunUntilIdle(); } -TEST_F(DownloadServiceControllerImplTest, FailedInitWithBadModel) { - base::HistogramTester histogram_tester; - EXPECT_CALL(*client_, OnServiceInitialized(_)).Times(0); - EXPECT_CALL(*client_, OnServiceUnavailable()).Times(1); - - InitializeController(); - store_->TriggerInit(false, base::MakeUnique<std::vector<Entry>>()); - driver_->MakeReady(); - - task_runner_->RunUntilIdle(); - histogram_tester.ExpectBucketCount( - "Download.Service.StartUpStatus", - static_cast<base::HistogramBase::Sample>(stats::StartUpResult::FAILURE), - 1); - histogram_tester.ExpectBucketCount( - "Download.Service.StartUpStatus", - static_cast<base::HistogramBase::Sample>( - stats::StartUpResult::FAILURE_REASON_MODEL), - 1); - - histogram_tester.ExpectTotalCount("Download.Service.StartUpStatus", 2); -} - TEST_F(DownloadServiceControllerImplTest, GetOwnerOfDownload) { Entry entry = test::BuildBasicEntry(); std::vector<Entry> entries = {entry}; - EXPECT_CALL(*client_, OnServiceInitialized(_)).Times(1); + EXPECT_CALL(*client_, OnServiceInitialized(false, _)).Times(1); EXPECT_CALL(*scheduler_, Next(_, _)).Times(1); EXPECT_CALL(*scheduler_, Reschedule(_)).Times(1); InitializeController(); driver_->MakeReady(); store_->TriggerInit(true, base::MakeUnique<std::vector<Entry>>(entries)); + file_monitor_->TriggerInit(true); task_runner_->RunUntilIdle(); @@ -367,13 +475,14 @@ } TEST_F(DownloadServiceControllerImplTest, AddDownloadAccepted) { - EXPECT_CALL(*client_, OnServiceInitialized(_)).Times(1); + EXPECT_CALL(*client_, OnServiceInitialized(false, _)).Times(1); EXPECT_CALL(*scheduler_, Next(_, _)).Times(1); EXPECT_CALL(*scheduler_, Reschedule(_)).Times(1); // Set up the Controller. InitializeController(); store_->TriggerInit(true, base::MakeUnique<std::vector<Entry>>()); + file_monitor_->TriggerInit(true); driver_->MakeReady(); task_runner_->RunUntilIdle(); @@ -399,7 +508,7 @@ } TEST_F(DownloadServiceControllerImplTest, AddDownloadFailsWithBackoff) { - EXPECT_CALL(*client_, OnServiceInitialized(_)).Times(1); + EXPECT_CALL(*client_, OnServiceInitialized(false, _)).Times(1); EXPECT_CALL(*scheduler_, Next(_, _)).Times(1); EXPECT_CALL(*scheduler_, Reschedule(_)).Times(1); @@ -409,6 +518,7 @@ // Set up the Controller. InitializeController(); store_->TriggerInit(true, base::MakeUnique<std::vector<Entry>>(entries)); + file_monitor_->TriggerInit(true); driver_->MakeReady(); task_runner_->RunUntilIdle(); @@ -429,7 +539,7 @@ TEST_F(DownloadServiceControllerImplTest, AddDownloadFailsWithDuplicateGuidInModel) { - EXPECT_CALL(*client_, OnServiceInitialized(_)).Times(1); + EXPECT_CALL(*client_, OnServiceInitialized(false, _)).Times(1); EXPECT_CALL(*scheduler_, Next(_, _)).Times(1); EXPECT_CALL(*scheduler_, Reschedule(_)).Times(1); @@ -439,6 +549,7 @@ // Set up the Controller. InitializeController(); store_->TriggerInit(true, base::MakeUnique<std::vector<Entry>>(entries)); + file_monitor_->TriggerInit(true); driver_->MakeReady(); task_runner_->RunUntilIdle(); @@ -459,11 +570,12 @@ EXPECT_CALL(*scheduler_, Next(_, _)).Times(1); EXPECT_CALL(*scheduler_, Reschedule(_)).Times(1); - EXPECT_CALL(*client_, OnServiceInitialized(_)).Times(1); + EXPECT_CALL(*client_, OnServiceInitialized(false, _)).Times(1); // Set up the Controller. InitializeController(); store_->TriggerInit(true, base::MakeUnique<std::vector<Entry>>()); + file_monitor_->TriggerInit(true); driver_->MakeReady(); task_runner_->RunUntilIdle(); @@ -489,13 +601,14 @@ } TEST_F(DownloadServiceControllerImplTest, AddDownloadFailsWithBadClient) { - EXPECT_CALL(*client_, OnServiceInitialized(_)).Times(1); + EXPECT_CALL(*client_, OnServiceInitialized(false, _)).Times(1); EXPECT_CALL(*scheduler_, Next(_, _)).Times(1); EXPECT_CALL(*scheduler_, Reschedule(_)).Times(1); // Set up the Controller. InitializeController(); store_->TriggerInit(true, base::MakeUnique<std::vector<Entry>>()); + file_monitor_->TriggerInit(true); driver_->MakeReady(); task_runner_->RunUntilIdle(); @@ -512,13 +625,14 @@ } TEST_F(DownloadServiceControllerImplTest, AddDownloadFailsWithClientCancel) { - EXPECT_CALL(*client_, OnServiceInitialized(_)).Times(1); + EXPECT_CALL(*client_, OnServiceInitialized(false, _)).Times(1); EXPECT_CALL(*scheduler_, Next(_, _)).Times(1); EXPECT_CALL(*scheduler_, Reschedule(_)).Times(1); // Set up the Controller. InitializeController(); store_->TriggerInit(true, base::MakeUnique<std::vector<Entry>>()); + file_monitor_->TriggerInit(true); driver_->MakeReady(); task_runner_->RunUntilIdle(); @@ -537,13 +651,14 @@ } TEST_F(DownloadServiceControllerImplTest, AddDownloadFailsWithInternalError) { - EXPECT_CALL(*client_, OnServiceInitialized(_)).Times(1); + EXPECT_CALL(*client_, OnServiceInitialized(false, _)).Times(1); EXPECT_CALL(*scheduler_, Next(_, _)).Times(1); EXPECT_CALL(*scheduler_, Reschedule(_)).Times(1); // Set up the Controller. InitializeController(); store_->TriggerInit(true, base::MakeUnique<std::vector<Entry>>()); + file_monitor_->TriggerInit(true); driver_->MakeReady(); task_runner_->RunUntilIdle(); @@ -569,7 +684,7 @@ entry3.state = Entry::State::COMPLETE; std::vector<Entry> entries = {entry1, entry2, entry3}; - EXPECT_CALL(*client_, OnServiceInitialized(_)).Times(1); + EXPECT_CALL(*client_, OnServiceInitialized(false, _)).Times(1); EXPECT_CALL(*scheduler_, Next(_, _)).Times(3); EXPECT_CALL(*scheduler_, Reschedule(_)).Times(3); @@ -577,6 +692,7 @@ // the scheduler. InitializeController(); store_->TriggerInit(true, base::MakeUnique<std::vector<Entry>>(entries)); + file_monitor_->TriggerInit(true); driver_->MakeReady(); // Setup download driver test data. @@ -616,12 +732,13 @@ entry2.state = Entry::State::ACTIVE; std::vector<Entry> entries = {entry1, entry2}; - EXPECT_CALL(*client_, OnServiceInitialized(_)).Times(1); + EXPECT_CALL(*client_, OnServiceInitialized(false, _)).Times(1); EXPECT_CALL(*scheduler_, Next(_, _)).Times(2); EXPECT_CALL(*scheduler_, Reschedule(_)).Times(2); InitializeController(); store_->TriggerInit(true, base::MakeUnique<std::vector<Entry>>(entries)); + file_monitor_->TriggerInit(true); driver_->MakeReady(); // Setup download driver test data. @@ -653,7 +770,7 @@ entry.state = Entry::State::ACTIVE; std::vector<Entry> entries = {entry}; - EXPECT_CALL(*client_, OnServiceInitialized(_)).Times(1); + EXPECT_CALL(*client_, OnServiceInitialized(false, _)).Times(1); EXPECT_CALL(*client_, OnDownloadFailed(entry.guid, Client::FailureReason::CANCELLED)) .Times(1); @@ -662,6 +779,7 @@ InitializeController(); store_->TriggerInit(true, base::MakeUnique<std::vector<Entry>>(entries)); + file_monitor_->TriggerInit(true); driver_->MakeReady(); DriverEntry driver_entry; @@ -679,7 +797,7 @@ entry.state = Entry::State::ACTIVE; std::vector<Entry> entries = {entry}; - EXPECT_CALL(*client_, OnServiceInitialized(_)).Times(1); + EXPECT_CALL(*client_, OnServiceInitialized(false, _)).Times(1); EXPECT_CALL(*client_, OnDownloadFailed(entry.guid, Client::FailureReason::NETWORK)) .Times(1); @@ -688,6 +806,7 @@ InitializeController(); store_->TriggerInit(true, base::MakeUnique<std::vector<Entry>>(entries)); + file_monitor_->TriggerInit(true); driver_->MakeReady(); DriverEntry driver_entry; @@ -710,7 +829,7 @@ BuildDriverEntry(entry2, DriverEntry::State::INTERRUPTED); std::vector<DriverEntry> dentries = {dentry1, dentry2}; - EXPECT_CALL(*client_, OnServiceInitialized(_)).Times(1); + EXPECT_CALL(*client_, OnServiceInitialized(false, _)).Times(1); // Set up the Controller. device_status_listener_->SetDeviceStatus( @@ -719,6 +838,7 @@ driver_->AddTestData(dentries); InitializeController(); store_->TriggerInit(true, base::MakeUnique<std::vector<Entry>>(entries)); + file_monitor_->TriggerInit(true); driver_->MakeReady(); task_runner_->RunUntilIdle(); @@ -747,13 +867,14 @@ entry.state = Entry::State::ACTIVE; std::vector<Entry> entries = {entry}; - EXPECT_CALL(*client_, OnServiceInitialized(_)).Times(1); + EXPECT_CALL(*client_, OnServiceInitialized(false, _)).Times(1); EXPECT_CALL(*client_, OnDownloadSucceeded(entry.guid, _, _)).Times(1); EXPECT_CALL(*scheduler_, Next(_, _)).Times(2); EXPECT_CALL(*scheduler_, Reschedule(_)).Times(2); InitializeController(); store_->TriggerInit(true, base::MakeUnique<std::vector<Entry>>(entries)); + file_monitor_->TriggerInit(true); driver_->MakeReady(); DriverEntry driver_entry; @@ -782,6 +903,7 @@ InitializeController(); store_->TriggerInit(true, base::MakeUnique<std::vector<Entry>>(entries)); + file_monitor_->TriggerInit(true); driver_->MakeReady(); DriverEntry driver_entry; @@ -804,12 +926,13 @@ entry.state = Entry::State::ACTIVE; std::vector<Entry> entries = {entry}; - EXPECT_CALL(*client_, OnServiceInitialized(_)).Times(1); + EXPECT_CALL(*client_, OnServiceInitialized(false, _)).Times(1); EXPECT_CALL(*scheduler_, Next(_, _)).Times(1); EXPECT_CALL(*scheduler_, Reschedule(_)).Times(1); InitializeController(); store_->TriggerInit(true, base::MakeUnique<std::vector<Entry>>(entries)); + file_monitor_->TriggerInit(true); driver_->MakeReady(); DriverEntry driver_entry; @@ -844,7 +967,7 @@ std::vector<Entry> entries = {entry1, entry2, entry3, entry4}; std::vector<DriverEntry> dentries = {dentry1, dentry3}; - EXPECT_CALL(*client_, OnServiceInitialized(_)).Times(1); + EXPECT_CALL(*client_, OnServiceInitialized(false, _)).Times(1); // Test FailureReason::TIMEDOUT. EXPECT_CALL(*client_, @@ -855,6 +978,7 @@ driver_->AddTestData(dentries); InitializeController(); store_->TriggerInit(true, base::MakeUnique<std::vector<Entry>>(entries)); + file_monitor_->TriggerInit(true); driver_->MakeReady(); task_runner_->RunUntilIdle(); @@ -884,7 +1008,7 @@ } TEST_F(DownloadServiceControllerImplTest, StartupRecovery) { - EXPECT_CALL(*client_, OnServiceInitialized(_)).Times(1); + EXPECT_CALL(*client_, OnServiceInitialized(false, _)).Times(1); std::vector<Entry> entries; std::vector<DriverEntry> driver_entries; @@ -967,6 +1091,7 @@ driver_->MakeReady(); store_->AutomaticallyTriggerAllFutureCallbacks(true); store_->TriggerInit(true, base::MakeUnique<std::vector<Entry>>(entries)); + file_monitor_->TriggerInit(true); // Allow the initialization routines and persistent layers to do their thing. task_runner_->RunUntilIdle(); @@ -1055,7 +1180,7 @@ std::vector<Entry> entries = {entry1, entry2, entry3}; std::vector<DriverEntry> dentries = {dentry1, dentry2}; - EXPECT_CALL(*client_, OnServiceInitialized(_)).Times(1); + EXPECT_CALL(*client_, OnServiceInitialized(false, _)).Times(1); // Set up the Controller. device_status_listener_->SetDeviceStatus( @@ -1064,6 +1189,7 @@ driver_->AddTestData(dentries); InitializeController(); store_->TriggerInit(true, base::MakeUnique<std::vector<Entry>>(entries)); + file_monitor_->TriggerInit(true); driver_->MakeReady(); task_runner_->RunUntilIdle(); @@ -1099,7 +1225,7 @@ std::vector<Entry> entries = {entry1, entry2}; std::vector<DriverEntry> dentries = {dentry1}; - EXPECT_CALL(*client_, OnServiceInitialized(_)).Times(1); + EXPECT_CALL(*client_, OnServiceInitialized(false, _)).Times(1); // Set up the Controller. device_status_listener_->SetDeviceStatus( @@ -1108,6 +1234,7 @@ driver_->AddTestData(dentries); InitializeController(); store_->TriggerInit(true, base::MakeUnique<std::vector<Entry>>(entries)); + file_monitor_->TriggerInit(true); driver_->MakeReady(); task_runner_->RunUntilIdle(); @@ -1181,10 +1308,11 @@ base::Time::Now() - base::TimeDelta::FromSeconds(2); std::vector<Entry> entries = {entry1, entry2}; - EXPECT_CALL(*client_, OnServiceInitialized(_)).Times(1); + EXPECT_CALL(*client_, OnServiceInitialized(false, _)).Times(1); InitializeController(); store_->TriggerInit(true, base::MakeUnique<std::vector<Entry>>(entries)); + file_monitor_->TriggerInit(true); driver_->MakeReady(); task_runner_->RunUntilIdle(); @@ -1200,7 +1328,7 @@ Entry entry2 = test::BuildBasicEntry(Entry::State::ACTIVE); std::vector<Entry> entries = {entry1, entry2}; - EXPECT_CALL(*client_, OnServiceInitialized(_)).Times(1); + EXPECT_CALL(*client_, OnServiceInitialized(false, _)).Times(1); // Setup the Configuration. config_->max_concurrent_downloads = 1u; @@ -1209,6 +1337,7 @@ // Setup the controller. InitializeController(); store_->TriggerInit(true, base::MakeUnique<std::vector<Entry>>(entries)); + file_monitor_->TriggerInit(true); store_->AutomaticallyTriggerAllFutureCallbacks(true); // Hit the max running configuration threshold, nothing should be called. @@ -1228,7 +1357,7 @@ Entry entry3 = test::BuildBasicEntry(Entry::State::PAUSED); std::vector<Entry> entries = {entry1, entry2, entry3}; - EXPECT_CALL(*client_, OnServiceInitialized(_)).Times(1); + EXPECT_CALL(*client_, OnServiceInitialized(false, _)).Times(1); // Setup the Configuration. config_->max_concurrent_downloads = 2u; @@ -1237,6 +1366,7 @@ // Setup the controller. InitializeController(); store_->TriggerInit(true, base::MakeUnique<std::vector<Entry>>(entries)); + file_monitor_->TriggerInit(true); store_->AutomaticallyTriggerAllFutureCallbacks(true); // Can have one more download due to max concurrent configuration.
diff --git a/components/download/internal/model.h b/components/download/internal/model.h index 8b5998b..c1b2ed0 100644 --- a/components/download/internal/model.h +++ b/components/download/internal/model.h
@@ -35,7 +35,7 @@ // Called asynchronously in response to a Model::HardRecover call. If // |success| is |false|, recovery of the Model and/or the underlying Store // failed. After this call there should be no entries stored in this Model. - virtual void OnHardRecoverComplete(bool success) = 0; + virtual void OnModelHardRecoverComplete(bool success) = 0; // Called when an Entry addition is complete. |success| determines whether // or not the entry has been successfully persisted to the Store.
diff --git a/components/download/internal/model_impl.cc b/components/download/internal/model_impl.cc index 1eee678..9a62961 100644 --- a/components/download/internal/model_impl.cc +++ b/components/download/internal/model_impl.cc
@@ -109,7 +109,7 @@ } void ModelImpl::OnHardRecoverFinished(bool success) { - client_->OnHardRecoverComplete(success); + client_->OnModelHardRecoverComplete(success); } void ModelImpl::OnAddFinished(DownloadClient client,
diff --git a/components/download/internal/model_impl_unittest.cc b/components/download/internal/model_impl_unittest.cc index 2367ac27..624715f7 100644 --- a/components/download/internal/model_impl_unittest.cc +++ b/components/download/internal/model_impl_unittest.cc
@@ -108,7 +108,7 @@ EXPECT_TRUE(store_->init_called()); store_->TriggerInit(true, base::MakeUnique<std::vector<Entry>>(entries)); - EXPECT_CALL(client_, OnHardRecoverComplete(true)); + EXPECT_CALL(client_, OnModelHardRecoverComplete(true)); model_->HardRecover(); store_->TriggerHardRecover(true); @@ -122,7 +122,7 @@ EXPECT_TRUE(store_->init_called()); store_->TriggerInit(false, base::MakeUnique<std::vector<Entry>>()); - EXPECT_CALL(client_, OnHardRecoverComplete(true)); + EXPECT_CALL(client_, OnModelHardRecoverComplete(true)); model_->HardRecover(); store_->TriggerHardRecover(true); @@ -140,7 +140,7 @@ EXPECT_TRUE(store_->init_called()); store_->TriggerInit(true, base::MakeUnique<std::vector<Entry>>(entries)); - EXPECT_CALL(client_, OnHardRecoverComplete(false)); + EXPECT_CALL(client_, OnModelHardRecoverComplete(false)); model_->HardRecover(); store_->TriggerHardRecover(false); @@ -154,7 +154,7 @@ EXPECT_TRUE(store_->init_called()); store_->TriggerInit(false, base::MakeUnique<std::vector<Entry>>()); - EXPECT_CALL(client_, OnHardRecoverComplete(false)); + EXPECT_CALL(client_, OnModelHardRecoverComplete(false)); model_->HardRecover(); store_->TriggerHardRecover(false);
diff --git a/components/download/internal/startup_status.cc b/components/download/internal/startup_status.cc index 6560cec..393e289 100644 --- a/components/download/internal/startup_status.cc +++ b/components/download/internal/startup_status.cc
@@ -9,6 +9,12 @@ StartupStatus::StartupStatus() = default; StartupStatus::~StartupStatus() = default; +void StartupStatus::Reset() { + driver_ok.reset(); + model_ok.reset(); + file_monitor_ok.reset(); +} + bool StartupStatus::Complete() const { return driver_ok.has_value() && model_ok.has_value() && file_monitor_ok.has_value();
diff --git a/components/download/internal/startup_status.h b/components/download/internal/startup_status.h index c9c0676..754428a 100644 --- a/components/download/internal/startup_status.h +++ b/components/download/internal/startup_status.h
@@ -20,6 +20,9 @@ base::Optional<bool> model_ok; base::Optional<bool> file_monitor_ok; + // Resets all startup state tracking. + void Reset(); + // Whether or not all components have finished initialization. Note that this // does not mean that all components were initialized successfully. bool Complete() const;
diff --git a/components/download/internal/stats.cc b/components/download/internal/stats.cc index 96d33b5..5b4f12c5 100644 --- a/components/download/internal/stats.cc +++ b/components/download/internal/stats.cc
@@ -117,9 +117,15 @@ } // Helper method to log StartUpResult. -void LogStartUpResult(StartUpResult result) { - base::UmaHistogramEnumeration("Download.Service.StartUpStatus", result, - StartUpResult::COUNT); +void LogStartUpResult(bool in_recovery, StartUpResult result) { + if (in_recovery) { + base::UmaHistogramEnumeration("Download.Service.StartUpStatus.Recovery", + result, StartUpResult::COUNT); + } else { + base::UmaHistogramEnumeration( + "Download.Service.StartUpStatus.Initialization", result, + StartUpResult::COUNT); + } } // Helper method to log the number of entries under a particular state. @@ -131,20 +137,20 @@ } // namespace -void LogControllerStartupStatus(const StartupStatus& status) { +void LogControllerStartupStatus(bool in_recovery, const StartupStatus& status) { DCHECK(status.Complete()); // Total counts for general success/failure rate. - LogStartUpResult(status.Ok() ? StartUpResult::SUCCESS - : StartUpResult::FAILURE); + LogStartUpResult(in_recovery, status.Ok() ? StartUpResult::SUCCESS + : StartUpResult::FAILURE); // Failure reasons. if (!status.driver_ok.value()) - LogStartUpResult(StartUpResult::FAILURE_REASON_DRIVER); + LogStartUpResult(in_recovery, StartUpResult::FAILURE_REASON_DRIVER); if (!status.model_ok.value()) - LogStartUpResult(StartUpResult::FAILURE_REASON_MODEL); + LogStartUpResult(in_recovery, StartUpResult::FAILURE_REASON_MODEL); if (!status.file_monitor_ok.value()) - LogStartUpResult(StartUpResult::FAILURE_REASON_FILE_MONITOR); + LogStartUpResult(in_recovery, StartUpResult::FAILURE_REASON_FILE_MONITOR); } void LogServiceApiAction(DownloadClient client, ServiceApiAction action) {
diff --git a/components/download/internal/stats.h b/components/download/internal/stats.h index c3bad31e..e100d35 100644 --- a/components/download/internal/stats.h +++ b/components/download/internal/stats.h
@@ -125,7 +125,7 @@ // Logs the results of starting up the Controller. Will log each failure reason // if |status| contains more than one initialization failure. -void LogControllerStartupStatus(const StartupStatus& status); +void LogControllerStartupStatus(bool in_recovery, const StartupStatus& status); // Logs an action taken on the service API. void LogServiceApiAction(DownloadClient client, ServiceApiAction action);
diff --git a/components/download/internal/test/empty_client.cc b/components/download/internal/test/empty_client.cc index 7f14c37..e27aeb0 100644 --- a/components/download/internal/test/empty_client.cc +++ b/components/download/internal/test/empty_client.cc
@@ -8,6 +8,7 @@ namespace test { void EmptyClient::OnServiceInitialized( + bool state_lost, const std::vector<std::string>& outstanding_download_guids) {} void EmptyClient::OnServiceUnavailable() {}
diff --git a/components/download/internal/test/empty_client.h b/components/download/internal/test/empty_client.h index 64fcc6a..d21eaa19 100644 --- a/components/download/internal/test/empty_client.h +++ b/components/download/internal/test/empty_client.h
@@ -18,6 +18,7 @@ // Client implementation. void OnServiceInitialized( + bool state_lost, const std::vector<std::string>& outstanding_download_guids) override; void OnServiceUnavailable() override; ShouldDownload OnDownloadStarted(
diff --git a/components/download/internal/test/mock_client.h b/components/download/internal/test/mock_client.h index ae8f21c..214fc91 100644 --- a/components/download/internal/test/mock_client.h +++ b/components/download/internal/test/mock_client.h
@@ -18,7 +18,8 @@ ~MockClient() override; // Client implementation. - MOCK_METHOD1(OnServiceInitialized, void(const std::vector<std::string>&)); + MOCK_METHOD2(OnServiceInitialized, + void(bool, const std::vector<std::string>&)); MOCK_METHOD0(OnServiceUnavailable, void()); MOCK_METHOD3( OnDownloadStarted,
diff --git a/components/download/internal/test/mock_model_client.h b/components/download/internal/test/mock_model_client.h index c8a33e99..c6cf3cf 100644 --- a/components/download/internal/test/mock_model_client.h +++ b/components/download/internal/test/mock_model_client.h
@@ -20,7 +20,7 @@ // Model::Client implementation. MOCK_METHOD1(OnModelReady, void(bool)); - MOCK_METHOD1(OnHardRecoverComplete, void(bool)); + MOCK_METHOD1(OnModelHardRecoverComplete, void(bool)); MOCK_METHOD3(OnItemAdded, void(bool, DownloadClient, const std::string&)); MOCK_METHOD3(OnItemUpdated, void(bool, DownloadClient, const std::string&)); MOCK_METHOD3(OnItemRemoved, void(bool, DownloadClient, const std::string&));
diff --git a/components/download/public/client.h b/components/download/public/client.h index 46d5cfb..518d7e22 100644 --- a/components/download/public/client.h +++ b/components/download/public/client.h
@@ -61,8 +61,12 @@ // Called when the DownloadService is initialized and ready to be interacted // with. |outstanding_download_guids| is a list of all downloads the - // DownloadService is aware of that are associated with this Client. + // DownloadService is aware of that are associated with this Client. If + // |state_lost| is |true|, the service ran into an error initializing and had + // to destroy all internal persisted state. At this point any saved files + // might not be available and any previously scheduled downloads are gone. virtual void OnServiceInitialized( + bool state_lost, const std::vector<std::string>& outstanding_download_guids) = 0; // Called when the DownloadService fails to initialize and should not be used.
diff --git a/components/proximity_auth/proximity_auth_pref_manager.cc b/components/proximity_auth/proximity_auth_pref_manager.cc index 39101cb..12a1c629 100644 --- a/components/proximity_auth/proximity_auth_pref_manager.cc +++ b/components/proximity_auth/proximity_auth_pref_manager.cc
@@ -31,6 +31,15 @@ registry->RegisterIntegerPref( prefs::kEasyUnlockProximityThreshold, 1, user_prefs::PrefRegistrySyncable::SYNCABLE_PREF); + + // TODO(tengs): For existing EasyUnlock users, we want to maintain their + // current behaviour and keep login enabled. However, for new users, we will + // disable login when setting up EasyUnlock. + // After a sufficient number of releases, we should make the default value + // false. + registry->RegisterBooleanPref( + prefs::kProximityAuthIsChromeOSLoginEnabled, true, + user_prefs::PrefRegistrySyncable::SYNCABLE_PREF); } bool ProximityAuthPrefManager::HasDeviceWithAddress( @@ -136,4 +145,13 @@ return static_cast<ProximityThreshold>(pref_value); } +void ProximityAuthPrefManager::SetIsChromeOSLoginEnabled(bool is_enabled) { + return pref_service_->SetBoolean(prefs::kProximityAuthIsChromeOSLoginEnabled, + is_enabled); +} + +bool ProximityAuthPrefManager::IsChromeOSLoginEnabled() { + return pref_service_->GetBoolean(prefs::kProximityAuthIsChromeOSLoginEnabled); +} + } // namespace proximity_auth
diff --git a/components/proximity_auth/proximity_auth_pref_manager.h b/components/proximity_auth/proximity_auth_pref_manager.h index 9d945e1..9418ad52 100644 --- a/components/proximity_auth/proximity_auth_pref_manager.h +++ b/components/proximity_auth/proximity_auth_pref_manager.h
@@ -84,6 +84,11 @@ virtual void SetProximityThreshold(ProximityThreshold value); virtual ProximityThreshold GetProximityThreshold() const; + // Setting and getter for whether EasyUnlock is enabled for ChromeOS login (in + // addition to screen lock). + virtual void SetIsChromeOSLoginEnabled(bool is_enabled); + virtual bool IsChromeOSLoginEnabled(); + private: const base::DictionaryValue* GetRemoteBleDevices() const;
diff --git a/components/proximity_auth/proximity_auth_pref_manager_unittest.cc b/components/proximity_auth/proximity_auth_pref_manager_unittest.cc index 78856f7..1c250775 100644 --- a/components/proximity_auth/proximity_auth_pref_manager_unittest.cc +++ b/components/proximity_auth/proximity_auth_pref_manager_unittest.cc
@@ -184,4 +184,15 @@ EXPECT_EQ(kProximityThreshold2, pref_manager.GetProximityThreshold()); } +TEST_F(ProximityAuthProximityAuthPrefManagerTest, IsChromeOSLoginEnabled) { + ProximityAuthPrefManager pref_manager(&pref_service_); + EXPECT_TRUE(pref_manager.IsChromeOSLoginEnabled()); + + pref_manager.SetIsChromeOSLoginEnabled(false); + EXPECT_FALSE(pref_manager.IsChromeOSLoginEnabled()); + + pref_manager.SetIsChromeOSLoginEnabled(true); + EXPECT_TRUE(pref_manager.IsChromeOSLoginEnabled()); +} + } // namespace proximity_auth
diff --git a/components/proximity_auth/proximity_auth_pref_names.cc b/components/proximity_auth/proximity_auth_pref_names.cc index 9ef2349..31abf9b0 100644 --- a/components/proximity_auth/proximity_auth_pref_names.cc +++ b/components/proximity_auth/proximity_auth_pref_names.cc
@@ -20,5 +20,10 @@ // Unlock. const char kEasyUnlockProximityThreshold[] = "easy_unlock.proximity_threshold"; +// Whether or not EasyUnlock is enabled on the ChromeOS login screen (in +// addition to the lock screen). +extern const char kProximityAuthIsChromeOSLoginEnabled[] = + "proximity_auth.is_chromeos_login_enabled"; + } // namespace prefs } // namespace proximity_auth
diff --git a/components/proximity_auth/proximity_auth_pref_names.h b/components/proximity_auth/proximity_auth_pref_names.h index de4198e..abc7a57 100644 --- a/components/proximity_auth/proximity_auth_pref_names.h +++ b/components/proximity_auth/proximity_auth_pref_names.h
@@ -11,6 +11,7 @@ extern const char kEasyUnlockProximityThreshold[]; extern const char kProximityAuthLastPasswordEntryTimestampMs[]; extern const char kProximityAuthRemoteBleDevices[]; +extern const char kProximityAuthIsChromeOSLoginEnabled[]; } // namespace prefs } // proximity_auth
diff --git a/extensions/shell/BUILD.gn b/extensions/shell/BUILD.gn index 46749156..38d9f84d 100644 --- a/extensions/shell/BUILD.gn +++ b/extensions/shell/BUILD.gn
@@ -188,6 +188,13 @@ ] } + if (is_desktop_linux) { + sources += [ + "browser/api/file_system/shell_file_system_delegate.cc", + "browser/api/file_system/shell_file_system_delegate.h", + ] + } + if (is_chromeos) { sources += [ "browser/api/vpn_provider/vpn_service_factory.cc",
diff --git a/extensions/shell/browser/api/file_system/shell_file_system_delegate.cc b/extensions/shell/browser/api/file_system/shell_file_system_delegate.cc new file mode 100644 index 0000000..58035a35 --- /dev/null +++ b/extensions/shell/browser/api/file_system/shell_file_system_delegate.cc
@@ -0,0 +1,63 @@ +// 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 "extensions/shell/browser/api/file_system/shell_file_system_delegate.h" + +#include "apps/saved_files_service.h" +#include "base/callback.h" +#include "base/files/file_path.h" +#include "base/logging.h" +#include "extensions/browser/api/file_system/saved_files_service_interface.h" + +namespace extensions { + +ShellFileSystemDelegate::ShellFileSystemDelegate() = default; + +ShellFileSystemDelegate::~ShellFileSystemDelegate() {} + +base::FilePath ShellFileSystemDelegate::GetDefaultDirectory() { + NOTIMPLEMENTED(); + return base::FilePath(); +} + +bool ShellFileSystemDelegate::ShowSelectFileDialog( + scoped_refptr<UIThreadExtensionFunction> extension_function, + ui::SelectFileDialog::Type type, + const base::FilePath& default_path, + const ui::SelectFileDialog::FileTypeInfo* file_types, + FileSystemDelegate::FilesSelectedCallback files_selected_callback, + base::OnceClosure file_selection_canceled_callback) { + NOTIMPLEMENTED(); + + // Run the cancel callback by default. + std::move(file_selection_canceled_callback).Run(); + + // Return true since this isn't a disallowed call, just not implemented. + return true; +} + +void ShellFileSystemDelegate::ConfirmSensitiveDirectoryAccess( + bool has_write_permission, + const base::string16& app_name, + content::WebContents* web_contents, + const base::Closure& on_accept, + const base::Closure& on_cancel) { + NOTIMPLEMENTED(); + + // Run the cancel callback by default. + on_cancel.Run(); +} + +int ShellFileSystemDelegate::GetDescriptionIdForAcceptType( + const std::string& accept_type) { + NOTIMPLEMENTED(); + return 0; +} + +SavedFilesServiceInterface* ShellFileSystemDelegate::GetSavedFilesService( + content::BrowserContext* browser_context) { + return apps::SavedFilesService::Get(browser_context); +} + +} // namespace extensions
diff --git a/extensions/shell/browser/api/file_system/shell_file_system_delegate.h b/extensions/shell/browser/api/file_system/shell_file_system_delegate.h new file mode 100644 index 0000000..df41e02 --- /dev/null +++ b/extensions/shell/browser/api/file_system/shell_file_system_delegate.h
@@ -0,0 +1,41 @@ +// 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 EXTENSIONS_SHELL_BROWSER_API_FILE_SYSTEM_SHELL_FILE_SYSTEM_DELEGATE_H_ +#define EXTENSIONS_SHELL_BROWSER_API_FILE_SYSTEM_SHELL_FILE_SYSTEM_DELEGATE_H_ + +#include "extensions/browser/api/file_system/file_system_delegate.h" + +namespace extensions { + +class ShellFileSystemDelegate : public FileSystemDelegate { + public: + ShellFileSystemDelegate(); + ~ShellFileSystemDelegate() override; + + // FileSystemDelegate: + base::FilePath GetDefaultDirectory() override; + bool ShowSelectFileDialog( + scoped_refptr<UIThreadExtensionFunction> extension_function, + ui::SelectFileDialog::Type type, + const base::FilePath& default_path, + const ui::SelectFileDialog::FileTypeInfo* file_types, + FileSystemDelegate::FilesSelectedCallback files_selected_callback, + base::OnceClosure file_selection_canceled_callback) override; + void ConfirmSensitiveDirectoryAccess(bool has_write_permission, + const base::string16& app_name, + content::WebContents* web_contents, + const base::Closure& on_accept, + const base::Closure& on_cancel) override; + int GetDescriptionIdForAcceptType(const std::string& accept_type) override; + SavedFilesServiceInterface* GetSavedFilesService( + content::BrowserContext* browser_context) override; + + private: + DISALLOW_COPY_AND_ASSIGN(ShellFileSystemDelegate); +}; + +} // namespace extensions + +#endif // EXTENSIONS_SHELL_BROWSER_API_FILE_SYSTEM_SHELL_FILE_SYSTEM_DELEGATE_H_
diff --git a/extensions/shell/browser/shell_extensions_api_client.cc b/extensions/shell/browser/shell_extensions_api_client.cc index e290498a..9dfcca2 100644 --- a/extensions/shell/browser/shell_extensions_api_client.cc +++ b/extensions/shell/browser/shell_extensions_api_client.cc
@@ -10,10 +10,15 @@ #include "extensions/shell/browser/shell_extension_web_contents_observer.h" #include "extensions/shell/browser/shell_virtual_keyboard_delegate.h" +#if defined(OS_LINUX) && !defined(OS_CHROMEOS) +#include "extensions/shell/browser/api/file_system/shell_file_system_delegate.h" +#endif + namespace extensions { -ShellExtensionsAPIClient::ShellExtensionsAPIClient() { -} +ShellExtensionsAPIClient::ShellExtensionsAPIClient() {} + +ShellExtensionsAPIClient::~ShellExtensionsAPIClient() {} void ShellExtensionsAPIClient::AttachWebContentsHelpers( content::WebContents* web_contents) const { @@ -30,4 +35,12 @@ return base::MakeUnique<ShellVirtualKeyboardDelegate>(); } +#if defined(OS_LINUX) && !defined(OS_CHROMEOS) +FileSystemDelegate* ShellExtensionsAPIClient::GetFileSystemDelegate() { + if (!file_system_delegate_) + file_system_delegate_ = base::MakeUnique<ShellFileSystemDelegate>(); + return file_system_delegate_.get(); +} +#endif + } // namespace extensions
diff --git a/extensions/shell/browser/shell_extensions_api_client.h b/extensions/shell/browser/shell_extensions_api_client.h index d42429a..6a26bea 100644 --- a/extensions/shell/browser/shell_extensions_api_client.h +++ b/extensions/shell/browser/shell_extensions_api_client.h
@@ -9,6 +9,8 @@ #include "extensions/browser/api/extensions_api_client.h" +#include "build/build_config.h" + namespace extensions { class VirtualKeyboardDelegate; @@ -16,6 +18,7 @@ class ShellExtensionsAPIClient : public ExtensionsAPIClient { public: ShellExtensionsAPIClient(); + ~ShellExtensionsAPIClient() override; // ExtensionsAPIClient implementation. void AttachWebContentsHelpers(content::WebContents* web_contents) const @@ -23,6 +26,16 @@ AppViewGuestDelegate* CreateAppViewGuestDelegate() const override; std::unique_ptr<VirtualKeyboardDelegate> CreateVirtualKeyboardDelegate() const override; +#if defined(OS_LINUX) && !defined(OS_CHROMEOS) + FileSystemDelegate* GetFileSystemDelegate() override; +#endif + + private: +#if defined(OS_LINUX) && !defined(OS_CHROMEOS) + std::unique_ptr<FileSystemDelegate> file_system_delegate_; +#endif + + DISALLOW_COPY_AND_ASSIGN(ShellExtensionsAPIClient); }; } // namespace extensions
diff --git a/media/capture/video/chromeos/video_capture_device_factory_chromeos.cc b/media/capture/video/chromeos/video_capture_device_factory_chromeos.cc index 83e4041..2b5460b 100644 --- a/media/capture/video/chromeos/video_capture_device_factory_chromeos.cc +++ b/media/capture/video/chromeos/video_capture_device_factory_chromeos.cc
@@ -14,13 +14,54 @@ VideoCaptureDeviceFactoryChromeOS::VideoCaptureDeviceFactoryChromeOS( scoped_refptr<base::SingleThreadTaskRunner> task_runner_for_screen_observer) : task_runner_for_screen_observer_(task_runner_for_screen_observer), - camera_hal_ipc_thread_("CameraHalIpcThread") {} + camera_hal_ipc_thread_("CameraHalIpcThread"), + initialized_(Init()) {} VideoCaptureDeviceFactoryChromeOS::~VideoCaptureDeviceFactoryChromeOS() { camera_hal_delegate_->Reset(); camera_hal_ipc_thread_.Stop(); } +std::unique_ptr<VideoCaptureDevice> +VideoCaptureDeviceFactoryChromeOS::CreateDevice( + const VideoCaptureDeviceDescriptor& device_descriptor) { + DCHECK(thread_checker_.CalledOnValidThread()); + if (!initialized_) { + return std::unique_ptr<VideoCaptureDevice>(); + } + return camera_hal_delegate_->CreateDevice(task_runner_for_screen_observer_, + device_descriptor); +} + +void VideoCaptureDeviceFactoryChromeOS::GetSupportedFormats( + const VideoCaptureDeviceDescriptor& device_descriptor, + VideoCaptureFormats* supported_formats) { + DCHECK(thread_checker_.CalledOnValidThread()); + if (!initialized_) { + return; + } + camera_hal_delegate_->GetSupportedFormats(device_descriptor, + supported_formats); +} + +void VideoCaptureDeviceFactoryChromeOS::GetDeviceDescriptors( + VideoCaptureDeviceDescriptors* device_descriptors) { + DCHECK(thread_checker_.CalledOnValidThread()); + if (!initialized_) { + return; + } + camera_hal_delegate_->GetDeviceDescriptors(device_descriptors); +} + +// static +bool VideoCaptureDeviceFactoryChromeOS::ShouldEnable() { + // Checks whether the Chrome OS binary which provides the HAL v3 camera + // service is installed on the device. If the binary exists we assume the + // device is using the new camera HAL v3 stack. + const base::FilePath kArcCamera3Service("/usr/bin/arc_camera3_service"); + return base::PathExists(kArcCamera3Service); +} + bool VideoCaptureDeviceFactoryChromeOS::Init() { if (!camera_hal_ipc_thread_.Start()) { LOG(ERROR) << "Module thread failed to start"; @@ -39,40 +80,6 @@ return true; } -std::unique_ptr<VideoCaptureDevice> -VideoCaptureDeviceFactoryChromeOS::CreateDevice( - const VideoCaptureDeviceDescriptor& device_descriptor) { - DCHECK(thread_checker_.CalledOnValidThread()); - DCHECK(camera_hal_delegate_); - return camera_hal_delegate_->CreateDevice(task_runner_for_screen_observer_, - device_descriptor); -} - -void VideoCaptureDeviceFactoryChromeOS::GetSupportedFormats( - const VideoCaptureDeviceDescriptor& device_descriptor, - VideoCaptureFormats* supported_formats) { - DCHECK(thread_checker_.CalledOnValidThread()); - DCHECK(camera_hal_delegate_); - camera_hal_delegate_->GetSupportedFormats(device_descriptor, - supported_formats); -} - -void VideoCaptureDeviceFactoryChromeOS::GetDeviceDescriptors( - VideoCaptureDeviceDescriptors* device_descriptors) { - DCHECK(thread_checker_.CalledOnValidThread()); - DCHECK(camera_hal_delegate_); - camera_hal_delegate_->GetDeviceDescriptors(device_descriptors); -} - -// static -bool VideoCaptureDeviceFactoryChromeOS::ShouldEnable() { - // Checks whether the Chrome OS binary which provides the HAL v3 camera - // service is installed on the device. If the binary exists we assume the - // device is using the new camera HAL v3 stack. - const base::FilePath kArcCamera3Service("/usr/bin/arc_camera3_service"); - return base::PathExists(kArcCamera3Service); -} - #if defined(OS_CHROMEOS) // static VideoCaptureDeviceFactory* @@ -89,12 +96,8 @@ // some special devices that may never be able to implement a camera HAL // v3. if (VideoCaptureDeviceFactoryChromeOS::ShouldEnable()) { - auto factory = base::MakeUnique<VideoCaptureDeviceFactoryChromeOS>( + return new VideoCaptureDeviceFactoryChromeOS( task_runner_for_screen_observer); - if (!factory->Init()) { - return nullptr; - } - return factory.release(); } else { return new VideoCaptureDeviceFactoryLinux(task_runner_for_screen_observer); }
diff --git a/media/capture/video/chromeos/video_capture_device_factory_chromeos.h b/media/capture/video/chromeos/video_capture_device_factory_chromeos.h index 49a5c70..0df76be5 100644 --- a/media/capture/video/chromeos/video_capture_device_factory_chromeos.h +++ b/media/capture/video/chromeos/video_capture_device_factory_chromeos.h
@@ -31,15 +31,15 @@ void GetDeviceDescriptors( VideoCaptureDeviceDescriptors* device_descriptors) final; - // Initializes the factory. The factory is functional only after this call - // succeeds. - bool Init(); - // A run-time check for whether we should enable // VideoCaptureDeviceFactoryChromeOS on the device. static bool ShouldEnable(); private: + // Initializes the factory. The factory is functional only after this call + // succeeds. + bool Init(); + const scoped_refptr<base::SingleThreadTaskRunner> task_runner_for_screen_observer_; @@ -53,6 +53,8 @@ // |camera_hal_ipc_thread_|. scoped_refptr<CameraHalDelegate> camera_hal_delegate_; + bool initialized_; + DISALLOW_COPY_AND_ASSIGN(VideoCaptureDeviceFactoryChromeOS); };
diff --git a/media/gpu/vaapi_wrapper.cc b/media/gpu/vaapi_wrapper.cc index 0df3cbb..4206ded 100644 --- a/media/gpu/vaapi_wrapper.cc +++ b/media/gpu/vaapi_wrapper.cc
@@ -27,6 +27,7 @@ #elif defined(USE_OZONE) #include "third_party/libva/va/drm/va_drm.h" #include "third_party/libva/va/va_drmcommon.h" +#include "third_party/libva/va/va_version.h" #include "ui/gfx/buffer_format_util.h" #include "ui/ozone/public/ozone_platform.h" #include "ui/ozone/public/surface_factory_ozone.h" @@ -1200,8 +1201,12 @@ DVLOG(1) << "VAAPI version: " << major_version_ << "." << minor_version_; } - if (VAAPIVersionLessThan(0, 39)) { - LOG(ERROR) << "VAAPI version < 0.39 is not supported."; + if (major_version_ != VA_MAJOR_VERSION || + minor_version_ != VA_MINOR_VERSION) { + LOG(ERROR) << "This build of Chromium requires VA-API version " + << VA_MAJOR_VERSION << "." << VA_MINOR_VERSION + << ", system version: " << major_version_ << "." + << minor_version_; return false; } return true; @@ -1230,9 +1235,4 @@ } #endif // USE_OZONE -bool VaapiWrapper::VADisplayState::VAAPIVersionLessThan(int major, int minor) { - return (major_version_ < major) || - (major_version_ == major && minor_version_ < minor); -} - } // namespace media
diff --git a/media/gpu/vaapi_wrapper.h b/media/gpu/vaapi_wrapper.h index 948f4fb..abb930f8 100644 --- a/media/gpu/vaapi_wrapper.h +++ b/media/gpu/vaapi_wrapper.h
@@ -250,9 +250,6 @@ #endif // USE_OZONE private: - // Returns true if the VAAPI version is less than the specified version. - bool VAAPIVersionLessThan(int major, int minor); - // Protected by |va_lock_|. int refcount_;
diff --git a/net/quic/core/crypto/crypto_framer.cc b/net/quic/core/crypto/crypto_framer.cc index 77d1bfa4..8d2ca6d 100644 --- a/net/quic/core/crypto/crypto_framer.cc +++ b/net/quic/core/crypto/crypto_framer.cc
@@ -64,6 +64,14 @@ return visitor.release(); } +QuicErrorCode CryptoFramer::error() const { + return error_; +} + +const std::string& CryptoFramer::error_detail() const { + return error_detail_; +} + bool CryptoFramer::ProcessInput(QuicStringPiece input, Perspective perspective) { DCHECK_EQ(QUIC_NO_ERROR, error_); @@ -80,6 +88,10 @@ return true; } +size_t CryptoFramer::InputBytesRemaining() const { + return buffer_.length(); +} + // static QuicData* CryptoFramer::ConstructHandshakeMessage( const CryptoHandshakeMessage& message,
diff --git a/net/quic/core/crypto/crypto_framer.h b/net/quic/core/crypto/crypto_framer.h index 0cf0d64..3daf862 100644 --- a/net/quic/core/crypto/crypto_framer.h +++ b/net/quic/core/crypto/crypto_framer.h
@@ -32,13 +32,29 @@ virtual void OnHandshakeMessage(const CryptoHandshakeMessage& message) = 0; }; +class QUIC_EXPORT_PRIVATE CryptoMessageParser { + public: + virtual ~CryptoMessageParser() {} + + virtual QuicErrorCode error() const = 0; + virtual const std::string& error_detail() const = 0; + + // Processes input data, which must be delivered in order. Returns + // false if there was an error, and true otherwise. + virtual bool ProcessInput(QuicStringPiece input, Perspective perspective) = 0; + + // Returns the number of bytes of buffered input data remaining to be + // parsed. + virtual size_t InputBytesRemaining() const = 0; +}; + // A class for framing the crypto messages that are exchanged in a QUIC // session. -class QUIC_EXPORT_PRIVATE CryptoFramer { +class QUIC_EXPORT_PRIVATE CryptoFramer : public CryptoMessageParser { public: CryptoFramer(); - virtual ~CryptoFramer(); + ~CryptoFramer() override; // ParseMessage parses exactly one message from the given QuicStringPiece. If // there is an error, the message is truncated, or the message has trailing @@ -55,16 +71,16 @@ visitor_ = visitor; } - QuicErrorCode error() const { return error_; } - const std::string& error_detail() const { return error_detail_; } + QuicErrorCode error() const override; + const std::string& error_detail() const override; // Processes input data, which must be delivered in order. Returns // false if there was an error, and true otherwise. - bool ProcessInput(QuicStringPiece input, Perspective perspective); + bool ProcessInput(QuicStringPiece input, Perspective perspective) override; // Returns the number of bytes of buffered input data remaining to be // parsed. - size_t InputBytesRemaining() const { return buffer_.length(); } + size_t InputBytesRemaining() const override; // Returns a new QuicData owned by the caller that contains a serialized // |message|, or nullptr if there was an error.
diff --git a/net/quic/core/quic_crypto_client_stream.cc b/net/quic/core/quic_crypto_client_stream.cc index 0f4a180..251d1301 100644 --- a/net/quic/core/quic_crypto_client_stream.cc +++ b/net/quic/core/quic_crypto_client_stream.cc
@@ -121,10 +121,8 @@ return handshaker_->crypto_negotiated_params(); } -void QuicCryptoClientStream::OnHandshakeMessage( - const CryptoHandshakeMessage& message) { - QuicCryptoClientStreamBase::OnHandshakeMessage(message); - handshaker_->OnHandshakeMessage(message); +CryptoMessageParser* QuicCryptoClientStream::crypto_message_parser() { + return handshaker_->crypto_message_parser(); } bool QuicCryptoClientStream::WasChannelIDSent() const { @@ -146,7 +144,8 @@ ProofVerifyContext* verify_context, QuicCryptoClientConfig* crypto_config, QuicCryptoClientStream::ProofHandler* proof_handler) - : stream_(stream), + : QuicCryptoHandshaker(stream, session), + stream_(stream), session_(session), next_state_(STATE_IDLE), num_client_hellos_(0), @@ -177,6 +176,7 @@ void QuicCryptoClientHandshaker::OnHandshakeMessage( const CryptoHandshakeMessage& message) { + QuicCryptoHandshaker::OnHandshakeMessage(message); if (message.tag() == kSCUP) { if (!handshake_confirmed()) { stream_->CloseConnectionWithDetails( @@ -242,6 +242,10 @@ return *crypto_negotiated_params_; } +CryptoMessageParser* QuicCryptoClientHandshaker::crypto_message_parser() { + return QuicCryptoHandshaker::crypto_message_parser(); +} + void QuicCryptoClientHandshaker::HandleServerConfigUpdateMessage( const CryptoHandshakeMessage& server_config_update) { DCHECK(server_config_update.tag() == kSCUP); @@ -403,7 +407,7 @@ static_cast<size_t>(max_packet_size - kFramingOverhead)); next_state_ = STATE_RECV_REJ; CryptoUtils::HashHandshakeMessage(out, &chlo_hash_, Perspective::IS_CLIENT); - stream_->SendHandshakeMessage(out); + SendHandshakeMessage(out); return; } @@ -437,7 +441,7 @@ *cached->proof_verify_details()); } next_state_ = STATE_RECV_SHLO; - stream_->SendHandshakeMessage(out); + SendHandshakeMessage(out); // Be prepared to decrypt with the new server write key. session()->connection()->SetAlternativeDecrypter( ENCRYPTION_INITIAL,
diff --git a/net/quic/core/quic_crypto_client_stream.h b/net/quic/core/quic_crypto_client_stream.h index 7707c96..ecb06e0 100644 --- a/net/quic/core/quic_crypto_client_stream.h +++ b/net/quic/core/quic_crypto_client_stream.h
@@ -84,9 +84,6 @@ // to handshake confirmation. virtual int num_scup_messages_received() const = 0; - // TODO(nharper): Move this to QuicCryptoClientHandshaker. - virtual void OnHandshakeMessage(const CryptoHandshakeMessage& message) = 0; - // Returns true if a channel ID was sent on this connection. virtual bool WasChannelIDSent() const = 0; @@ -106,6 +103,9 @@ // Returns the parameters negotiated in the crypto handshake. virtual const QuicCryptoNegotiatedParameters& crypto_negotiated_params() const = 0; + + // Used by QuicCryptoStream to parse data received on this stream. + virtual CryptoMessageParser* crypto_message_parser() = 0; }; // ProofHandler is an interface that handles callbacks from the crypto @@ -146,9 +146,7 @@ bool handshake_confirmed() const override; const QuicCryptoNegotiatedParameters& crypto_negotiated_params() const override; - - // CryptoFramerVisitorInterface implementation - void OnHandshakeMessage(const CryptoHandshakeMessage& message) override; + CryptoMessageParser* crypto_message_parser() override; // Returns true if a channel ID was sent on this connection. bool WasChannelIDSent() const; @@ -168,7 +166,8 @@ // An implementation of QuicCryptoClientStream::HandshakerDelegate which uses // QUIC crypto as the crypto handshake protocol. class QUIC_EXPORT_PRIVATE QuicCryptoClientHandshaker - : public QuicCryptoClientStream::HandshakerDelegate { + : public QuicCryptoClientStream::HandshakerDelegate, + public QuicCryptoHandshaker { public: QuicCryptoClientHandshaker( const QuicServerId& server_id, @@ -184,7 +183,6 @@ bool CryptoConnect() override; int num_sent_client_hellos() const override; int num_scup_messages_received() const override; - void OnHandshakeMessage(const CryptoHandshakeMessage& message) override; bool WasChannelIDSent() const override; bool WasChannelIDSourceCallbackRun() const override; std::string chlo_hash() const override; @@ -192,6 +190,10 @@ bool handshake_confirmed() const override; const QuicCryptoNegotiatedParameters& crypto_negotiated_params() const override; + CryptoMessageParser* crypto_message_parser() override; + + // From QuicCryptoHandshaker + void OnHandshakeMessage(const CryptoHandshakeMessage& message) override; private: // ChannelIDSourceCallbackImpl is passed as the callback method to
diff --git a/net/quic/core/quic_crypto_client_stream_test.cc b/net/quic/core/quic_crypto_client_stream_test.cc index 92ca0dd..2453761 100644 --- a/net/quic/core/quic_crypto_client_stream_test.cc +++ b/net/quic/core/quic_crypto_client_stream_test.cc
@@ -60,12 +60,6 @@ stream(), server_options_); } - void ConstructHandshakeMessage(Perspective perspective) { - CryptoFramer framer; - message_data_.reset( - framer.ConstructHandshakeMessage(message_, perspective)); - } - QuicCryptoClientStream* stream() { return session_->GetMutableCryptoStream(); } @@ -77,7 +71,6 @@ std::unique_ptr<TestQuicSpdyClientSession> session_; QuicServerId server_id_; CryptoHandshakeMessage message_; - std::unique_ptr<QuicData> message_data_; QuicCryptoClientConfig crypto_config_; crypto_test_utils::FakeServerOptions server_options_; }; @@ -100,23 +93,19 @@ *connection_, CloseConnection(QUIC_CRYPTO_MESSAGE_AFTER_HANDSHAKE_COMPLETE, _, _)); message_.set_tag(kCHLO); - ConstructHandshakeMessage(Perspective::IS_CLIENT); - stream()->OnStreamFrame(QuicStreamFrame(kCryptoStreamId, /*fin=*/false, - /*offset=*/0, - message_data_->AsStringPiece())); + crypto_test_utils::SendHandshakeMessageToStream(stream(), message_, + Perspective::IS_CLIENT); } TEST_F(QuicCryptoClientStreamTest, BadMessageType) { stream()->CryptoConnect(); message_.set_tag(kCHLO); - ConstructHandshakeMessage(Perspective::IS_CLIENT); EXPECT_CALL(*connection_, CloseConnection(QUIC_INVALID_CRYPTO_MESSAGE_TYPE, "Expected REJ", _)); - stream()->OnStreamFrame(QuicStreamFrame(kCryptoStreamId, /*fin=*/false, - /*offset=*/0, - message_data_->AsStringPiece())); + crypto_test_utils::SendHandshakeMessageToStream(stream(), message_, + Perspective::IS_CLIENT); } TEST_F(QuicCryptoClientStreamTest, NegotiatedParameters) { @@ -224,10 +213,8 @@ const uint64_t expiry_seconds = 60 * 60 * 24 * 2; server_config_update.SetValue(kSTTL, expiry_seconds); - std::unique_ptr<QuicData> data(CryptoFramer::ConstructHandshakeMessage( - server_config_update, Perspective::IS_SERVER)); - stream()->OnStreamFrame(QuicStreamFrame(kCryptoStreamId, /*fin=*/false, - /*offset=*/0, data->AsStringPiece())); + crypto_test_utils::SendHandshakeMessageToStream( + stream(), server_config_update, Perspective::IS_SERVER); // Make sure that the STK and SCFG are cached correctly. EXPECT_EQ("xstk", state->source_address_token()); @@ -286,10 +273,8 @@ EXPECT_TRUE(ok); EXPECT_CALL(*session_, OnProofValid(testing::_)); - std::unique_ptr<QuicData> data(CryptoFramer::ConstructHandshakeMessage( - server_config_update, Perspective::IS_SERVER)); - stream()->OnStreamFrame(QuicStreamFrame(kCryptoStreamId, /*fin=*/false, - /*offset=*/0, data->AsStringPiece())); + crypto_test_utils::SendHandshakeMessageToStream( + stream(), server_config_update, Perspective::IS_SERVER); // Recreate connection with the new config and verify a 0-RTT attempt. CreateConnection(); @@ -308,10 +293,8 @@ CloseConnection(QUIC_CRYPTO_UPDATE_BEFORE_HANDSHAKE_COMPLETE, _, _)); CryptoHandshakeMessage server_config_update; server_config_update.set_tag(kSCUP); - std::unique_ptr<QuicData> data(CryptoFramer::ConstructHandshakeMessage( - server_config_update, Perspective::IS_SERVER)); - stream()->OnStreamFrame(QuicStreamFrame(kCryptoStreamId, /*fin=*/false, - /*offset=*/0, data->AsStringPiece())); + crypto_test_utils::SendHandshakeMessageToStream( + stream(), server_config_update, Perspective::IS_SERVER); } TEST_F(QuicCryptoClientStreamTest, NoChannelID) {
diff --git a/net/quic/core/quic_crypto_server_stream.cc b/net/quic/core/quic_crypto_server_stream.cc index 98c63d5..7119a8e67 100644 --- a/net/quic/core/quic_crypto_server_stream.cc +++ b/net/quic/core/quic_crypto_server_stream.cc
@@ -96,12 +96,6 @@ handshaker()->CancelOutstandingCallbacks(); } -void QuicCryptoServerStream::OnHandshakeMessage( - const CryptoHandshakeMessage& message) { - QuicCryptoServerStreamBase::OnHandshakeMessage(message); - handshaker()->OnHandshakeMessage(message); -} - bool QuicCryptoServerStream::GetBase64SHA256ClientChannelID( string* output) const { return handshaker()->GetBase64SHA256ClientChannelID(output); @@ -169,6 +163,10 @@ return handshaker()->crypto_negotiated_params(); } +CryptoMessageParser* QuicCryptoServerStream::crypto_message_parser() { + return handshaker()->crypto_message_parser(); +} + QuicCryptoServerStream::HandshakerDelegate* QuicCryptoServerStream::handshaker() const { return handshaker_.get(); @@ -181,7 +179,8 @@ bool use_stateless_rejects_if_peer_supported, QuicSession* session, QuicCryptoServerStream::Helper* helper) - : stream_(stream), + : QuicCryptoHandshaker(stream, session), + stream_(stream), session_(session), crypto_config_(crypto_config), compressed_certs_cache_(compressed_certs_cache), @@ -224,6 +223,7 @@ void QuicCryptoServerHandshaker::OnHandshakeMessage( const CryptoHandshakeMessage& message) { + QuicCryptoHandshaker::OnHandshakeMessage(message); ++num_handshake_messages_; chlo_packet_size_ = session()->connection()->GetCurrentPacket().length(); @@ -315,7 +315,7 @@ // retransmitted. session()->connection()->EnableSavingCryptoPackets(); } - stream_->SendHandshakeMessage(*reply); + SendHandshakeMessage(*reply); if (reply->tag() == kSREJ) { DCHECK(use_stateless_rejects_if_peer_supported_); @@ -364,7 +364,7 @@ crypto_negotiated_params_->initial_crypters.decrypter.release()); session()->connection()->SetDiversificationNonce(*diversification_nonce); - stream_->SendHandshakeMessage(*reply); + SendHandshakeMessage(*reply); session()->connection()->SetEncrypter( ENCRYPTION_FORWARD_SECURE, @@ -522,6 +522,10 @@ return *crypto_negotiated_params_; } +CryptoMessageParser* QuicCryptoServerHandshaker::crypto_message_parser() { + return QuicCryptoHandshaker::crypto_message_parser(); +} + void QuicCryptoServerHandshaker::ProcessClientHello( QuicReferenceCountedPointer<ValidateClientHelloResultCallback::Result> result,
diff --git a/net/quic/core/quic_crypto_server_stream.h b/net/quic/core/quic_crypto_server_stream.h index f187991..29271e48 100644 --- a/net/quic/core/quic_crypto_server_stream.h +++ b/net/quic/core/quic_crypto_server_stream.h
@@ -92,9 +92,6 @@ // client hello. virtual void CancelOutstandingCallbacks() = 0; - // TODO(nharper): Move this to QuicCryptoServerHandshaker. - virtual void OnHandshakeMessage(const CryptoHandshakeMessage& message) = 0; - // GetBase64SHA256ClientChannelID sets |*output| to the base64 encoded, // SHA-256 hash of the client's ChannelID key and returns true, if the // client presented a ChannelID. Otherwise it returns false. @@ -135,6 +132,9 @@ // Returns the parameters negotiated in the crypto handshake. virtual const QuicCryptoNegotiatedParameters& crypto_negotiated_params() const = 0; + + // Used by QuicCryptoStream to parse data received on this stream. + virtual CryptoMessageParser* crypto_message_parser() = 0; }; class Helper { @@ -167,7 +167,6 @@ // From QuicCryptoServerStreamBase void CancelOutstandingCallbacks() override; - void OnHandshakeMessage(const CryptoHandshakeMessage& message) override; bool GetBase64SHA256ClientChannelID(std::string* output) const override; void SendServerConfigUpdate( const CachedNetworkParameters* cached_network_params) override; @@ -194,6 +193,7 @@ bool handshake_confirmed() const override; const QuicCryptoNegotiatedParameters& crypto_negotiated_params() const override; + CryptoMessageParser* crypto_message_parser() override; protected: // Provided so that subclasses can provide their own handshaker. @@ -206,7 +206,8 @@ }; class QUIC_EXPORT_PRIVATE QuicCryptoServerHandshaker - : public QuicCryptoServerStream::HandshakerDelegate { + : public QuicCryptoServerStream::HandshakerDelegate, + public QuicCryptoHandshaker { public: // |crypto_config| must outlive the stream. // |session| must outlive the stream. @@ -222,7 +223,6 @@ // From HandshakerDelegate void CancelOutstandingCallbacks() override; - void OnHandshakeMessage(const CryptoHandshakeMessage& message) override; bool GetBase64SHA256ClientChannelID(std::string* output) const override; void SendServerConfigUpdate( const CachedNetworkParameters* cached_network_params) override; @@ -239,10 +239,15 @@ CachedNetworkParameters cached_network_params) override; bool ShouldSendExpectCTHeader() const override; + // From QuicCryptoStream bool encryption_established() const override; bool handshake_confirmed() const override; const QuicCryptoNegotiatedParameters& crypto_negotiated_params() const override; + CryptoMessageParser* crypto_message_parser() override; + + // From QuicCryptoHandshaker + void OnHandshakeMessage(const CryptoHandshakeMessage& message) override; protected: virtual void ProcessClientHello(
diff --git a/net/quic/core/quic_crypto_server_stream_test.cc b/net/quic/core/quic_crypto_server_stream_test.cc index c91de4f..4166a27 100644 --- a/net/quic/core/quic_crypto_server_stream_test.cc +++ b/net/quic/core/quic_crypto_server_stream_test.cc
@@ -132,12 +132,6 @@ client_session_.reset(client_session); } - void ConstructHandshakeMessage(Perspective perspective) { - CryptoFramer framer; - message_data_.reset( - framer.ConstructHandshakeMessage(message_, perspective)); - } - int CompleteCryptoHandshake() { CHECK(server_connection_); CHECK(server_session_ != nullptr); @@ -183,7 +177,6 @@ std::unique_ptr<TestQuicSpdyClientSession> client_session_; CryptoHandshakeMessage message_; - std::unique_ptr<QuicData> message_data_; crypto_test_utils::FakeClientOptions client_options_; // Which QUIC versions the client and server support. @@ -389,22 +382,18 @@ *server_connection_, CloseConnection(QUIC_CRYPTO_MESSAGE_AFTER_HANDSHAKE_COMPLETE, _, _)); message_.set_tag(kCHLO); - ConstructHandshakeMessage(Perspective::IS_CLIENT); - server_stream()->OnStreamFrame( - QuicStreamFrame(kCryptoStreamId, /*fin=*/false, /*offset=*/0, - message_data_->AsStringPiece())); + crypto_test_utils::SendHandshakeMessageToStream(server_stream(), message_, + Perspective::IS_CLIENT); } TEST_P(QuicCryptoServerStreamTest, BadMessageType) { Initialize(); message_.set_tag(kSHLO); - ConstructHandshakeMessage(Perspective::IS_SERVER); EXPECT_CALL(*server_connection_, CloseConnection(QUIC_INVALID_CRYPTO_MESSAGE_TYPE, _, _)); - server_stream()->OnStreamFrame( - QuicStreamFrame(kCryptoStreamId, /*fin=*/false, /*offset=*/0, - message_data_->AsStringPiece())); + crypto_test_utils::SendHandshakeMessageToStream(server_stream(), message_, + Perspective::IS_SERVER); } TEST_P(QuicCryptoServerStreamTest, ChannelID) { @@ -472,7 +461,6 @@ TEST_P(QuicCryptoServerStreamTest, DoesPeerSupportStatelessRejects) { Initialize(); - ConstructHandshakeMessage(Perspective::IS_CLIENT); QuicConfig stateless_reject_config = DefaultQuicConfigStatelessRejects(); stateless_reject_config.ToHandshakeMessage(&message_); EXPECT_TRUE( @@ -570,7 +558,8 @@ // Send in the CHLO, and check that a callback is now pending in the // ProofSource. - server_stream()->OnHandshakeMessage(chlo); + crypto_test_utils::SendHandshakeMessageToStream(server_stream(), chlo, + Perspective::IS_CLIENT); EXPECT_EQ(GetFakeProofSource()->NumPendingCallbacks(), 1); // Send in a second CHLO while processing of the first is still pending. @@ -581,7 +570,8 @@ *server_connection_, CloseConnection(QUIC_CRYPTO_MESSAGE_WHILE_VALIDATING_CLIENT_HELLO, "Unexpected handshake message while processing CHLO", _)); - server_stream()->OnHandshakeMessage(chlo); + crypto_test_utils::SendHandshakeMessageToStream(server_stream(), chlo, + Perspective::IS_CLIENT); } } // namespace
diff --git a/net/quic/core/quic_crypto_stream.cc b/net/quic/core/quic_crypto_stream.cc index 7534419..3af89889 100644 --- a/net/quic/core/quic_crypto_stream.cc +++ b/net/quic/core/quic_crypto_stream.cc
@@ -25,7 +25,6 @@ QuicCryptoStream::QuicCryptoStream(QuicSession* session) : QuicStream(kCryptoStreamId, session) { - crypto_framer_.set_visitor(this); // The crypto stream is exempt from connection level flow control. DisableConnectionFlowControlForThisStream(); } @@ -42,18 +41,6 @@ /*offset=*/0); } -void QuicCryptoStream::OnError(CryptoFramer* framer) { - QUIC_DLOG(WARNING) << "Error processing crypto data: " - << QuicErrorCodeToString(framer->error()); -} - -void QuicCryptoStream::OnHandshakeMessage( - const CryptoHandshakeMessage& message) { - QUIC_DVLOG(1) << ENDPOINT << "Received " - << message.DebugString(session()->perspective()); - session()->OnCryptoHandshakeMessageReceived(message); -} - void QuicCryptoStream::OnDataAvailable() { struct iovec iov; while (true) { @@ -62,13 +49,15 @@ break; } QuicStringPiece data(static_cast<char*>(iov.iov_base), iov.iov_len); - if (!crypto_framer_.ProcessInput(data, session()->perspective())) { - CloseConnectionWithDetails(crypto_framer_.error(), - crypto_framer_.error_detail()); + if (!crypto_message_parser()->ProcessInput(data, + session()->perspective())) { + CloseConnectionWithDetails(crypto_message_parser()->error(), + crypto_message_parser()->error_detail()); return; } sequencer()->MarkConsumed(iov.iov_len); - if (handshake_confirmed() && crypto_framer_.InputBytesRemaining() == 0 && + if (handshake_confirmed() && + crypto_message_parser()->InputBytesRemaining() == 0 && FLAGS_quic_reloadable_flag_quic_release_crypto_stream_buffer) { QUIC_FLAG_COUNT(quic_reloadable_flag_quic_release_crypto_stream_buffer); // If the handshake is complete and the current message has been fully @@ -79,17 +68,6 @@ } } -void QuicCryptoStream::SendHandshakeMessage( - const CryptoHandshakeMessage& message) { - QUIC_DVLOG(1) << ENDPOINT << "Sending " - << message.DebugString(session()->perspective()); - session()->connection()->NeuterUnencryptedPackets(); - session()->OnCryptoHandshakeMessageSent(message); - const QuicData& data = message.GetSerialized(session()->perspective()); - WriteOrBufferData(QuicStringPiece(data.data(), data.length()), false, - nullptr); -} - bool QuicCryptoStream::ExportKeyingMaterial(QuicStringPiece label, QuicStringPiece context, size_t result_len, @@ -116,4 +94,39 @@ /* context= */ "", 32, result); } +QuicCryptoHandshaker::QuicCryptoHandshaker(QuicCryptoStream* stream, + QuicSession* session) + : stream_(stream), session_(session) { + crypto_framer_.set_visitor(this); +} + +QuicCryptoHandshaker::~QuicCryptoHandshaker() {} + +void QuicCryptoHandshaker::SendHandshakeMessage( + const CryptoHandshakeMessage& message) { + QUIC_DVLOG(1) << ENDPOINT << "Sending " + << message.DebugString(session()->perspective()); + session()->connection()->NeuterUnencryptedPackets(); + session()->OnCryptoHandshakeMessageSent(message); + const QuicData& data = message.GetSerialized(session()->perspective()); + stream_->WriteOrBufferData(QuicStringPiece(data.data(), data.length()), false, + nullptr); +} + +void QuicCryptoHandshaker::OnError(CryptoFramer* framer) { + QUIC_DLOG(WARNING) << "Error processing crypto data: " + << QuicErrorCodeToString(framer->error()); +} + +void QuicCryptoHandshaker::OnHandshakeMessage( + const CryptoHandshakeMessage& message) { + QUIC_DVLOG(1) << ENDPOINT << "Received " + << message.DebugString(session()->perspective()); + session()->OnCryptoHandshakeMessageReceived(message); +} + +CryptoMessageParser* QuicCryptoHandshaker::crypto_message_parser() { + return &crypto_framer_; +} + } // namespace net
diff --git a/net/quic/core/quic_crypto_stream.h b/net/quic/core/quic_crypto_stream.h index 3f4b9b8..fdbf4dc0 100644 --- a/net/quic/core/quic_crypto_stream.h +++ b/net/quic/core/quic_crypto_stream.h
@@ -31,9 +31,7 @@ // // For more details: // https://docs.google.com/document/d/1g5nIXAIkN_Y-7XJW5K45IblHd_L2f5LTaDUDwvZ5L6g/edit?usp=sharing -class QUIC_EXPORT_PRIVATE QuicCryptoStream - : public QuicStream, - public CryptoFramerVisitorInterface { +class QUIC_EXPORT_PRIVATE QuicCryptoStream : public QuicStream { public: explicit QuicCryptoStream(QuicSession* session); @@ -43,17 +41,9 @@ // handshake message for |version|. static QuicByteCount CryptoMessageFramingOverhead(QuicVersion version); - // CryptoFramerVisitorInterface implementation - void OnError(CryptoFramer* framer) override; - void OnHandshakeMessage(const CryptoHandshakeMessage& message) override; - // QuicStream implementation void OnDataAvailable() override; - // Sends |message| to the peer. - // TODO(wtc): return a success/failure status. - void SendHandshakeMessage(const CryptoHandshakeMessage& message); - // Performs key extraction to derive a new secret of |result_len| bytes // dependent on |label|, |context|, and the stream's negotiated subkey secret. // Returns false if the handshake has not been confirmed or the parameters are @@ -83,10 +73,38 @@ virtual const QuicCryptoNegotiatedParameters& crypto_negotiated_params() const = 0; + // Provides the message parser to use when data is received on this stream. + virtual CryptoMessageParser* crypto_message_parser() = 0; + private: + DISALLOW_COPY_AND_ASSIGN(QuicCryptoStream); +}; + +class QUIC_EXPORT_PRIVATE QuicCryptoHandshaker + : public CryptoFramerVisitorInterface { + public: + QuicCryptoHandshaker(QuicCryptoStream* stream, QuicSession* session); + + ~QuicCryptoHandshaker() override; + + // Sends |message| to the peer. + // TODO(wtc): return a success/failure status. + void SendHandshakeMessage(const CryptoHandshakeMessage& message); + + void OnError(CryptoFramer* framer) override; + void OnHandshakeMessage(const CryptoHandshakeMessage& message) override; + + CryptoMessageParser* crypto_message_parser(); + + private: + QuicSession* session() { return session_; } + + QuicCryptoStream* stream_; + QuicSession* session_; + CryptoFramer crypto_framer_; - DISALLOW_COPY_AND_ASSIGN(QuicCryptoStream); + DISALLOW_COPY_AND_ASSIGN(QuicCryptoHandshaker); }; } // namespace net
diff --git a/net/quic/core/quic_crypto_stream_test.cc b/net/quic/core/quic_crypto_stream_test.cc index ca24a4fe..bf29b6c 100644 --- a/net/quic/core/quic_crypto_stream_test.cc +++ b/net/quic/core/quic_crypto_stream_test.cc
@@ -24,10 +24,12 @@ namespace test { namespace { -class MockQuicCryptoStream : public QuicCryptoStream { +class MockQuicCryptoStream : public QuicCryptoStream, + public QuicCryptoHandshaker { public: explicit MockQuicCryptoStream(QuicSession* session) : QuicCryptoStream(session), + QuicCryptoHandshaker(this, session), params_(new QuicCryptoNegotiatedParameters) {} void OnHandshakeMessage(const CryptoHandshakeMessage& message) override { @@ -43,6 +45,9 @@ const override { return *params_; } + CryptoMessageParser* crypto_message_parser() override { + return QuicCryptoHandshaker::crypto_message_parser(); + } private: QuicReferenceCountedPointer<QuicCryptoNegotiatedParameters> params_;
diff --git a/net/quic/core/quic_server_session_base_test.cc b/net/quic/core/quic_server_session_base_test.cc index b6ac5dbf..8c1eb879 100644 --- a/net/quic/core/quic_server_session_base_test.cc +++ b/net/quic/core/quic_server_session_base_test.cc
@@ -645,7 +645,8 @@ // Feed the CHLO into the crypto stream, which will trigger a call to // ProofSource::GetProof - crypto_stream->OnHandshakeMessage(chlo); + crypto_test_utils::SendHandshakeMessageToStream(crypto_stream, chlo, + Perspective::IS_CLIENT); ASSERT_EQ(GetFakeProofSource()->NumPendingCallbacks(), 1); // Destroy the stream
diff --git a/net/quic/core/quic_session_test.cc b/net/quic/core/quic_session_test.cc index 8bfcb7d..fd1c15ee 100644 --- a/net/quic/core/quic_session_test.cc +++ b/net/quic/core/quic_session_test.cc
@@ -49,10 +49,11 @@ const SpdyPriority kHighestPriority = kV3HighestPriority; -class TestCryptoStream : public QuicCryptoStream { +class TestCryptoStream : public QuicCryptoStream, public QuicCryptoHandshaker { public: explicit TestCryptoStream(QuicSession* session) : QuicCryptoStream(session), + QuicCryptoHandshaker(this, session), encryption_established_(false), handshake_confirmed_(false), params_(new QuicCryptoNegotiatedParameters) {} @@ -89,10 +90,15 @@ const override { return *params_; } + CryptoMessageParser* crypto_message_parser() override { + return QuicCryptoHandshaker::crypto_message_parser(); + } MOCK_METHOD0(OnCanWrite, void()); private: + using QuicCryptoStream::session; + bool encryption_established_; bool handshake_confirmed_; QuicReferenceCountedPointer<QuicCryptoNegotiatedParameters> params_;
diff --git a/net/quic/test_tools/crypto_test_utils.cc b/net/quic/test_tools/crypto_test_utils.cc index 3cc2d2c44..70206f2 100644 --- a/net/quic/test_tools/crypto_test_utils.cc +++ b/net/quic/test_tools/crypto_test_utils.cc
@@ -488,6 +488,15 @@ crypto_config->AddDefaultConfig(rand, clock, options)); } +void SendHandshakeMessageToStream(QuicCryptoStream* stream, + const CryptoHandshakeMessage& message, + Perspective perspective) { + const QuicData& data = message.GetSerialized(perspective); + QuicStreamFrame frame(kCryptoStreamId, false, stream->stream_bytes_read(), + data.AsStringPiece()); + stream->OnStreamFrame(frame); +} + void CommunicateHandshakeMessages(PacketSavingConnection* client_conn, QuicCryptoStream* client, PacketSavingConnection* server_conn, @@ -928,7 +937,10 @@ ASSERT_EQ(0u, crypto_framer.InputBytesRemaining()); for (const CryptoHandshakeMessage& message : crypto_visitor.messages()) { - dest_stream->OnHandshakeMessage(message); + SendHandshakeMessageToStream(dest_stream, message, + dest_perspective == Perspective::IS_SERVER + ? Perspective::IS_CLIENT + : Perspective::IS_SERVER); } QuicConnectionPeer::SetCurrentPacket(dest_conn, QuicStringPiece(nullptr, 0)); }
diff --git a/net/quic/test_tools/crypto_test_utils.h b/net/quic/test_tools/crypto_test_utils.h index e961220..1bc2317 100644 --- a/net/quic/test_tools/crypto_test_utils.h +++ b/net/quic/test_tools/crypto_test_utils.h
@@ -139,6 +139,12 @@ QuicCryptoServerConfig* crypto_config, const FakeServerOptions& options); +// Sends the handshake message |message| to stream |stream| with the perspective +// that the message is coming from |perspective|. +void SendHandshakeMessageToStream(QuicCryptoStream* stream, + const CryptoHandshakeMessage& message, + Perspective perspective); + // CommunicateHandshakeMessages moves messages from |client| to |server| and // back until |clients|'s handshake has completed. void CommunicateHandshakeMessages(PacketSavingConnection* client_conn,
diff --git a/net/quic/test_tools/mock_crypto_client_stream.cc b/net/quic/test_tools/mock_crypto_client_stream.cc index c5f19069..ba12953 100644 --- a/net/quic/test_tools/mock_crypto_client_stream.cc +++ b/net/quic/test_tools/mock_crypto_client_stream.cc
@@ -29,6 +29,7 @@ verify_context, crypto_config, session), + QuicCryptoHandshaker(this, session), handshake_mode_(handshake_mode), encryption_established_(false), handshake_confirmed_(false), @@ -126,6 +127,10 @@ return *crypto_negotiated_params_; } +CryptoMessageParser* MockCryptoClientStream::crypto_message_parser() { + return &crypto_framer_; +} + void MockCryptoClientStream::SendOnCryptoHandshakeEvent( QuicSession::CryptoHandshakeEvent event) { encryption_established_ = true;
diff --git a/net/quic/test_tools/mock_crypto_client_stream.h b/net/quic/test_tools/mock_crypto_client_stream.h index 4fb896e..30e1f9b 100644 --- a/net/quic/test_tools/mock_crypto_client_stream.h +++ b/net/quic/test_tools/mock_crypto_client_stream.h
@@ -18,7 +18,8 @@ namespace net { -class MockCryptoClientStream : public QuicCryptoClientStream { +class MockCryptoClientStream : public QuicCryptoClientStream, + public QuicCryptoHandshaker { public: // TODO(zhongyi): might consider move HandshakeMode up to // MockCryptoClientStreamFactory. @@ -59,12 +60,11 @@ // QuicCryptoClientStream implementation. bool CryptoConnect() override; - - // QuicCryptoStream implementation. bool encryption_established() const override; bool handshake_confirmed() const override; const QuicCryptoNegotiatedParameters& crypto_negotiated_params() const override; + CryptoMessageParser* crypto_message_parser() override; // Invokes the sessions's CryptoHandshakeEvent method with the specified // event. @@ -72,6 +72,9 @@ HandshakeMode handshake_mode_; + protected: + using QuicCryptoClientStream::session; + private: void SetConfigNegotiated(); @@ -79,6 +82,7 @@ bool handshake_confirmed_; QuicReferenceCountedPointer<QuicCryptoNegotiatedParameters> crypto_negotiated_params_; + CryptoFramer crypto_framer_; const QuicServerId server_id_; const ProofVerifyDetailsChromium* proof_verify_details_;
diff --git a/net/quic/test_tools/quic_test_utils.cc b/net/quic/test_tools/quic_test_utils.cc index 09ff585..0b42b8a 100644 --- a/net/quic/test_tools/quic_test_utils.cc +++ b/net/quic/test_tools/quic_test_utils.cc
@@ -412,6 +412,10 @@ return *params_; } +CryptoMessageParser* MockQuicCryptoStream::crypto_message_parser() { + return &crypto_framer_; +} + MockQuicSpdySession::MockQuicSpdySession(QuicConnection* connection) : QuicSpdySession(connection, nullptr, DefaultQuicConfig()) { crypto_stream_.reset(new MockQuicCryptoStream(this));
diff --git a/net/quic/test_tools/quic_test_utils.h b/net/quic/test_tools/quic_test_utils.h index 414a9c01..500f761 100644 --- a/net/quic/test_tools/quic_test_utils.h +++ b/net/quic/test_tools/quic_test_utils.h
@@ -537,9 +537,11 @@ bool handshake_confirmed() const override; const QuicCryptoNegotiatedParameters& crypto_negotiated_params() const override; + CryptoMessageParser* crypto_message_parser() override; private: QuicReferenceCountedPointer<QuicCryptoNegotiatedParameters> params_; + CryptoFramer crypto_framer_; }; class MockQuicSpdySession : public QuicSpdySession {
diff --git a/net/tools/quic/quic_client_session_test.cc b/net/tools/quic/quic_client_session_test.cc index 87ab095..b45312a 100644 --- a/net/tools/quic/quic_client_session_test.cc +++ b/net/tools/quic/quic_client_session_test.cc
@@ -169,7 +169,8 @@ CryptoHandshakeMessage rej; crypto_test_utils::FillInDummyReject(&rej, /* stateless */ false); EXPECT_TRUE(session_->IsEncryptionEstablished()); - session_->GetMutableCryptoStream()->OnHandshakeMessage(rej); + crypto_test_utils::SendHandshakeMessageToStream( + session_->GetMutableCryptoStream(), rej, Perspective::IS_CLIENT); EXPECT_FALSE(session_->IsEncryptionEstablished()); EXPECT_EQ(ENCRYPTION_NONE, QuicPacketCreatorPeer::GetEncryptionLevel(
diff --git a/pdf/out_of_process_instance.cc b/pdf/out_of_process_instance.cc index 8b9c9f19..b7bbddca 100644 --- a/pdf/out_of_process_instance.cc +++ b/pdf/out_of_process_instance.cc
@@ -528,11 +528,20 @@ dict.Get(pp::Var(kJSPrintPreviewUrl)).is_string() && dict.Get(pp::Var(kJSPrintPreviewGrayscale)).is_bool() && dict.Get(pp::Var(kJSPrintPreviewPageCount)).is_int()) { + int print_preview_page_count = + std::max(dict.Get(pp::Var(kJSPrintPreviewPageCount)).AsInt(), 0); + if (print_preview_page_count <= 0) { + NOTREACHED(); + return; + } + + print_preview_page_count_ = print_preview_page_count; url_ = dict.Get(pp::Var(kJSPrintPreviewUrl)).AsString(); // For security reasons we crash if the URL that is trying to be loaded here // isn't a print preview one. CHECK(IsPrintPreview()); CHECK(IsPrintPreviewUrl(url_)); + preview_pages_info_ = std::queue<PreviewPageInfo>(); preview_document_load_state_ = LOAD_STATE_COMPLETE; document_load_state_ = LOAD_STATE_LOADING; @@ -542,9 +551,6 @@ engine_->SetGrayscale(dict.Get(pp::Var(kJSPrintPreviewGrayscale)).AsBool()); engine_->New(url_.c_str(), nullptr /* empty header */); - print_preview_page_count_ = - std::max(dict.Get(pp::Var(kJSPrintPreviewPageCount)).AsInt(), 0); - paint_manager_.InvalidateRect(pp::Rect(pp::Point(), plugin_size_)); } else if (type == kJSLoadPreviewPageType && dict.Get(pp::Var(kJSPreviewPageUrl)).is_string() && @@ -1387,20 +1393,12 @@ preview_document_load_state_ = LOAD_STATE_COMPLETE; - const std::string& url = preview_pages_info_.front().first; int dest_page_index = preview_pages_info_.front().second; - int src_page_index = ExtractPrintPreviewPageIndex(url); - if (src_page_index > 0 && dest_page_index > -1 && preview_engine_) - engine_->AppendPage(preview_engine_.get(), dest_page_index); + DCHECK_GE(dest_page_index, 0); + DCHECK(preview_engine_); + engine_->AppendPage(preview_engine_.get(), dest_page_index); - preview_pages_info_.pop(); - // |print_preview_page_count_| is not updated yet. Do not load any - // other preview pages until this information is available. - if (print_preview_page_count_ == 0) - return; - - if (!preview_pages_info_.empty()) - LoadAvailablePreviewPage(); + LoadNextPreviewPage(); } void OutOfProcessInstance::DocumentLoadFailed() { @@ -1437,10 +1435,7 @@ } preview_document_load_state_ = LOAD_STATE_FAILED; - preview_pages_info_.pop(); - - if (!preview_pages_info_.empty()) - LoadAvailablePreviewPage(); + LoadNextPreviewPage(); } pp::Instance* OutOfProcessInstance::GetPluginInstance() { @@ -1602,8 +1597,20 @@ int dest_page_index) { DCHECK(IsPrintPreview()); + if (dest_page_index < 0 || dest_page_index >= print_preview_page_count_) { + NOTREACHED(); + return; + } + int src_page_index = ExtractPrintPreviewPageIndex(url); - if (src_page_index < 1) + if (src_page_index < 0) { + NOTREACHED(); + return; + } + + // Print Preview JS will call loadPreviewPage() for every page, including + // page 0. Just ignore it. + if (src_page_index == 0) return; preview_pages_info_.push(std::make_pair(url, dest_page_index)); @@ -1612,22 +1619,24 @@ void OutOfProcessInstance::LoadAvailablePreviewPage() { if (preview_pages_info_.empty() || - document_load_state_ != LOAD_STATE_COMPLETE) { - return; - } - - const std::string& url = preview_pages_info_.front().first; - int dest_page_index = preview_pages_info_.front().second; - int src_page_index = ExtractPrintPreviewPageIndex(url); - if (src_page_index < 1 || dest_page_index >= print_preview_page_count_ || + document_load_state_ != LOAD_STATE_COMPLETE || preview_document_load_state_ == LOAD_STATE_LOADING) { return; } preview_document_load_state_ = LOAD_STATE_LOADING; + const std::string& url = preview_pages_info_.front().first; LoadUrl(url, /*is_print_preview=*/true); } +void OutOfProcessInstance::LoadNextPreviewPage() { + DCHECK(!preview_pages_info_.empty()); + preview_pages_info_.pop(); + + if (!preview_pages_info_.empty()) + LoadAvailablePreviewPage(); +} + void OutOfProcessInstance::UserMetricsRecordAction(const std::string& action) { // TODO(raymes): Move this function to PPB_UMA_Private. pp::PDF::UserMetricsRecordAction(this, pp::Var(action));
diff --git a/pdf/out_of_process_instance.h b/pdf/out_of_process_instance.h index ab8a0f0..e4f33b2 100644 --- a/pdf/out_of_process_instance.h +++ b/pdf/out_of_process_instance.h
@@ -208,6 +208,9 @@ // Load the next available preview page into the blank page. void LoadAvailablePreviewPage(); + // Called after a preview page has loaded or failed to load. + void LoadNextPreviewPage(); + // Bound the given scroll offset to the document. pp::FloatPoint BoundScrollOffsetToDocument( const pp::FloatPoint& scroll_offset); @@ -331,9 +334,12 @@ std::vector<int> print_preview_page_numbers_; // Used to manage loaded print preview page information. A |PreviewPageInfo| - // consists of data source url string and the page index in the destination + // consists of data source URL string and the page index in the destination // document. - typedef std::pair<std::string, int> PreviewPageInfo; + // The URL string embeds a page number that can be found with + // ExtractPrintPreviewPageIndex(). This page number is always greater than 0. + // The page index is always in the range of [0, print_preview_page_count_). + using PreviewPageInfo = std::pair<std::string, int>; std::queue<PreviewPageInfo> preview_pages_info_; // Used to signal the browser about focus changes to trigger the OSK.
diff --git a/testing/buildbot/chromium.fyi.json b/testing/buildbot/chromium.fyi.json index d23fc91..2e8008c 100644 --- a/testing/buildbot/chromium.fyi.json +++ b/testing/buildbot/chromium.fyi.json
@@ -10009,11 +10009,8 @@ }, "Fuchsia": { "additional_compile_targets": [ - "d8", "gl_unittests", - "media_unittests", - "skia_unittests", - "ui_base_unittests" + "media_unittests" ], "gtest_tests": [ { @@ -10078,16 +10075,28 @@ "can_use_on_swarming_builders": false }, "test": "net_unittests" + }, + { + "swarming": { + "can_use_on_swarming_builders": false + }, + "test": "skia_unittests" + }, + { + "args": [ + "--test-launcher-filter-file=../../testing/buildbot/filters/fuchsia.ui_base_unittests.filter" + ], + "swarming": { + "can_use_on_swarming_builders": false + }, + "test": "ui_base_unittests" } ] }, "Fuchsia (dbg)": { "additional_compile_targets": [ - "d8", "gl_unittests", - "media_unittests", - "skia_unittests", - "ui_base_unittests" + "media_unittests" ], "gtest_tests": [ { @@ -10152,6 +10161,21 @@ "can_use_on_swarming_builders": false }, "test": "net_unittests" + }, + { + "swarming": { + "can_use_on_swarming_builders": false + }, + "test": "skia_unittests" + }, + { + "args": [ + "--test-launcher-filter-file=../../testing/buildbot/filters/fuchsia.ui_base_unittests.filter" + ], + "swarming": { + "can_use_on_swarming_builders": false + }, + "test": "ui_base_unittests" } ] },
diff --git a/testing/buildbot/filters/fuchsia.ui_base_unittests.filter b/testing/buildbot/filters/fuchsia.ui_base_unittests.filter new file mode 100644 index 0000000..709b7fb --- /dev/null +++ b/testing/buildbot/filters/fuchsia.ui_base_unittests.filter
@@ -0,0 +1,9 @@ +# TODO(fuchsia): Fix these tests and remove the filter. crbug.com/740608 . + +-*DataPackTest.* +-BytesFormattingTest.FormatBytes +-ResourceBundleImageTest.* +-ResourceBundleTest.DelegateGetNativeImageNamed +-ResourceBundleTest.DelegateGetPathForLocalePack +-ResourceBundleTest.LocaleDataPakExists +-TimeFormatTest.*
diff --git a/testing/buildbot/gn_isolate_map.pyl b/testing/buildbot/gn_isolate_map.pyl index f5c36f8..fc2fe81 100644 --- a/testing/buildbot/gn_isolate_map.pyl +++ b/testing/buildbot/gn_isolate_map.pyl
@@ -403,10 +403,6 @@ "label": "//crypto:crypto_unittests", "type": "console_test_launcher", }, - "d8": { - "label": "//v8:d8", - "type": "additional_compile_target", - }, "dbus_unittests": { "label": "//dbus:dbus_unittests", "type": "windowed_test_launcher",
diff --git a/third_party/WebKit/LayoutTests/TestExpectations b/third_party/WebKit/LayoutTests/TestExpectations index 2da2057..46786d2 100644 --- a/third_party/WebKit/LayoutTests/TestExpectations +++ b/third_party/WebKit/LayoutTests/TestExpectations
@@ -835,7 +835,6 @@ crbug.com/108417 external/wpt/html/rendering/non-replaced-elements/tables/table-border-1.html [ Failure ] crbug.com/490511 external/wpt/html/rendering/non-replaced-elements/the-hr-element-0/color.html [ Failure ] crbug.com/490511 external/wpt/html/rendering/non-replaced-elements/the-hr-element-0/width.html [ Failure ] -crbug.com/742672 external/wpt/html/semantics/document-metadata/the-meta-element/pragma-directives/attr-meta-http-equiv-refresh/moving-documents.html [ Failure Pass ] crbug.com/692560 external/wpt/html/semantics/document-metadata/styling/LinkStyle.html [ Failure Pass ] crbug.com/627706 external/wpt/html/semantics/embedded-content/the-img-element/invalid-src.html [ Skip ] @@ -3106,3 +3105,7 @@ crbug.com/626703 [ Win7 ] external/wpt/domxpath/xml_xpath_runner.html [ Timeout Pass ] crbug.com/626703 [ Win7 ] external/wpt/html/semantics/document-metadata/the-meta-element/pragma-directives/attr-meta-http-equiv-refresh/parsing.html [ Failure Pass ] crbug.com/626703 [ Win7 ] external/wpt/html/syntax/parsing/html5lib_tests16.html?run_type=uri [ Timeout Pass ] + +crbug.com/746128 [ Win7 ] media/controls/video-enter-exit-fullscreen-without-hovering-doesnt-show-controls.html [ Failure ] + +crbug.com/626703 [ Win7 ] external/wpt/image-decodes/image-decode-path-changes.html [ Pass Crash ] \ No newline at end of file
diff --git a/third_party/WebKit/LayoutTests/accessibility/aom-string-properties.html b/third_party/WebKit/LayoutTests/accessibility/aom-string-properties.html index 58524ee..10b9346 100644 --- a/third_party/WebKit/LayoutTests/accessibility/aom-string-properties.html +++ b/third_party/WebKit/LayoutTests/accessibility/aom-string-properties.html
@@ -61,6 +61,20 @@ }, "AccessibleNode.current"); </script> +<div role="textbox" id="hasPopUp"></div> + +<script> +test(function(t) { + var node = document.getElementById("hasPopUp"); + var axNode = accessibilityController.accessibleElementById("hasPopUp"); + assert_equals(axNode.hasPopup, false); + node.accessibleNode.hasPopUp = true; + assert_equals(axNode.hasPopup, true); + // TODO(dmazzoni): Test support for ARIA 1.1 values like "dialog", "tree", etc. + // when those are mapped through to platform APIs. +}, "AccessibleNode.hasPopUp"); +</script> + <div role="textbox" id="invalid"></div> <script>
diff --git a/third_party/WebKit/LayoutTests/external/wpt/html/dom/reflection-tabular-expected.txt b/third_party/WebKit/LayoutTests/external/wpt/html/dom/reflection-tabular-expected.txt index 49b5c2f..b85d44f 100644 --- a/third_party/WebKit/LayoutTests/external/wpt/html/dom/reflection-tabular-expected.txt +++ b/third_party/WebKit/LayoutTests/external/wpt/html/dom/reflection-tabular-expected.txt
@@ -32,10 +32,7 @@ PASS colgroup.tabIndex: 24 tests PASS colgroup.span: 11 tests FAIL colgroup.span: setAttribute() to 4294967296 assert_equals: IDL get expected 1000 but got 1 -PASS colgroup.span: 49 tests -FAIL colgroup.span: IDL set to 2147483648 assert_equals: getAttribute() expected "1" but got "0" -FAIL colgroup.span: IDL set to 4294967295 assert_equals: getAttribute() expected "1" but got "0" -PASS colgroup.span: 2 tests +PASS colgroup.span: 53 tests PASS colgroup.align: 32 tests PASS colgroup.ch (<colgroup char>): 32 tests PASS colgroup.chOff (<colgroup charoff>): 32 tests @@ -50,10 +47,7 @@ PASS col.tabIndex: 24 tests PASS col.span: 11 tests FAIL col.span: setAttribute() to 4294967296 assert_equals: IDL get expected 1000 but got 1 -PASS col.span: 49 tests -FAIL col.span: IDL set to 2147483648 assert_equals: getAttribute() expected "1" but got "0" -FAIL col.span: IDL set to 4294967295 assert_equals: getAttribute() expected "1" but got "0" -PASS col.span: 2 tests +PASS col.span: 53 tests PASS col.align: 32 tests PASS col.ch (<col char>): 32 tests PASS col.chOff (<col charoff>): 32 tests @@ -113,10 +107,7 @@ PASS td.tabIndex: 24 tests PASS td.colSpan: 11 tests FAIL td.colSpan: setAttribute() to 4294967296 assert_equals: IDL get expected 1000 but got 1 -PASS td.colSpan: 49 tests -FAIL td.colSpan: IDL set to 2147483648 assert_equals: getAttribute() expected "1" but got "0" -FAIL td.colSpan: IDL set to 4294967295 assert_equals: getAttribute() expected "1" but got "0" -PASS td.colSpan: 2 tests +PASS td.colSpan: 53 tests PASS td.rowSpan: 6 tests FAIL td.rowSpan: setAttribute() to 0 assert_equals: IDL get expected 0 but got 1 PASS td.rowSpan: 4 tests @@ -128,9 +119,7 @@ FAIL td.rowSpan: IDL set to 0 assert_equals: IDL get expected 0 but got 1 PASS td.rowSpan: 3 tests FAIL td.rowSpan: IDL set to "-0" assert_equals: IDL get expected 0 but got 1 -FAIL td.rowSpan: IDL set to 2147483648 assert_equals: getAttribute() expected "1" but got "0" -FAIL td.rowSpan: IDL set to 4294967295 assert_equals: getAttribute() expected "1" but got "0" -PASS td.rowSpan: 2 tests +PASS td.rowSpan: 4 tests PASS td.scope: 72 tests PASS td.abbr: 32 tests PASS td.align: 32 tests @@ -151,10 +140,7 @@ PASS th.tabIndex: 24 tests PASS th.colSpan: 11 tests FAIL th.colSpan: setAttribute() to 4294967296 assert_equals: IDL get expected 1000 but got 1 -PASS th.colSpan: 49 tests -FAIL th.colSpan: IDL set to 2147483648 assert_equals: getAttribute() expected "1" but got "0" -FAIL th.colSpan: IDL set to 4294967295 assert_equals: getAttribute() expected "1" but got "0" -PASS th.colSpan: 2 tests +PASS th.colSpan: 53 tests PASS th.rowSpan: 6 tests FAIL th.rowSpan: setAttribute() to 0 assert_equals: IDL get expected 0 but got 1 PASS th.rowSpan: 4 tests @@ -166,9 +152,7 @@ FAIL th.rowSpan: IDL set to 0 assert_equals: IDL get expected 0 but got 1 PASS th.rowSpan: 3 tests FAIL th.rowSpan: IDL set to "-0" assert_equals: IDL get expected 0 but got 1 -FAIL th.rowSpan: IDL set to 2147483648 assert_equals: getAttribute() expected "1" but got "0" -FAIL th.rowSpan: IDL set to 4294967295 assert_equals: getAttribute() expected "1" but got "0" -PASS th.rowSpan: 2 tests +PASS th.rowSpan: 4 tests PASS th.scope: 72 tests PASS th.abbr: 32 tests PASS th.align: 32 tests
diff --git a/third_party/WebKit/LayoutTests/external/wpt/html/semantics/document-metadata/the-meta-element/pragma-directives/attr-meta-http-equiv-refresh/moving-documents.html b/third_party/WebKit/LayoutTests/external/wpt/html/semantics/document-metadata/the-meta-element/pragma-directives/attr-meta-http-equiv-refresh/moving-documents.html deleted file mode 100644 index 241a8e0..0000000 --- a/third_party/WebKit/LayoutTests/external/wpt/html/semantics/document-metadata/the-meta-element/pragma-directives/attr-meta-http-equiv-refresh/moving-documents.html +++ /dev/null
@@ -1,51 +0,0 @@ -<!DOCTYPE html> -<meta charset="utf-8"> -<title>When moving between documents, must refresh the original document</title> -<script src="/resources/testharness.js"></script> -<script src="/resources/testharnessreport.js"></script> -<link rel="help" href="https://html.spec.whatwg.org/multipage/semantics.html#attr-meta-http-equiv-refresh"> - -<div id="log"></div> - -<script> -"use strict"; - -const sourceIFrame = document.createElement("iframe"); -const destIFrame = document.createElement("iframe"); -let sourceLoadCount = 0; -let destLoadCount = 0; - -sourceIFrame.onload = () => { - ++sourceLoadCount; - - if (sourceLoadCount === 2) { - assert_equals(sourceIFrame.contentDocument.body.textContent.trim(), "foo"); - done(); - } - - maybeStartTest(); -}; - -destIFrame.onload = () => { - ++destLoadCount; - - if (destLoadCount === 2) { - assert_unreached("The iframe into which the meta was moved must not refresh"); - } - - maybeStartTest(); -}; - -function maybeStartTest() { - if (sourceLoadCount === 1 && destLoadCount === 1) { - const meta = sourceIFrame.contentDocument.querySelector("meta"); - destIFrame.contentDocument.body.appendChild(meta); - } -} - -sourceIFrame.src = "support/refresh.sub.html?input=" + encodeURIComponent("1; url=foo"); -destIFrame.src = "support/ufoo"; - -document.body.appendChild(sourceIFrame); -document.body.appendChild(destIFrame); -</script>
diff --git a/third_party/WebKit/LayoutTests/external/wpt/html/semantics/document-metadata/the-meta-element/pragma-directives/attr-meta-http-equiv-refresh/parsing.html b/third_party/WebKit/LayoutTests/external/wpt/html/semantics/document-metadata/the-meta-element/pragma-directives/attr-meta-http-equiv-refresh/parsing.html index cbfc180..e96251cf 100644 --- a/third_party/WebKit/LayoutTests/external/wpt/html/semantics/document-metadata/the-meta-element/pragma-directives/attr-meta-http-equiv-refresh/parsing.html +++ b/third_party/WebKit/LayoutTests/external/wpt/html/semantics/document-metadata/the-meta-element/pragma-directives/attr-meta-http-equiv-refresh/parsing.html
@@ -1,5 +1,6 @@ <!doctype html> <meta charset=utf-8> +<meta name=timeout content=long> <title>Parsing of meta refresh</title> <script src=/resources/testharness.js></script> <script src=/resources/testharnessreport.js></script>
diff --git a/third_party/WebKit/LayoutTests/external/wpt/html/semantics/document-metadata/the-meta-element/pragma-directives/attr-meta-http-equiv-refresh/remove-from-document.html b/third_party/WebKit/LayoutTests/external/wpt/html/semantics/document-metadata/the-meta-element/pragma-directives/attr-meta-http-equiv-refresh/remove-from-document.html new file mode 100644 index 0000000..66a067f --- /dev/null +++ b/third_party/WebKit/LayoutTests/external/wpt/html/semantics/document-metadata/the-meta-element/pragma-directives/attr-meta-http-equiv-refresh/remove-from-document.html
@@ -0,0 +1,36 @@ +<!DOCTYPE html> +<meta charset="utf-8"> +<title>A meta must refresh the original document even if it was removed.</title> +<script src="/resources/testharness.js"></script> +<script src="/resources/testharnessreport.js"></script> +<link rel="help" href="https://html.spec.whatwg.org/multipage/semantics.html#attr-meta-http-equiv-refresh"> + +<div id="log"></div> + +<script> +"use strict"; + +const sourceIFrame = document.createElement("iframe"); +let sourceLoadCount = 0; + +sourceIFrame.onload = () => { + ++sourceLoadCount; + + if (sourceLoadCount === 2) { + assert_equals(sourceIFrame.contentDocument.body.textContent.trim(), "foo"); + done(); + } + + maybeStartTest(); +}; + +function maybeStartTest() { + if (sourceLoadCount === 1) { + sourceIFrame.contentDocument.querySelector("meta").remove(); + } +} + +sourceIFrame.src = "support/refresh.sub.html?input=" + encodeURIComponent("1; url=foo"); + +document.body.appendChild(sourceIFrame); +</script>
diff --git a/third_party/WebKit/LayoutTests/http/tests/worklet/webexposed/global-interface-listing-paint-worklet-expected.txt b/third_party/WebKit/LayoutTests/http/tests/worklet/webexposed/global-interface-listing-paint-worklet-expected.txt index 678a90d..ab6f4184 100644 --- a/third_party/WebKit/LayoutTests/http/tests/worklet/webexposed/global-interface-listing-paint-worklet-expected.txt +++ b/third_party/WebKit/LayoutTests/http/tests/worklet/webexposed/global-interface-listing-paint-worklet-expected.txt
@@ -29,6 +29,8 @@ CONSOLE MESSAGE: line 147: getter x CONSOLE MESSAGE: line 147: getter y CONSOLE MESSAGE: line 147: method constructor +CONSOLE MESSAGE: line 147: setter x +CONSOLE MESSAGE: line 147: setter y CONSOLE MESSAGE: line 147: interface CSSResourceValue : CSSStyleValue CONSOLE MESSAGE: line 147: getter state CONSOLE MESSAGE: line 147: method constructor
diff --git a/third_party/WebKit/LayoutTests/typedcssom/cssPositionValue-expected.txt b/third_party/WebKit/LayoutTests/typedcssom/cssPositionValue-expected.txt deleted file mode 100644 index 38423efd..0000000 --- a/third_party/WebKit/LayoutTests/typedcssom/cssPositionValue-expected.txt +++ /dev/null
@@ -1,4 +0,0 @@ -This is a testharness.js-based test. -FAIL toString() returns a string with the x and y positions cssStrings separated by a space CSSCalcValue is not defined -Harness: the test ran to completion. -
diff --git a/third_party/WebKit/LayoutTests/typedcssom/cssPositionValue.html b/third_party/WebKit/LayoutTests/typedcssom/cssPositionValue.html index 39298f2a..58df3e1 100644 --- a/third_party/WebKit/LayoutTests/typedcssom/cssPositionValue.html +++ b/third_party/WebKit/LayoutTests/typedcssom/cssPositionValue.html
@@ -4,18 +4,50 @@ <script> -test(function() { - assert_equals(new CSSPositionValue(new CSSUnitValue(50, 'px'), - new CSSCalcValue({px: -10, em: -3.2, pt: 0})).toString(), '50px calc((-3.2em - 10px) + 0pt)'); - assert_equals(new CSSPositionValue(new CSSUnitValue(50, 'px'), - new CSSUnitValue(2, 'em')).toString(), '50px 2em'); - assert_equals(new CSSPositionValue(new CSSCalcValue({px: -10, em: -3.2, pt: 0}), - new CSSCalcValue({px: -10, em: 3.2})).toString(), 'calc((-3.2em - 10px) + 0pt) calc(3.2em - 10px)'); - assert_equals(new CSSPositionValue(new CSSCalcValue({px: -10, em: -3.2, pt: 0}), - new CSSUnitValue(10, 'percent')).toString(), 'calc((-3.2em - 10px) + 0pt) 10%'); -}, "toString() returns a string with the x and y positions cssStrings separated by a space"); +test(() => { + let position = new CSSPositionValue(CSS.percent(1), CSS.percent(2)); + assert_equals(position.x.value, 1); + assert_equals(position.y.value, 2); + assert_equals(position.x.unit, 'percent'); + assert_equals(position.y.unit, 'percent'); +}, "Constructor accepts percentages"); + +test(() => { + assert_throws(new TypeError(), () => { + new CSSPositionValue(CSS.deg(1), CSS.px(1)); + }); + assert_throws(new TypeError(), () => { + new CSSPositionValue(CSS.px(1), CSS.deg(1)); + }); +}, "Constructor throws if a CSSNumericValue that is not a length or " + + "percentage is given"); + +for (let xy of ["x", "y"]) { + test(() => { + let position = new CSSPositionValue(CSS.px(1), CSS.px(2)); + position[xy] = CSS.em(3); + assert_equals(position[xy].value, 3); + assert_equals(position[xy].unit, 'em'); + }, xy + " can be set to a length"); + + test(() => { + let position = new CSSPositionValue(CSS.px(1), CSS.px(2)); + position[xy] = CSS.percent(3); + assert_equals(position[xy].value, 3); + assert_equals(position[xy].unit, 'percent'); + }, xy + " can be set to a percentage"); + + test(() => { + let position = new CSSPositionValue(CSS.px(1), CSS.px(2)); + assert_throws(new TypeError(), () => { + position[xy] = CSS.deg(3); + }); + }, "Setting " + xy + " to something not a length or percentage throws"); +} + +test(() => { + assert_equals( + new CSSPositionValue(CSS.px(50), CSS.em(2)).toString(), '50px 2em'); +}, "toString returns the x and y cssText separated by a space"); </script> - -<body> -</body>
diff --git a/third_party/WebKit/LayoutTests/virtual/service-worker-navigation-preload-disabled/webexposed/global-interface-listing-expected.txt b/third_party/WebKit/LayoutTests/virtual/service-worker-navigation-preload-disabled/webexposed/global-interface-listing-expected.txt index 915b86e..1eb66da24 100644 --- a/third_party/WebKit/LayoutTests/virtual/service-worker-navigation-preload-disabled/webexposed/global-interface-listing-expected.txt +++ b/third_party/WebKit/LayoutTests/virtual/service-worker-navigation-preload-disabled/webexposed/global-interface-listing-expected.txt
@@ -32,6 +32,7 @@ getter errorMessage getter expanded getter flowTo + getter hasPopUp getter hidden getter invalid getter keyShortcuts @@ -79,6 +80,7 @@ setter errorMessage setter expanded setter flowTo + setter hasPopUp setter hidden setter invalid setter keyShortcuts @@ -729,6 +731,8 @@ getter x getter y method constructor + setter x + setter y interface CSSResourceValue : CSSStyleValue attribute @@toStringTag getter state
diff --git a/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt b/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt index 6bc4c458..7bb150cf 100644 --- a/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt +++ b/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt
@@ -32,6 +32,7 @@ getter errorMessage getter expanded getter flowTo + getter hasPopUp getter hidden getter invalid getter keyShortcuts @@ -79,6 +80,7 @@ setter errorMessage setter expanded setter flowTo + setter hasPopUp setter hidden setter invalid setter keyShortcuts @@ -729,6 +731,8 @@ getter x getter y method constructor + setter x + setter y interface CSSResourceValue : CSSStyleValue attribute @@toStringTag getter state
diff --git a/third_party/WebKit/Source/core/BUILD.gn b/third_party/WebKit/Source/core/BUILD.gn index 325958b..c120cc0 100644 --- a/third_party/WebKit/Source/core/BUILD.gn +++ b/third_party/WebKit/Source/core/BUILD.gn
@@ -470,6 +470,7 @@ "$blink_core_output_dir/css/properties/CSSPropertyAPIAnimationIterationCount.h", "$blink_core_output_dir/css/properties/CSSPropertyAPIAnimationName.h", "$blink_core_output_dir/css/properties/CSSPropertyAPIAnimationPlayState.h", + "$blink_core_output_dir/css/properties/CSSPropertyAPIAutoOrString.h", "$blink_core_output_dir/css/properties/CSSPropertyAPIBaselineShift.h", "$blink_core_output_dir/css/properties/CSSPropertyAPIBorderColor.h", "$blink_core_output_dir/css/properties/CSSPropertyAPIBorderImageOutset.h", @@ -594,9 +595,12 @@ "$blink_core_output_dir/css/properties/CSSPropertyDescriptor.h", "$blink_core_output_dir/css/properties/CSSShorthandPropertyAPIAnimation.h", "$blink_core_output_dir/css/properties/CSSShorthandPropertyAPIBackgroundPosition.h", + "$blink_core_output_dir/css/properties/CSSShorthandPropertyAPIBorderColor.h", "$blink_core_output_dir/css/properties/CSSShorthandPropertyAPIBorderImage.h", "$blink_core_output_dir/css/properties/CSSShorthandPropertyAPIBorderRadius.h", "$blink_core_output_dir/css/properties/CSSShorthandPropertyAPIBorderSpacing.h", + "$blink_core_output_dir/css/properties/CSSShorthandPropertyAPIBorderStyle.h", + "$blink_core_output_dir/css/properties/CSSShorthandPropertyAPIBorderWidth.h", "$blink_core_output_dir/css/properties/CSSShorthandPropertyAPIColumns.h", "$blink_core_output_dir/css/properties/CSSShorthandPropertyAPIFlex.h", "$blink_core_output_dir/css/properties/CSSShorthandPropertyAPIFont.h", @@ -605,8 +609,11 @@ "$blink_core_output_dir/css/properties/CSSShorthandPropertyAPIOutline.h", "$blink_core_output_dir/css/properties/CSSShorthandPropertyAPIOverflow.h", "$blink_core_output_dir/css/properties/CSSShorthandPropertyAPIOffset.h", + "$blink_core_output_dir/css/properties/CSSShorthandPropertyAPIPadding.h", + "$blink_core_output_dir/css/properties/CSSShorthandPropertyAPIScrollPadding.h", "$blink_core_output_dir/css/properties/CSSShorthandPropertyAPIScrollPaddingBlock.h", "$blink_core_output_dir/css/properties/CSSShorthandPropertyAPIScrollPaddingInline.h", + "$blink_core_output_dir/css/properties/CSSShorthandPropertyAPIScrollSnapMargin.h", "$blink_core_output_dir/css/properties/CSSShorthandPropertyAPIScrollSnapMarginBlock.h", "$blink_core_output_dir/css/properties/CSSShorthandPropertyAPIScrollSnapMarginInline.h", "$blink_core_output_dir/css/properties/CSSShorthandPropertyAPITextDecoration.h",
diff --git a/third_party/WebKit/Source/core/css/BUILD.gn b/third_party/WebKit/Source/core/css/BUILD.gn index 6798fb1..b6c47555 100644 --- a/third_party/WebKit/Source/core/css/BUILD.gn +++ b/third_party/WebKit/Source/core/css/BUILD.gn
@@ -380,6 +380,7 @@ "properties/CSSPropertyAPIAnimationIterationCount.cpp", "properties/CSSPropertyAPIAnimationName.cpp", "properties/CSSPropertyAPIAnimationPlayState.cpp", + "properties/CSSPropertyAPIAutoOrString.cpp", "properties/CSSPropertyAPIBaselineShift.cpp", "properties/CSSPropertyAPIBorderColor.cpp", "properties/CSSPropertyAPIBorderImageOutset.cpp", @@ -543,9 +544,12 @@ "properties/CSSPropertyWebkitBorderWidthUtils.h", "properties/CSSShorthandPropertyAPIAnimation.cpp", "properties/CSSShorthandPropertyAPIBackgroundPosition.cpp", + "properties/CSSShorthandPropertyAPIBorderColor.cpp", "properties/CSSShorthandPropertyAPIBorderImage.cpp", "properties/CSSShorthandPropertyAPIBorderRadius.cpp", "properties/CSSShorthandPropertyAPIBorderSpacing.cpp", + "properties/CSSShorthandPropertyAPIBorderStyle.cpp", + "properties/CSSShorthandPropertyAPIBorderWidth.cpp", "properties/CSSShorthandPropertyAPIColumns.cpp", "properties/CSSShorthandPropertyAPIFlex.cpp", "properties/CSSShorthandPropertyAPIFont.cpp", @@ -554,8 +558,11 @@ "properties/CSSShorthandPropertyAPIOffset.cpp", "properties/CSSShorthandPropertyAPIOutline.cpp", "properties/CSSShorthandPropertyAPIOverflow.cpp", + "properties/CSSShorthandPropertyAPIPadding.cpp", + "properties/CSSShorthandPropertyAPIScrollPadding.cpp", "properties/CSSShorthandPropertyAPIScrollPaddingBlock.cpp", "properties/CSSShorthandPropertyAPIScrollPaddingInline.cpp", + "properties/CSSShorthandPropertyAPIScrollSnapMargin.cpp", "properties/CSSShorthandPropertyAPIScrollSnapMarginBlock.cpp", "properties/CSSShorthandPropertyAPIScrollSnapMarginInline.cpp", "properties/CSSShorthandPropertyAPITextDecoration.cpp",
diff --git a/third_party/WebKit/Source/core/css/CSSPageRule.cpp b/third_party/WebKit/Source/core/css/CSSPageRule.cpp index 4bcb3cf..fd856c20 100644 --- a/third_party/WebKit/Source/core/css/CSSPageRule.cpp +++ b/third_party/WebKit/Source/core/css/CSSPageRule.cpp
@@ -58,7 +58,7 @@ CSSParserContext* context = CSSParserContext::Create(ParserContext(), nullptr); CSSSelectorList selector_list = CSSParser::ParsePageSelector( - context, parentStyleSheet() ? parentStyleSheet()->Contents() : nullptr, + *context, parentStyleSheet() ? parentStyleSheet()->Contents() : nullptr, selector_text); if (!selector_list.IsValid()) return;
diff --git a/third_party/WebKit/Source/core/css/CSSProperties.json5 b/third_party/WebKit/Source/core/css/CSSProperties.json5 index d6a1634e..8ca3dd5d 100644 --- a/third_party/WebKit/Source/core/css/CSSProperties.json5 +++ b/third_party/WebKit/Source/core/css/CSSProperties.json5
@@ -559,6 +559,8 @@ }, { name: "-webkit-locale", + api_class: "CSSPropertyAPIAutoOrString", + api_methods: ["parseSingleValue"], custom_value: true, font: true, inherited: true, @@ -2552,7 +2554,6 @@ name: "text-decoration-line", api_class: "CSSPropertyAPITextDecorationLine", api_methods: ["parseSingleValue"], - api_class: "CSSPropertyAPITextDecorationLine", converter: "ConvertFlags<TextDecoration>", name_for_methods: "TextDecoration", runtime_flag: "CSS3TextDecorations", @@ -3053,6 +3054,8 @@ }, { name: "-webkit-hyphenate-character", + api_class: "CSSPropertyAPIAutoOrString", + api_methods: ["parseSingleValue"], converter: "ConvertString<CSSValueAuto>", inherited: true, name_for_methods: "HyphenationString", @@ -3630,7 +3633,6 @@ name: "-webkit-text-decorations-in-effect", api_class: "CSSPropertyAPITextDecorationLine", api_methods: ["parseSingleValue"], - api_class: "CSSPropertyAPITextDecorationLine", builder_skip: true, inherited: true, }, @@ -3707,6 +3709,8 @@ { name: "border-color", longhands: ["border-top-color", "border-right-color", "border-bottom-color", "border-left-color"], + api_class: true, + api_methods: ["parseShorthand"], }, { name: "border-image", @@ -3737,6 +3741,8 @@ { name: "border-style", longhands: ["border-top-style", "border-right-style", "border-bottom-style", "border-left-style"], + api_class: true, + api_methods: ["parseShorthand"], keywords: ["none"], typedom_types: ["Image"], }, @@ -3747,6 +3753,8 @@ { name: "border-width", longhands: ["border-top-width", "border-right-width", "border-bottom-width", "border-left-width"], + api_class: true, + api_methods: ["parseShorthand"], }, { name: "flex", @@ -3856,6 +3864,8 @@ { name: "padding", longhands: ["padding-top", "padding-right", "padding-bottom", "padding-left"], + api_class: true, + api_methods: ["parseShorthand"], }, { name: "page-break-after", @@ -3872,6 +3882,8 @@ { name: "scroll-padding", longhands: ["scroll-padding-top", "scroll-padding-right", "scroll-padding-bottom", "scroll-padding-left"], + api_class: true, + api_methods: ["parseShorthand"], runtime_flag: "CSSScrollSnapPoints", }, { @@ -3891,6 +3903,8 @@ { name: "scroll-snap-margin", longhands: ["scroll-snap-margin-top", "scroll-snap-margin-right", "scroll-snap-margin-bottom", "scroll-snap-margin-left"], + api_class: true, + api_methods: ["parseShorthand"], runtime_flag: "CSSScrollSnapPoints", }, {
diff --git a/third_party/WebKit/Source/core/css/CSSSelector.cpp b/third_party/WebKit/Source/core/css/CSSSelector.cpp index bf28303..3ed4ad2 100644 --- a/third_party/WebKit/Source/core/css/CSSSelector.cpp +++ b/third_party/WebKit/Source/core/css/CSSSelector.cpp
@@ -31,6 +31,7 @@ #include "core/HTMLNames.h" #include "core/css/CSSMarkup.h" #include "core/css/CSSSelectorList.h" +#include "core/css/parser/CSSParserContext.h" #include "platform/wtf/Assertions.h" #include "platform/wtf/HashMap.h" #include "platform/wtf/StdLibExtras.h" @@ -495,6 +496,7 @@ } void CSSSelector::UpdatePseudoType(const AtomicString& value, + const CSSParserContext& context, bool has_arguments, CSSParserMode mode) { DCHECK(match_ == kPseudoClass || match_ == kPseudoElement); @@ -525,11 +527,14 @@ case kPseudoSelection: case kPseudoWebKitCustomElement: case kPseudoContent: - case kPseudoShadow: case kPseudoSlotted: if (match_ != kPseudoElement) pseudo_type_ = kPseudoUnknown; break; + case kPseudoShadow: + if (match_ != kPseudoElement || context.IsDynamicProfile()) + pseudo_type_ = kPseudoUnknown; + break; case kPseudoBlinkInternalElement: if (match_ != kPseudoElement || mode != kUASheetMode) pseudo_type_ = kPseudoUnknown; @@ -796,6 +801,7 @@ result = " > " + builder.ToString() + result; break; case kShadowDeep: + case kShadowDeepAsDescendant: result = " /deep/ " + builder.ToString() + result; break; case kShadowPiercingDescendant:
diff --git a/third_party/WebKit/Source/core/css/CSSSelector.h b/third_party/WebKit/Source/core/css/CSSSelector.h index 319b859..f959904 100644 --- a/third_party/WebKit/Source/core/css/CSSSelector.h +++ b/third_party/WebKit/Source/core/css/CSSSelector.h
@@ -25,6 +25,7 @@ #include <memory> #include "core/CoreExport.h" +#include "core/css/parser/CSSParserContext.h" #include "core/css/parser/CSSParserMode.h" #include "core/dom/QualifiedName.h" #include "core/style/ComputedStyleConstants.h" @@ -133,6 +134,7 @@ // Special cases for shadow DOM related selectors. kShadowPiercingDescendant, // >>> combinator kShadowDeep, // /deep/ combinator + kShadowDeepAsDescendant, // /deep/ as an alias for descendant kShadowPseudo, // ::shadow pseudo element kShadowSlot // ::slotted() pseudo element }; @@ -239,7 +241,11 @@ PseudoType GetPseudoType() const { return static_cast<PseudoType>(pseudo_type_); } - void UpdatePseudoType(const AtomicString&, bool has_arguments, CSSParserMode); + + void UpdatePseudoType(const AtomicString&, + const CSSParserContext&, + bool has_arguments, + CSSParserMode); void UpdatePseudoPage(const AtomicString&); static PseudoType ParsePseudoType(const AtomicString&, bool has_arguments);
diff --git a/third_party/WebKit/Source/core/css/SelectorChecker.cpp b/third_party/WebKit/Source/core/css/SelectorChecker.cpp index f036cbf..1fcc5f1 100644 --- a/third_party/WebKit/Source/core/css/SelectorChecker.cpp +++ b/third_party/WebKit/Source/core/css/SelectorChecker.cpp
@@ -339,6 +339,10 @@ next_context.pseudo_id = kPseudoIdNone; switch (relation) { + case CSSSelector::kShadowDeepAsDescendant: + Deprecation::CountDeprecation(context.element->GetDocument(), + WebFeature::kCSSDeepCombinator); + // fall through case CSSSelector::kDescendant: if (context.selector->RelationIsAffectedByPseudoContent()) { for (Element* element = context.element; element; @@ -416,11 +420,6 @@ return kSelectorFailsAllSiblings; case CSSSelector::kShadowPseudo: { - if (!is_ua_rule_ && mode_ != kQueryingRules && - context.selector->GetPseudoType() == CSSSelector::kPseudoShadow) { - Deprecation::CountDeprecation(context.element->GetDocument(), - WebFeature::kCSSSelectorPseudoShadow); - } // If we're in the same tree-scope as the scoping element, then following // a shadow descendant combinator would escape that and thus the scope. if (context.scope && context.scope->OwnerShadowHost() &&
diff --git a/third_party/WebKit/Source/core/css/SelectorFilter.cpp b/third_party/WebKit/Source/core/css/SelectorFilter.cpp index 4b50414..2f3acfa3 100644 --- a/third_party/WebKit/Source/core/css/SelectorFilter.cpp +++ b/third_party/WebKit/Source/core/css/SelectorFilter.cpp
@@ -177,6 +177,7 @@ *identifier_hashes = 0; return; case CSSSelector::kDescendant: + case CSSSelector::kShadowDeepAsDescendant: case CSSSelector::kChild: // Fall through. case CSSSelector::kShadowPseudo:
diff --git a/third_party/WebKit/Source/core/css/cssom/CSSPositionValue.cpp b/third_party/WebKit/Source/core/css/cssom/CSSPositionValue.cpp index 0967d36..ca273c0 100644 --- a/third_party/WebKit/Source/core/css/cssom/CSSPositionValue.cpp +++ b/third_party/WebKit/Source/core/css/cssom/CSSPositionValue.cpp
@@ -4,11 +4,52 @@ #include "core/css/cssom/CSSPositionValue.h" +#include "bindings/core/v8/ExceptionState.h" #include "core/css/CSSValuePair.h" #include "core/css/cssom/CSSNumericValue.h" namespace blink { +CSSPositionValue* CSSPositionValue::Create(CSSNumericValue* x, + CSSNumericValue* y, + ExceptionState& exception_state) { + if (x->GetType() != CSSStyleValue::StyleValueType::kLengthType && + x->GetType() != CSSStyleValue::StyleValueType::kPercentType) { + exception_state.ThrowTypeError( + "Must pass length or percentage to x in CSSPositionValue"); + return nullptr; + } + if (y->GetType() != CSSStyleValue::StyleValueType::kLengthType && + y->GetType() != CSSStyleValue::StyleValueType::kPercentType) { + exception_state.ThrowTypeError( + "Must pass length or percentage to y in CSSPositionValue"); + return nullptr; + } + return new CSSPositionValue(x, y); +} + +void CSSPositionValue::setX(CSSNumericValue* x, + ExceptionState& exception_state) { + if (x->GetType() != CSSStyleValue::StyleValueType::kLengthType && + x->GetType() != CSSStyleValue::StyleValueType::kPercentType) { + exception_state.ThrowTypeError( + "Must pass length or percentage to x in CSSPositionValue"); + return; + } + x_ = x; +} + +void CSSPositionValue::setY(CSSNumericValue* y, + ExceptionState& exception_state) { + if (y->GetType() != CSSStyleValue::StyleValueType::kLengthType && + y->GetType() != CSSStyleValue::StyleValueType::kPercentType) { + exception_state.ThrowTypeError( + "Must pass length or percentage to y in CSSPositionValue"); + return; + } + y_ = y; +} + CSSValue* CSSPositionValue::ToCSSValue() const { return CSSValuePair::Create(x_->ToCSSValue(), y_->ToCSSValue(), CSSValuePair::kKeepIdenticalValues);
diff --git a/third_party/WebKit/Source/core/css/cssom/CSSPositionValue.h b/third_party/WebKit/Source/core/css/cssom/CSSPositionValue.h index 693dd53..59c12d83 100644 --- a/third_party/WebKit/Source/core/css/cssom/CSSPositionValue.h +++ b/third_party/WebKit/Source/core/css/cssom/CSSPositionValue.h
@@ -12,24 +12,28 @@ namespace blink { class CSSNumericValue; +class ExceptionState; class CORE_EXPORT CSSPositionValue final : public CSSStyleValue { WTF_MAKE_NONCOPYABLE(CSSPositionValue); DEFINE_WRAPPERTYPEINFO(); public: - static CSSPositionValue* Create(const CSSNumericValue* x, - const CSSNumericValue* y) { - return new CSSPositionValue(x, y); - } + // Constructor defined in the IDL. + static CSSPositionValue* Create(CSSNumericValue* x, + CSSNumericValue* y, + ExceptionState&); - // Bindings require a non const return value. - CSSNumericValue* x() const { return const_cast<CSSNumericValue*>(x_.Get()); } - CSSNumericValue* y() const { return const_cast<CSSNumericValue*>(y_.Get()); } + // Getters and setters defined in the IDL. + CSSNumericValue* x() { return x_.Get(); } + CSSNumericValue* y() { return y_.Get(); } + void setX(CSSNumericValue* x, ExceptionState&); + void setY(CSSNumericValue* x, ExceptionState&); - StyleValueType GetType() const override { return kPositionType; } + // Internal methods - from CSSStyleValue. + StyleValueType GetType() const final { return kPositionType; } - CSSValue* ToCSSValue() const override; + CSSValue* ToCSSValue() const final; DEFINE_INLINE_VIRTUAL_TRACE() { visitor->Trace(x_); @@ -38,11 +42,10 @@ } protected: - CSSPositionValue(const CSSNumericValue* x, const CSSNumericValue* y) - : x_(x), y_(y) {} + CSSPositionValue(CSSNumericValue* x, CSSNumericValue* y) : x_(x), y_(y) {} - Member<const CSSNumericValue> x_; - Member<const CSSNumericValue> y_; + Member<CSSNumericValue> x_; + Member<CSSNumericValue> y_; }; } // namespace blink
diff --git a/third_party/WebKit/Source/core/css/cssom/CSSPositionValue.idl b/third_party/WebKit/Source/core/css/cssom/CSSPositionValue.idl index 405f8ed..d0cac56 100644 --- a/third_party/WebKit/Source/core/css/cssom/CSSPositionValue.idl +++ b/third_party/WebKit/Source/core/css/cssom/CSSPositionValue.idl
@@ -6,9 +6,10 @@ // background-position. // Spec: https://drafts.css-houdini.org/css-typed-om/#positionvalue-objects [ - Constructor(CSSNumericValue x, CSSNumericValue y), - Exposed(Window CSSTypedOM, PaintWorklet CSSPaintAPI) + Constructor(CSSNumericValue x, CSSNumericValue y), + Exposed(Window CSSTypedOM, PaintWorklet CSSPaintAPI), + RaisesException=Constructor ] interface CSSPositionValue : CSSStyleValue { - readonly attribute CSSNumericValue x; - readonly attribute CSSNumericValue y; + [RaisesException=Setter] attribute CSSNumericValue x; + [RaisesException=Setter] attribute CSSNumericValue y; };
diff --git a/third_party/WebKit/Source/core/css/parser/CSSParser.cpp b/third_party/WebKit/Source/core/css/parser/CSSParser.cpp index b6fcb75..0e15635 100644 --- a/third_party/WebKit/Source/core/css/parser/CSSParser.cpp +++ b/third_party/WebKit/Source/core/css/parser/CSSParser.cpp
@@ -4,6 +4,7 @@ #include "core/css/parser/CSSParser.h" +#include <memory> #include "core/css/CSSColorValue.h" #include "core/css/CSSKeyframeRule.h" #include "core/css/StyleColor.h" @@ -17,7 +18,6 @@ #include "core/css/parser/CSSTokenizer.h" #include "core/css/parser/CSSVariableParser.h" #include "core/layout/LayoutTheme.h" -#include <memory> namespace blink { @@ -48,11 +48,11 @@ } CSSSelectorList CSSParser::ParsePageSelector( - const CSSParserContext* context, + const CSSParserContext& context, StyleSheetContents* style_sheet_contents, const String& selector) { CSSTokenizer tokenizer(selector); - return CSSParserImpl::ParsePageSelector(tokenizer.TokenRange(), + return CSSParserImpl::ParsePageSelector(context, tokenizer.TokenRange(), style_sheet_contents); }
diff --git a/third_party/WebKit/Source/core/css/parser/CSSParser.h b/third_party/WebKit/Source/core/css/parser/CSSParser.h index 0111aacf..19444258 100644 --- a/third_party/WebKit/Source/core/css/parser/CSSParser.h +++ b/third_party/WebKit/Source/core/css/parser/CSSParser.h
@@ -5,11 +5,11 @@ #ifndef CSSParser_h #define CSSParser_h +#include <memory> #include "core/CSSPropertyNames.h" #include "core/CoreExport.h" #include "core/css/StylePropertySet.h" #include "core/css/parser/CSSParserContext.h" -#include <memory> namespace blink { @@ -40,7 +40,7 @@ static CSSSelectorList ParseSelector(const CSSParserContext*, StyleSheetContents*, const String&); - static CSSSelectorList ParsePageSelector(const CSSParserContext*, + static CSSSelectorList ParsePageSelector(const CSSParserContext&, StyleSheetContents*, const String&); static bool ParseDeclarationList(const CSSParserContext*,
diff --git a/third_party/WebKit/Source/core/css/parser/CSSParserImpl.cpp b/third_party/WebKit/Source/core/css/parser/CSSParserImpl.cpp index 73ac4128..94249e4 100644 --- a/third_party/WebKit/Source/core/css/parser/CSSParserImpl.cpp +++ b/third_party/WebKit/Source/core/css/parser/CSSParserImpl.cpp
@@ -254,6 +254,7 @@ } CSSSelectorList CSSParserImpl::ParsePageSelector( + const CSSParserContext& context, CSSParserTokenRange range, StyleSheetContents* style_sheet) { // We only support a small subset of the css-page spec. @@ -734,7 +735,8 @@ StyleRulePage* CSSParserImpl::ConsumePageRule(CSSParserTokenRange prelude, CSSParserTokenRange block) { - CSSSelectorList selector_list = ParsePageSelector(prelude, style_sheet_); + CSSSelectorList selector_list = + ParsePageSelector(*context_, prelude, style_sheet_); if (!selector_list.IsValid()) return nullptr; // Parse error, invalid @page selector
diff --git a/third_party/WebKit/Source/core/css/parser/CSSParserImpl.h b/third_party/WebKit/Source/core/css/parser/CSSParserImpl.h index df169ad..ca11f336 100644 --- a/third_party/WebKit/Source/core/css/parser/CSSParserImpl.h +++ b/third_party/WebKit/Source/core/css/parser/CSSParserImpl.h
@@ -84,7 +84,8 @@ const CSSParserContext*, StyleSheetContents*, bool defer_property_parsing = false); - static CSSSelectorList ParsePageSelector(CSSParserTokenRange, + static CSSSelectorList ParsePageSelector(const CSSParserContext&, + CSSParserTokenRange, StyleSheetContents*); static ImmutableStylePropertySet* ParseCustomPropertySet(CSSParserTokenRange);
diff --git a/third_party/WebKit/Source/core/css/parser/CSSParserSelector.h b/third_party/WebKit/Source/core/css/parser/CSSParserSelector.h index 9aab817..976cc47 100644 --- a/third_party/WebKit/Source/core/css/parser/CSSParserSelector.h +++ b/third_party/WebKit/Source/core/css/parser/CSSParserSelector.h
@@ -29,6 +29,8 @@ namespace blink { +class CSSParserContext; + class CORE_EXPORT CSSParserSelector { WTF_MAKE_NONCOPYABLE(CSSParserSelector); USING_FAST_MALLOC(CSSParserSelector); @@ -74,9 +76,10 @@ } void UpdatePseudoType(const AtomicString& value, + const CSSParserContext& context, bool has_arguments, CSSParserMode mode) const { - selector_->UpdatePseudoType(value, has_arguments, mode); + selector_->UpdatePseudoType(value, context, has_arguments, mode); } void UpdatePseudoPage(const AtomicString& value) { selector_->UpdatePseudoPage(value);
diff --git a/third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp b/third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp index b83c452..f214cfa 100644 --- a/third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp +++ b/third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp
@@ -304,12 +304,6 @@ return nullptr; } -static CSSValue* ConsumeLocale(CSSParserTokenRange& range) { - if (range.Peek().Id() == CSSValueAuto) - return ConsumeIdent(range); - return ConsumeString(range); -} - static CSSFunctionValue* ConsumeFilterFunction( CSSParserTokenRange& range, const CSSParserContext* context) { @@ -1003,9 +997,6 @@ case CSSPropertyWebkitLogicalWidth: case CSSPropertyWebkitLogicalHeight: return CSSPropertyLengthUtils::ConsumeWidthOrHeight(range_, *context_); - case CSSPropertyWebkitHyphenateCharacter: - case CSSPropertyWebkitLocale: - return ConsumeLocale(range_); case CSSPropertyAnimationDelay: case CSSPropertyTransitionDelay: return ConsumeCommaSeparatedList(ConsumeTime, range_, kValueRangeAll); @@ -1447,38 +1438,6 @@ return range_.AtEnd(); } -bool CSSPropertyParser::Consume4Values(const StylePropertyShorthand& shorthand, - bool important) { - DCHECK_EQ(shorthand.length(), 4u); - const CSSPropertyID* longhands = shorthand.properties(); - const CSSValue* top = ParseSingleValue(longhands[0], shorthand.id()); - if (!top) - return false; - - const CSSValue* right = ParseSingleValue(longhands[1], shorthand.id()); - const CSSValue* bottom = nullptr; - const CSSValue* left = nullptr; - if (right) { - bottom = ParseSingleValue(longhands[2], shorthand.id()); - if (bottom) - left = ParseSingleValue(longhands[3], shorthand.id()); - } - - if (!right) - right = top; - if (!bottom) - bottom = top; - if (!left) - left = right; - - AddParsedProperty(longhands[0], shorthand.id(), *top, important); - AddParsedProperty(longhands[1], shorthand.id(), *right, important); - AddParsedProperty(longhands[2], shorthand.id(), *bottom, important); - AddParsedProperty(longhands[3], shorthand.id(), *left, important); - - return range_.AtEnd(); -} - static inline CSSValueID MapFromPageBreakBetween(CSSValueID value) { if (value == CSSValueAlways) return CSSValuePage; @@ -2111,8 +2070,6 @@ case CSSPropertyTextDecoration: DCHECK(RuntimeEnabledFeatures::CSS3TextDecorationsEnabled()); return ConsumeShorthandGreedily(textDecorationShorthand(), important); - case CSSPropertyPadding: - return Consume4Values(paddingShorthand(), important); case CSSPropertyMarker: { const CSSValue* marker = ParseSingleValue(CSSPropertyMarkerStart); if (!marker || !range_.AtEnd()) @@ -2131,12 +2088,6 @@ return ConsumeShorthandGreedily(columnRuleShorthand(), important); case CSSPropertyListStyle: return ConsumeShorthandGreedily(listStyleShorthand(), important); - case CSSPropertyBorderColor: - return Consume4Values(borderColorShorthand(), important); - case CSSPropertyBorderStyle: - return Consume4Values(borderStyleShorthand(), important); - case CSSPropertyBorderWidth: - return Consume4Values(borderWidthShorthand(), important); case CSSPropertyBorderTop: return ConsumeShorthandGreedily(borderTopShorthand(), important); case CSSPropertyBorderRight: @@ -2208,10 +2159,6 @@ return ConsumePlaceItemsShorthand(important); case CSSPropertyPlaceSelf: return ConsumePlaceSelfShorthand(important); - case CSSPropertyScrollPadding: - return Consume4Values(scrollPaddingShorthand(), important); - case CSSPropertyScrollSnapMargin: - return Consume4Values(scrollSnapMarginShorthand(), important); case CSSPropertyScrollBoundaryBehavior: return Consume2Values(scrollBoundaryBehaviorShorthand(), important); default:
diff --git a/third_party/WebKit/Source/core/css/parser/CSSPropertyParser.h b/third_party/WebKit/Source/core/css/parser/CSSPropertyParser.h index 6c96729f..6f3ef0a 100644 --- a/third_party/WebKit/Source/core/css/parser/CSSPropertyParser.h +++ b/third_party/WebKit/Source/core/css/parser/CSSPropertyParser.h
@@ -86,7 +86,6 @@ bool ParseShorthand(CSSPropertyID, bool important); bool ConsumeShorthandGreedily(const StylePropertyShorthand&, bool important); bool Consume2Values(const StylePropertyShorthand&, bool important); - bool Consume4Values(const StylePropertyShorthand&, bool important); bool ConsumeBackgroundShorthand(const StylePropertyShorthand&, bool important);
diff --git a/third_party/WebKit/Source/core/css/parser/CSSPropertyParserHelpers.h b/third_party/WebKit/Source/core/css/parser/CSSPropertyParserHelpers.h index 5e0b998..040ccc2b 100644 --- a/third_party/WebKit/Source/core/css/parser/CSSPropertyParserHelpers.h +++ b/third_party/WebKit/Source/core/css/parser/CSSPropertyParserHelpers.h
@@ -131,14 +131,13 @@ CSSParserTokenRange&, bool& needs_legacy_parsing); -// ConsumeShorthandVia2LonghandAPIs, ConsumeShorthandVia4LonghandAPIs and -// ConsumeShorthandGreedilyViaLonghandAPIs are based on CSSPropertyParsers' -// Consume2Values, Consume4Values and ConsumeShorthandGreedily. +// ConsumeShorthandVia2LonghandAPIs and ConsumeShorthandGreedilyViaLonghandAPIs +// are based on CSSPropertyParsers' Consume2Values and ConsumeShorthandGreedily. // They all delegate parsing of a shorthand property to its respective longhand // components. The difference is the functions in this Helpers file expect // component longhands to have API implementations already because each // shorthand will call its component longhand APIs' parseShorthand method. -// Consume4Values and ConsumeShorthandGreedily will be removed soon, when +// Consume2Values and ConsumeShorthandGreedily will be removed soon, when // shorthand properties are ribbonised (i.e. have their own APIs). Until then, // there is a slight code duplication between the two versions for the following // reasons:
diff --git a/third_party/WebKit/Source/core/css/parser/CSSSelectorParser.cpp b/third_party/WebKit/Source/core/css/parser/CSSSelectorParser.cpp index 19174f62..b96b3eda 100644 --- a/third_party/WebKit/Source/core/css/parser/CSSSelectorParser.cpp +++ b/third_party/WebKit/Source/core/css/parser/CSSSelectorParser.cpp
@@ -442,7 +442,7 @@ AtomicString value = token.Value().ToAtomicString().LowerASCII(); bool has_arguments = token.GetType() == kFunctionToken; - selector->UpdatePseudoType(value, has_arguments, context_->Mode()); + selector->UpdatePseudoType(value, *context_, has_arguments, context_->Mode()); if (!RuntimeEnabledFeatures::CSSSelectorsFocusWithinEnabled() && selector->GetPseudoType() == CSSSelector::kPseudoFocusWithin) @@ -582,7 +582,8 @@ const CSSParserToken& slash = range.ConsumeIncludingWhitespace(); if (slash.GetType() != kDelimiterToken || slash.Delimiter() != '/') failed_parsing_ = true; - return CSSSelector::kShadowDeep; + return context_->IsDynamicProfile() ? CSSSelector::kShadowDeepAsDescendant + : CSSSelector::kShadowDeep; } default:
diff --git a/third_party/WebKit/Source/core/css/properties/CSSPropertyAPIAutoOrString.cpp b/third_party/WebKit/Source/core/css/properties/CSSPropertyAPIAutoOrString.cpp new file mode 100644 index 0000000..9480544 --- /dev/null +++ b/third_party/WebKit/Source/core/css/properties/CSSPropertyAPIAutoOrString.cpp
@@ -0,0 +1,22 @@ +// 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/css/properties/CSSPropertyAPIAutoOrString.h" + +#include "core/css/CSSStringValue.h" +#include "core/css/parser/CSSPropertyParserHelpers.h" + +namespace blink { +class CSSParserLocalContext; + +const CSSValue* CSSPropertyAPIAutoOrString::parseSingleValue( + CSSParserTokenRange& range, + const CSSParserContext&, + const CSSParserLocalContext&) { + if (range.Peek().Id() == CSSValueAuto) + return CSSPropertyParserHelpers::ConsumeIdent(range); + return CSSPropertyParserHelpers::ConsumeString(range); +} + +} // namespace blink
diff --git a/third_party/WebKit/Source/core/css/properties/CSSShorthandPropertyAPIBorderColor.cpp b/third_party/WebKit/Source/core/css/properties/CSSShorthandPropertyAPIBorderColor.cpp new file mode 100644 index 0000000..d8dc28b --- /dev/null +++ b/third_party/WebKit/Source/core/css/properties/CSSShorthandPropertyAPIBorderColor.cpp
@@ -0,0 +1,22 @@ +// 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/css/properties/CSSShorthandPropertyAPIBorderColor.h" + +#include "core/StylePropertyShorthand.h" +#include "core/css/parser/CSSPropertyParserHelpers.h" + +namespace blink { + +bool CSSShorthandPropertyAPIBorderColor::parseShorthand( + bool important, + CSSParserTokenRange& range, + const CSSParserContext& context, + bool, + HeapVector<CSSProperty, 256>& properties) { + return CSSPropertyParserHelpers::ConsumeShorthandVia4LonghandAPIs( + borderColorShorthand(), important, context, range, properties); +} + +} // namespace blink
diff --git a/third_party/WebKit/Source/core/css/properties/CSSShorthandPropertyAPIBorderStyle.cpp b/third_party/WebKit/Source/core/css/properties/CSSShorthandPropertyAPIBorderStyle.cpp new file mode 100644 index 0000000..cce2e6f --- /dev/null +++ b/third_party/WebKit/Source/core/css/properties/CSSShorthandPropertyAPIBorderStyle.cpp
@@ -0,0 +1,22 @@ +// 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/css/properties/CSSShorthandPropertyAPIBorderStyle.h" + +#include "core/StylePropertyShorthand.h" +#include "core/css/parser/CSSPropertyParserHelpers.h" + +namespace blink { + +bool CSSShorthandPropertyAPIBorderStyle::parseShorthand( + bool important, + CSSParserTokenRange& range, + const CSSParserContext& context, + bool, + HeapVector<CSSProperty, 256>& properties) { + return CSSPropertyParserHelpers::ConsumeShorthandVia4LonghandAPIs( + borderStyleShorthand(), important, context, range, properties); +} + +} // namespace blink
diff --git a/third_party/WebKit/Source/core/css/properties/CSSShorthandPropertyAPIBorderWidth.cpp b/third_party/WebKit/Source/core/css/properties/CSSShorthandPropertyAPIBorderWidth.cpp new file mode 100644 index 0000000..7a5e594 --- /dev/null +++ b/third_party/WebKit/Source/core/css/properties/CSSShorthandPropertyAPIBorderWidth.cpp
@@ -0,0 +1,22 @@ +// 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/css/properties/CSSShorthandPropertyAPIBorderWidth.h" + +#include "core/StylePropertyShorthand.h" +#include "core/css/parser/CSSPropertyParserHelpers.h" + +namespace blink { + +bool CSSShorthandPropertyAPIBorderWidth::parseShorthand( + bool important, + CSSParserTokenRange& range, + const CSSParserContext& context, + bool, + HeapVector<CSSProperty, 256>& properties) { + return CSSPropertyParserHelpers::ConsumeShorthandVia4LonghandAPIs( + borderWidthShorthand(), important, context, range, properties); +} + +} // namespace blink
diff --git a/third_party/WebKit/Source/core/css/properties/CSSShorthandPropertyAPIPadding.cpp b/third_party/WebKit/Source/core/css/properties/CSSShorthandPropertyAPIPadding.cpp new file mode 100644 index 0000000..3d85209 --- /dev/null +++ b/third_party/WebKit/Source/core/css/properties/CSSShorthandPropertyAPIPadding.cpp
@@ -0,0 +1,22 @@ +// 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/css/properties/CSSShorthandPropertyAPIPadding.h" + +#include "core/StylePropertyShorthand.h" +#include "core/css/parser/CSSPropertyParserHelpers.h" + +namespace blink { + +bool CSSShorthandPropertyAPIPadding::parseShorthand( + bool important, + CSSParserTokenRange& range, + const CSSParserContext& context, + bool, + HeapVector<CSSProperty, 256>& properties) { + return CSSPropertyParserHelpers::ConsumeShorthandVia4LonghandAPIs( + paddingShorthand(), important, context, range, properties); +} + +} // namespace blink
diff --git a/third_party/WebKit/Source/core/css/properties/CSSShorthandPropertyAPIScrollPadding.cpp b/third_party/WebKit/Source/core/css/properties/CSSShorthandPropertyAPIScrollPadding.cpp new file mode 100644 index 0000000..8a67d5e --- /dev/null +++ b/third_party/WebKit/Source/core/css/properties/CSSShorthandPropertyAPIScrollPadding.cpp
@@ -0,0 +1,22 @@ +// 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/css/properties/CSSShorthandPropertyAPIScrollPadding.h" + +#include "core/StylePropertyShorthand.h" +#include "core/css/parser/CSSPropertyParserHelpers.h" + +namespace blink { + +bool CSSShorthandPropertyAPIScrollPadding::parseShorthand( + bool important, + CSSParserTokenRange& range, + const CSSParserContext& context, + bool, + HeapVector<CSSProperty, 256>& properties) { + return CSSPropertyParserHelpers::ConsumeShorthandVia4LonghandAPIs( + scrollPaddingShorthand(), important, context, range, properties); +} + +} // namespace blink
diff --git a/third_party/WebKit/Source/core/css/properties/CSSShorthandPropertyAPIScrollSnapMargin.cpp b/third_party/WebKit/Source/core/css/properties/CSSShorthandPropertyAPIScrollSnapMargin.cpp new file mode 100644 index 0000000..7af9637 --- /dev/null +++ b/third_party/WebKit/Source/core/css/properties/CSSShorthandPropertyAPIScrollSnapMargin.cpp
@@ -0,0 +1,22 @@ +// 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/css/properties/CSSShorthandPropertyAPIScrollSnapMargin.h" + +#include "core/StylePropertyShorthand.h" +#include "core/css/parser/CSSPropertyParserHelpers.h" + +namespace blink { + +bool CSSShorthandPropertyAPIScrollSnapMargin::parseShorthand( + bool important, + CSSParserTokenRange& range, + const CSSParserContext& context, + bool, + HeapVector<CSSProperty, 256>& properties) { + return CSSPropertyParserHelpers::ConsumeShorthandVia4LonghandAPIs( + scrollSnapMarginShorthand(), important, context, range, properties); +} + +} // namespace blink
diff --git a/third_party/WebKit/Source/core/dom/AccessibleNode.cpp b/third_party/WebKit/Source/core/dom/AccessibleNode.cpp index 18a4673..9823ee43 100644 --- a/third_party/WebKit/Source/core/dom/AccessibleNode.cpp +++ b/third_party/WebKit/Source/core/dom/AccessibleNode.cpp
@@ -24,6 +24,8 @@ return aria_checkedAttr; case AOMStringProperty::kCurrent: return aria_currentAttr; + case AOMStringProperty::kHasPopUp: + return aria_haspopupAttr; case AOMStringProperty::kInvalid: return aria_invalidAttr; case AOMStringProperty::kKeyShortcuts: @@ -685,6 +687,15 @@ NotifyAttributeChanged(aria_flowtoAttr); } +AtomicString AccessibleNode::hasPopUp() const { + return GetProperty(element_, AOMStringProperty::kHasPopUp); +} + +void AccessibleNode::setHasPopUp(const AtomicString& has_popup) { + SetStringProperty(AOMStringProperty::kHasPopUp, has_popup); + NotifyAttributeChanged(aria_haspopupAttr); +} + bool AccessibleNode::hidden(bool& is_null) const { return GetProperty(element_, AOMBooleanProperty::kHidden, is_null); }
diff --git a/third_party/WebKit/Source/core/dom/AccessibleNode.h b/third_party/WebKit/Source/core/dom/AccessibleNode.h index d7362df..4dce32f 100644 --- a/third_party/WebKit/Source/core/dom/AccessibleNode.h +++ b/third_party/WebKit/Source/core/dom/AccessibleNode.h
@@ -25,6 +25,7 @@ kAutocomplete, kChecked, kCurrent, + kHasPopUp, kInvalid, kKeyShortcuts, kLabel, @@ -228,6 +229,9 @@ AccessibleNodeList* flowTo() const; void setFlowTo(AccessibleNodeList*); + AtomicString hasPopUp() const; + void setHasPopUp(const AtomicString&); + bool hidden(bool& is_null) const; void setHidden(bool, bool is_null);
diff --git a/third_party/WebKit/Source/core/dom/AccessibleNode.idl b/third_party/WebKit/Source/core/dom/AccessibleNode.idl index 0c8356c..e9909ab 100644 --- a/third_party/WebKit/Source/core/dom/AccessibleNode.idl +++ b/third_party/WebKit/Source/core/dom/AccessibleNode.idl
@@ -24,6 +24,7 @@ attribute AccessibleNode? errorMessage; attribute boolean? expanded; attribute AccessibleNodeList? flowTo; + attribute DOMString? hasPopUp; attribute boolean? hidden; attribute DOMString? invalid; attribute DOMString? keyShortcuts;
diff --git a/third_party/WebKit/Source/core/dom/Element.cpp b/third_party/WebKit/Source/core/dom/Element.cpp index a75f292..6b4d879 100644 --- a/third_party/WebKit/Source/core/dom/Element.cpp +++ b/third_party/WebKit/Source/core/dom/Element.cpp
@@ -3621,12 +3621,13 @@ } void Element::SetUnsignedIntegralAttribute(const QualifiedName& attribute_name, - unsigned value) { + unsigned value, + unsigned default_value) { // Range restrictions are enforced for unsigned IDL attributes that // reflect content attributes, // http://www.whatwg.org/specs/web-apps/current-work/multipage/common-dom-interfaces.html#reflecting-content-attributes-in-idl-attributes if (value > 0x7fffffffu) - value = 0; + value = default_value; setAttribute(attribute_name, AtomicString::Number(value)); }
diff --git a/third_party/WebKit/Source/core/dom/Element.h b/third_party/WebKit/Source/core/dom/Element.h index 1395bda..1cdeaf8d 100644 --- a/third_party/WebKit/Source/core/dom/Element.h +++ b/third_party/WebKit/Source/core/dom/Element.h
@@ -156,7 +156,8 @@ int GetIntegralAttribute(const QualifiedName& attribute_name) const; void SetIntegralAttribute(const QualifiedName& attribute_name, int value); void SetUnsignedIntegralAttribute(const QualifiedName& attribute_name, - unsigned value); + unsigned value, + unsigned default_value = 0); double GetFloatingPointAttribute( const QualifiedName& attribute_name, double fallback_value = std::numeric_limits<double>::quiet_NaN()) const;
diff --git a/third_party/WebKit/Source/core/dom/StyleEngineTest.cpp b/third_party/WebKit/Source/core/dom/StyleEngineTest.cpp index da00b16..b44d24aa 100644 --- a/third_party/WebKit/Source/core/dom/StyleEngineTest.cpp +++ b/third_party/WebKit/Source/core/dom/StyleEngineTest.cpp
@@ -364,12 +364,6 @@ EXPECT_EQ(ScheduleInvalidationsForRules( *shadow_root, ".a ::content span { background: green}"), kRuleSetInvalidationFullRecalc); - EXPECT_EQ(ScheduleInvalidationsForRules( - *shadow_root, ".a /deep/ span { background: green}"), - kRuleSetInvalidationFullRecalc); - EXPECT_EQ(ScheduleInvalidationsForRules( - *shadow_root, ".a::shadow span { background: green}"), - kRuleSetInvalidationFullRecalc); } TEST_F(StyleEngineTest, HasViewportDependentMediaQueries) {
diff --git a/third_party/WebKit/Source/core/editing/InputMethodController.cpp b/third_party/WebKit/Source/core/editing/InputMethodController.cpp index dbd7f98..8ac57001 100644 --- a/third_party/WebKit/Source/core/editing/InputMethodController.cpp +++ b/third_party/WebKit/Source/core/editing/InputMethodController.cpp
@@ -34,6 +34,7 @@ #include "core/dom/Text.h" #include "core/editing/EditingUtilities.h" #include "core/editing/Editor.h" +#include "core/editing/FrameSelection.h" #include "core/editing/commands/TypingCommand.h" #include "core/editing/markers/DocumentMarkerController.h" #include "core/editing/state_machines/BackwardCodePointStateMachine.h" @@ -344,11 +345,24 @@ SelectionInDOMTree::Builder().SetBaseAndExtent(range).Build(), 0); } +bool IsCompositionTooLong(const Element& element) { + if (isHTMLInputElement(element)) + return toHTMLInputElement(element).TooLong(); + if (isHTMLTextAreaElement(element)) + return toHTMLTextAreaElement(element).TooLong(); + return false; +} + bool InputMethodController::FinishComposingText( ConfirmCompositionBehavior confirm_behavior) { if (!HasComposition()) return false; + // If text is longer than maxlength, give input/textarea's handler a chance to + // clamp the text by replacing the composition with the same value. + const bool is_too_long = + IsCompositionTooLong(*GetDocument().FocusedElement()); + const String& composing = ComposingText(); if (confirm_behavior == kKeepSelection) { @@ -359,8 +373,12 @@ const PlainTextRange& old_offsets = GetSelectionOffsets(); Editor::RevealSelectionScope reveal_selection_scope(&GetEditor()); - Clear(); - DispatchCompositionEndEvent(GetFrame(), composing); + if (is_too_long) { + ReplaceComposition(ComposingText()); + } else { + Clear(); + DispatchCompositionEndEvent(GetFrame(), composing); + } // TODO(editing-dev): Use of updateStyleAndLayoutIgnorePendingStylesheets // needs to be audited. see http://crbug.com/590369 for more details. @@ -392,7 +410,11 @@ if (composition_range.IsNull()) return false; - Clear(); + if (is_too_long) { + ReplaceComposition(ComposingText()); + } else { + Clear(); + } if (!MoveCaret(composition_range.End())) return false; @@ -688,7 +710,7 @@ GetDocument().UpdateStyleAndLayoutIgnorePendingStylesheets(); // We shouldn't close typing in the middle of setComposition. - SetEditableSelectionOffsets(selected_range, kNotUserTriggered); + SetEditableSelectionOffsets(selected_range, TypingContinuation::kContinue); if (underlines.IsEmpty()) { GetDocument().Markers().AddCompositionMarker( @@ -799,23 +821,37 @@ } bool InputMethodController::SetSelectionOffsets( + const PlainTextRange& selection_offsets) { + return SetSelectionOffsets(selection_offsets, TypingContinuation::kEnd); +} + +bool InputMethodController::SetSelectionOffsets( const PlainTextRange& selection_offsets, - FrameSelection::SetSelectionOptions options) { + TypingContinuation Typing_continuation) { const EphemeralRange range = EphemeralRangeForOffsets(selection_offsets); if (range.IsNull()) return false; GetFrame().Selection().SetSelection( - SelectionInDOMTree::Builder().SetBaseAndExtent(range).Build(), options); + SelectionInDOMTree::Builder().SetBaseAndExtent(range).Build(), + Typing_continuation == TypingContinuation::kEnd + ? FrameSelection::kCloseTyping + : 0); return true; } bool InputMethodController::SetEditableSelectionOffsets( + const PlainTextRange& selection_offsets) { + return SetEditableSelectionOffsets(selection_offsets, + TypingContinuation::kEnd); +} + +bool InputMethodController::SetEditableSelectionOffsets( const PlainTextRange& selection_offsets, - FrameSelection::SetSelectionOptions options) { + TypingContinuation typing_continuation) { if (!GetEditor().CanEdit()) return false; - return SetSelectionOffsets(selection_offsets, options); + return SetSelectionOffsets(selection_offsets, typing_continuation); } PlainTextRange InputMethodController::CreateRangeForSelection(
diff --git a/third_party/WebKit/Source/core/editing/InputMethodController.h b/third_party/WebKit/Source/core/editing/InputMethodController.h index ebb1fab1..65c7e0b 100644 --- a/third_party/WebKit/Source/core/editing/InputMethodController.h +++ b/third_party/WebKit/Source/core/editing/InputMethodController.h
@@ -30,7 +30,6 @@ #include "core/dom/SynchronousMutationObserver.h" #include "core/editing/CompositionUnderline.h" #include "core/editing/EphemeralRange.h" -#include "core/editing/FrameSelection.h" #include "core/editing/PlainTextRange.h" #include "platform/heap/Handle.h" #include "platform/wtf/Vector.h" @@ -42,6 +41,7 @@ class Editor; class LocalFrame; class Range; +enum class TypingContinuation; class CORE_EXPORT InputMethodController final : public GarbageCollectedFinalized<InputMethodController>, @@ -92,9 +92,7 @@ PlainTextRange GetSelectionOffsets() const; // Returns true if setting selection to specified offsets, otherwise false. - bool SetEditableSelectionOffsets( - const PlainTextRange&, - FrameSelection::SetSelectionOptions = FrameSelection::kCloseTyping); + bool SetEditableSelectionOffsets(const PlainTextRange&); void ExtendSelectionAndDelete(int before, int after); PlainTextRange CreateRangeForSelection(int start, int end, @@ -129,9 +127,7 @@ EphemeralRange EphemeralRangeForOffsets(const PlainTextRange&) const; // Returns true if selection offsets were successfully set. - bool SetSelectionOffsets( - const PlainTextRange&, - FrameSelection::SetSelectionOptions = FrameSelection::kCloseTyping); + bool SetSelectionOffsets(const PlainTextRange&); void AddCompositionUnderlines(const Vector<CompositionUnderline>& underlines, ContainerNode* base_element, @@ -164,6 +160,12 @@ // Implements |SynchronousMutationObserver|. void ContextDestroyed(Document*) final; + + // Returns true if setting selection to specified offsets, otherwise false. + bool SetEditableSelectionOffsets(const PlainTextRange&, TypingContinuation); + + // Returns true if selection offsets were successfully set. + bool SetSelectionOffsets(const PlainTextRange&, TypingContinuation); }; } // namespace blink
diff --git a/third_party/WebKit/Source/core/editing/InputMethodControllerTest.cpp b/third_party/WebKit/Source/core/editing/InputMethodControllerTest.cpp index c1b0175..4a08e3a2 100644 --- a/third_party/WebKit/Source/core/editing/InputMethodControllerTest.cpp +++ b/third_party/WebKit/Source/core/editing/InputMethodControllerTest.cpp
@@ -2287,4 +2287,18 @@ EXPECT_EQ(kWebTextInputTypeContentEditable, Controller().TextInputType()); } +// http://crbug.com/721666 +TEST_F(InputMethodControllerTest, MaxLength) { + HTMLInputElement* input = toHTMLInputElement( + InsertHTMLElement("<input id='a' maxlength='4'/>", "a")); + + EXPECT_EQ(kWebTextInputTypeText, Controller().TextInputType()); + + Controller().SetComposition("abcde", Vector<CompositionUnderline>(), 4, 4); + EXPECT_STREQ("abcde", input->value().Utf8().data()); + + Controller().FinishComposingText(InputMethodController::kKeepSelection); + EXPECT_STREQ("abcd", input->value().Utf8().data()); +} + } // namespace blink
diff --git a/third_party/WebKit/Source/core/editing/SelectionAdjuster.cpp b/third_party/WebKit/Source/core/editing/SelectionAdjuster.cpp index 3c85a2da..9d4d022b 100644 --- a/third_party/WebKit/Source/core/editing/SelectionAdjuster.cpp +++ b/third_party/WebKit/Source/core/editing/SelectionAdjuster.cpp
@@ -151,62 +151,58 @@ return Position(); } -} // namespace - -void SelectionAdjuster::AdjustSelectionToAvoidCrossingShadowBoundaries( - VisibleSelection* selection) { - // Note: |m_selectionType| isn't computed yet. - DCHECK(selection->Base().IsNotNull()); - DCHECK(selection->Extent().IsNotNull()); - DCHECK(selection->Start().IsNotNull()); - DCHECK(selection->End().IsNotNull()); - - // TODO(hajimehoshi): Checking treeScope is wrong when a node is - // distributed, but we leave it as it is for backward compatibility. - if (selection->Start().AnchorNode()->GetTreeScope() == - selection->End().AnchorNode()->GetTreeScope()) - return; - - if (selection->IsBaseFirst()) { - const Position& new_end = AdjustPositionForEnd( - selection->End(), selection->Start().ComputeContainerNode()); - selection->extent_ = new_end; - selection->end_ = new_end; - return; - } - - const Position& new_start = AdjustPositionForStart( - selection->Start(), selection->End().ComputeContainerNode()); - selection->extent_ = new_start; - selection->start_ = new_start; +// TODO(hajimehoshi): Checking treeScope is wrong when a node is +// distributed, but we leave it as it is for backward compatibility. +bool IsCrossingShadowBoundaries(const EphemeralRange& range) { + DCHECK(range.IsNotNull()); + return range.StartPosition().AnchorNode()->GetTreeScope() != + range.EndPosition().AnchorNode()->GetTreeScope(); } -// This function is called twice. The first is called when |m_start| and |m_end| -// or |m_extent| are same, and the second when |m_start| and |m_end| are changed -// after downstream/upstream. -void SelectionAdjuster::AdjustSelectionToAvoidCrossingShadowBoundaries( - VisibleSelectionInFlatTree* selection) { - Node* const shadow_host_start = - EnclosingShadowHostForStart(selection->Start()); - Node* const shadow_host_end = EnclosingShadowHostForEnd(selection->End()); - if (shadow_host_start == shadow_host_end) - return; +} // namespace - if (selection->IsBaseFirst()) { - Node* const shadow_host = - shadow_host_start ? shadow_host_start : shadow_host_end; - const PositionInFlatTree& new_end = - AdjustPositionInFlatTreeForEnd(selection->End(), shadow_host); - selection->extent_ = new_end; - selection->end_ = new_end; - return; - } +Position SelectionAdjuster::AdjustSelectionStartToAvoidCrossingShadowBoundaries( + const EphemeralRange& range) { + DCHECK(range.IsNotNull()); + if (!IsCrossingShadowBoundaries(range)) + return range.StartPosition(); + return AdjustPositionForStart(range.StartPosition(), + range.EndPosition().ComputeContainerNode()); +} + +Position SelectionAdjuster::AdjustSelectionEndToAvoidCrossingShadowBoundaries( + const EphemeralRange& range) { + DCHECK(range.IsNotNull()); + if (!IsCrossingShadowBoundaries(range)) + return range.EndPosition(); + return AdjustPositionForEnd(range.EndPosition(), + range.StartPosition().ComputeContainerNode()); +} + +PositionInFlatTree +SelectionAdjuster::AdjustSelectionStartToAvoidCrossingShadowBoundaries( + const EphemeralRangeInFlatTree& range) { + Node* const shadow_host_start = + EnclosingShadowHostForStart(range.StartPosition()); + Node* const shadow_host_end = EnclosingShadowHostForEnd(range.EndPosition()); + if (shadow_host_start == shadow_host_end) + return range.StartPosition(); Node* const shadow_host = shadow_host_end ? shadow_host_end : shadow_host_start; - const PositionInFlatTree& new_start = - AdjustPositionInFlatTreeForStart(selection->Start(), shadow_host); - selection->extent_ = new_start; - selection->start_ = new_start; + return AdjustPositionInFlatTreeForStart(range.StartPosition(), shadow_host); +} + +PositionInFlatTree +SelectionAdjuster::AdjustSelectionEndToAvoidCrossingShadowBoundaries( + const EphemeralRangeInFlatTree& range) { + Node* const shadow_host_start = + EnclosingShadowHostForStart(range.StartPosition()); + Node* const shadow_host_end = EnclosingShadowHostForEnd(range.EndPosition()); + if (shadow_host_start == shadow_host_end) + return range.EndPosition(); + Node* const shadow_host = + shadow_host_start ? shadow_host_start : shadow_host_end; + return AdjustPositionInFlatTreeForEnd(range.EndPosition(), shadow_host); } } // namespace blink
diff --git a/third_party/WebKit/Source/core/editing/SelectionAdjuster.h b/third_party/WebKit/Source/core/editing/SelectionAdjuster.h index f7c1047..836b83c 100644 --- a/third_party/WebKit/Source/core/editing/SelectionAdjuster.h +++ b/third_party/WebKit/Source/core/editing/SelectionAdjuster.h
@@ -17,9 +17,14 @@ STATIC_ONLY(SelectionAdjuster); public: - static void AdjustSelectionToAvoidCrossingShadowBoundaries(VisibleSelection*); - static void AdjustSelectionToAvoidCrossingShadowBoundaries( - VisibleSelectionInFlatTree*); + static Position AdjustSelectionStartToAvoidCrossingShadowBoundaries( + const EphemeralRange&); + static Position AdjustSelectionEndToAvoidCrossingShadowBoundaries( + const EphemeralRange&); + static PositionInFlatTree AdjustSelectionStartToAvoidCrossingShadowBoundaries( + const EphemeralRangeInFlatTree&); + static PositionInFlatTree AdjustSelectionEndToAvoidCrossingShadowBoundaries( + const EphemeralRangeInFlatTree&); }; } // namespace blink
diff --git a/third_party/WebKit/Source/core/editing/VisibleSelection.cpp b/third_party/WebKit/Source/core/editing/VisibleSelection.cpp index 88cfd71..a55d7d2 100644 --- a/third_party/WebKit/Source/core/editing/VisibleSelection.cpp +++ b/third_party/WebKit/Source/core/editing/VisibleSelection.cpp
@@ -483,7 +483,15 @@ granularity); end_ = new_end.IsNotNull() ? new_end : end; - AdjustSelectionToAvoidCrossingShadowBoundaries(); + if (base_is_first_) { + end_ = SelectionAdjuster::AdjustSelectionEndToAvoidCrossingShadowBoundaries( + EphemeralRangeTemplate<Strategy>(start_, end_)); + } else { + start_ = + SelectionAdjuster::AdjustSelectionStartToAvoidCrossingShadowBoundaries( + EphemeralRangeTemplate<Strategy>(start_, end_)); + } + AdjustSelectionToAvoidCrossingEditingBoundaries(); UpdateSelectionType(); @@ -554,14 +562,6 @@ return visible_selection; } -template <typename Strategy> -void VisibleSelectionTemplate< - Strategy>::AdjustSelectionToAvoidCrossingShadowBoundaries() { - if (base_.IsNull() || start_.IsNull() || base_.IsNull()) - return; - SelectionAdjuster::AdjustSelectionToAvoidCrossingShadowBoundaries(this); -} - static Element* LowestEditableAncestor(Node* node) { while (node) { if (HasEditableStyle(*node))
diff --git a/third_party/WebKit/Source/core/editing/VisibleSelection.h b/third_party/WebKit/Source/core/editing/VisibleSelection.h index 3e6210b3..4bfda490 100644 --- a/third_party/WebKit/Source/core/editing/VisibleSelection.h +++ b/third_party/WebKit/Source/core/editing/VisibleSelection.h
@@ -155,7 +155,6 @@ void Validate(const SelectionTemplate<Strategy>&, TextGranularity); // Support methods for Validate() - void AdjustSelectionToAvoidCrossingShadowBoundaries(); void AdjustSelectionToAvoidCrossingEditingBoundaries(); void UpdateSelectionType();
diff --git a/third_party/WebKit/Source/core/editing/commands/TypingCommand.cpp b/third_party/WebKit/Source/core/editing/commands/TypingCommand.cpp index 2509e4f..682a212 100644 --- a/third_party/WebKit/Source/core/editing/commands/TypingCommand.cpp +++ b/third_party/WebKit/Source/core/editing/commands/TypingCommand.cpp
@@ -897,95 +897,91 @@ frame->GetSpellChecker().UpdateMarkersForWordsAffectedByEditing(false); - switch (EndingSelection().GetSelectionType()) { - case kRangeSelection: - ForwardDeleteKeyPressedInternal(EndingSelection(), EndingSelection(), - kill_ring, editing_state); - return; - case kCaretSelection: { - smart_delete_ = false; - GetDocument().UpdateStyleAndLayoutIgnorePendingStylesheets(); - - // Handle delete at beginning-of-block case. - // Do nothing in the case that the caret is at the start of a - // root editable element or at the start of a document. - SelectionModifier selection_modifier(*frame, EndingSelection()); - selection_modifier.Modify(FrameSelection::kAlterationExtend, - kDirectionForward, granularity); - if (kill_ring && selection_modifier.Selection().IsCaret() && - granularity != TextGranularity::kCharacter) { - selection_modifier.Modify(FrameSelection::kAlterationExtend, - kDirectionForward, - TextGranularity::kCharacter); - } - - Position downstream_end = - MostForwardCaretPosition(EndingSelection().End()); - VisiblePosition visible_end = EndingSelection().VisibleEnd(); - Node* enclosing_table_cell = - EnclosingNodeOfType(visible_end.DeepEquivalent(), &IsTableCell); - if (enclosing_table_cell && - visible_end.DeepEquivalent() == - VisiblePosition::LastPositionInNode(*enclosing_table_cell) - .DeepEquivalent()) - return; - if (visible_end.DeepEquivalent() == - EndOfParagraph(visible_end).DeepEquivalent()) - downstream_end = MostForwardCaretPosition( - NextPositionOf(visible_end, kCannotCrossEditingBoundary) - .DeepEquivalent()); - // When deleting tables: Select the table first, then perform the deletion - if (IsDisplayInsideTable(downstream_end.ComputeContainerNode()) && - downstream_end.ComputeOffsetInContainerNode() <= - CaretMinOffset(downstream_end.ComputeContainerNode())) { - SetEndingSelection( - SelectionInDOMTree::Builder() - .SetBaseAndExtentDeprecated( - EndingSelection().End(), - Position::AfterNode(*downstream_end.ComputeContainerNode())) - .SetIsDirectional(EndingSelection().IsDirectional()) - .Build()); - TypingAddedToOpenCommand(kForwardDeleteKey); - return; - } - - // deleting to end of paragraph when at end of paragraph needs to merge - // the next paragraph (if any) - if (granularity == TextGranularity::kParagraphBoundary && - selection_modifier.Selection().IsCaret() && - IsEndOfParagraph(selection_modifier.Selection().VisibleEnd())) { - selection_modifier.Modify(FrameSelection::kAlterationExtend, - kDirectionForward, - TextGranularity::kCharacter); - } - - const VisibleSelection& selection_to_delete = - selection_modifier.Selection(); - if (!StartingSelection().IsRange() || - MostBackwardCaretPosition(selection_to_delete.Base()) != - StartingSelection().Start()) { - ForwardDeleteKeyPressedInternal( - selection_to_delete, selection_to_delete, kill_ring, editing_state); - return; - } - // It's a little tricky to compute what the starting selection would - // have been in the original document. We can't let the VisibleSelection - // class's validation kick in or it'll adjust for us based on the - // current state of the document and we'll get the wrong result. - const VisibleSelection& selection_after_undo = - VisibleSelection::CreateWithoutValidationDeprecated( - StartingSelection().Start(), - ComputeExtentForForwardDeleteUndo(selection_to_delete, - StartingSelection().End()), - TextAffinity::kDownstream); - ForwardDeleteKeyPressedInternal(selection_to_delete, selection_after_undo, - kill_ring, editing_state); - return; - } - case kNoSelection: - NOTREACHED(); - break; + if (EndingSelection().IsRange()) { + ForwardDeleteKeyPressedInternal(EndingSelection(), EndingSelection(), + kill_ring, editing_state); + return; } + + if (!EndingSelection().IsCaret()) { + NOTREACHED(); + return; + } + + smart_delete_ = false; + GetDocument().UpdateStyleAndLayoutIgnorePendingStylesheets(); + + // Handle delete at beginning-of-block case. + // Do nothing in the case that the caret is at the start of a + // root editable element or at the start of a document. + SelectionModifier selection_modifier(*frame, EndingSelection()); + selection_modifier.Modify(FrameSelection::kAlterationExtend, + kDirectionForward, granularity); + if (kill_ring && selection_modifier.Selection().IsCaret() && + granularity != TextGranularity::kCharacter) { + selection_modifier.Modify(FrameSelection::kAlterationExtend, + kDirectionForward, TextGranularity::kCharacter); + } + + Position downstream_end = MostForwardCaretPosition(EndingSelection().End()); + VisiblePosition visible_end = EndingSelection().VisibleEnd(); + Node* enclosing_table_cell = + EnclosingNodeOfType(visible_end.DeepEquivalent(), &IsTableCell); + if (enclosing_table_cell && + visible_end.DeepEquivalent() == + VisiblePosition::LastPositionInNode(*enclosing_table_cell) + .DeepEquivalent()) + return; + if (visible_end.DeepEquivalent() == + EndOfParagraph(visible_end).DeepEquivalent()) { + downstream_end = MostForwardCaretPosition( + NextPositionOf(visible_end, kCannotCrossEditingBoundary) + .DeepEquivalent()); + } + // When deleting tables: Select the table first, then perform the deletion + if (IsDisplayInsideTable(downstream_end.ComputeContainerNode()) && + downstream_end.ComputeOffsetInContainerNode() <= + CaretMinOffset(downstream_end.ComputeContainerNode())) { + SetEndingSelection( + SelectionInDOMTree::Builder() + .SetBaseAndExtentDeprecated( + EndingSelection().End(), + Position::AfterNode(*downstream_end.ComputeContainerNode())) + .SetIsDirectional(EndingSelection().IsDirectional()) + .Build()); + TypingAddedToOpenCommand(kForwardDeleteKey); + return; + } + + // deleting to end of paragraph when at end of paragraph needs to merge + // the next paragraph (if any) + if (granularity == TextGranularity::kParagraphBoundary && + selection_modifier.Selection().IsCaret() && + IsEndOfParagraph(selection_modifier.Selection().VisibleEnd())) { + selection_modifier.Modify(FrameSelection::kAlterationExtend, + kDirectionForward, TextGranularity::kCharacter); + } + + const VisibleSelection& selection_to_delete = selection_modifier.Selection(); + if (!StartingSelection().IsRange() || + MostBackwardCaretPosition(selection_to_delete.Base()) != + StartingSelection().Start()) { + ForwardDeleteKeyPressedInternal(selection_to_delete, selection_to_delete, + kill_ring, editing_state); + return; + } + // It's a little tricky to compute what the starting selection would + // have been in the original document. We can't let the VisibleSelection + // class's validation kick in or it'll adjust for us based on the + // current state of the document and we'll get the wrong result. + const VisibleSelection& selection_after_undo = + VisibleSelection::CreateWithoutValidationDeprecated( + StartingSelection().Start(), + ComputeExtentForForwardDeleteUndo(selection_to_delete, + StartingSelection().End()), + TextAffinity::kDownstream); + ForwardDeleteKeyPressedInternal(selection_to_delete, selection_after_undo, + kill_ring, editing_state); } void TypingCommand::ForwardDeleteKeyPressedInternal(
diff --git a/third_party/WebKit/Source/core/html/HTMLTableCellElement.cpp b/third_party/WebKit/Source/core/html/HTMLTableCellElement.cpp index 1de7d39..f7fda5a 100644 --- a/third_party/WebKit/Source/core/html/HTMLTableCellElement.cpp +++ b/third_party/WebKit/Source/core/html/HTMLTableCellElement.cpp
@@ -38,6 +38,11 @@ using namespace HTMLNames; +namespace { +const unsigned kDefaultColSpan = 1; +const unsigned kDefaultRowSpan = 1; +} // namespace + inline HTMLTableCellElement::HTMLTableCellElement(const QualifiedName& tag_name, Document& document) : HTMLTablePartElement(tag_name, document) {} @@ -49,7 +54,7 @@ unsigned value = 0; if (col_span_value.IsEmpty() || !ParseHTMLNonNegativeInteger(col_span_value, value)) - return 1; + return kDefaultColSpan; // Counting for https://github.com/whatwg/html/issues/1198 UseCounter::Count(GetDocument(), WebFeature::kHTMLTableCellElementColspan); if (value > 8190) { @@ -67,7 +72,7 @@ unsigned value = 0; if (row_span_value.IsEmpty() || !ParseHTMLNonNegativeInteger(row_span_value, value)) - return 1; + return kDefaultRowSpan; return std::max(1u, std::min(value, MaxRowSpan())); } @@ -160,7 +165,7 @@ } void HTMLTableCellElement::setColSpan(unsigned n) { - SetUnsignedIntegralAttribute(colspanAttr, n); + SetUnsignedIntegralAttribute(colspanAttr, n, kDefaultColSpan); } const AtomicString& HTMLTableCellElement::Headers() const { @@ -168,7 +173,7 @@ } void HTMLTableCellElement::setRowSpan(unsigned n) { - SetUnsignedIntegralAttribute(rowspanAttr, n); + SetUnsignedIntegralAttribute(rowspanAttr, n, kDefaultRowSpan); } } // namespace blink
diff --git a/third_party/WebKit/Source/core/html/HTMLTableColElement.cpp b/third_party/WebKit/Source/core/html/HTMLTableColElement.cpp index aa6274d..67d28fcb 100644 --- a/third_party/WebKit/Source/core/html/HTMLTableColElement.cpp +++ b/third_party/WebKit/Source/core/html/HTMLTableColElement.cpp
@@ -36,6 +36,10 @@ using namespace HTMLNames; +namespace { +const unsigned kDefaultSpan = 1; +} // namespace + inline HTMLTableColElement::HTMLTableColElement(const QualifiedName& tag_name, Document& document) : HTMLTablePartElement(tag_name, document), span_(1) {} @@ -69,7 +73,7 @@ new_span < 1) { // If the value of span is not a valid non-negative integer greater than // zero, set it to 1. - new_span = 1; + new_span = kDefaultSpan; } new_span = std::min(new_span, HTMLTableCellElement::MaxColSpan()); span_ = new_span; @@ -100,7 +104,7 @@ } void HTMLTableColElement::setSpan(unsigned n) { - SetUnsignedIntegralAttribute(spanAttr, n); + SetUnsignedIntegralAttribute(spanAttr, n, kDefaultSpan); } const AtomicString& HTMLTableColElement::Width() const {
diff --git a/third_party/WebKit/Source/core/workers/WorkerThread.cpp b/third_party/WebKit/Source/core/workers/WorkerThread.cpp index 537645b6..9fb1a4e 100644 --- a/third_party/WebKit/Source/core/workers/WorkerThread.cpp +++ b/third_party/WebKit/Source/core/workers/WorkerThread.cpp
@@ -136,27 +136,16 @@ { MutexLocker lock(thread_state_mutex_); - if (requested_to_terminate_) return; requested_to_terminate_ = true; - - if (ShouldScheduleToTerminateExecution(lock)) { - // Schedule a task to forcibly terminate the script execution in case that - // the shutdown sequence does not start on the worker thread in a certain - // time period. - DCHECK(!forcible_termination_task_handle_.IsActive()); - forcible_termination_task_handle_ = - parent_frame_task_runners_->Get(TaskType::kUnspecedTimer) - ->PostDelayedCancellableTask( - BLINK_FROM_HERE, - WTF::Bind(&WorkerThread::EnsureScriptExecutionTerminates, - WTF::Unretained(this), - ExitCode::kAsyncForciblyTerminated), - forcible_termination_delay_); - } } + // Schedule a task to forcibly terminate the script execution in case that the + // shutdown sequence does not start on the worker thread in a certain time + // period. + ScheduleToTerminateScriptExecution(); + worker_thread_lifecycle_context_->NotifyContextDestroyed(); inspector_task_runner_->Kill(); @@ -346,7 +335,19 @@ ConvertToBaseCallback(WTF::Bind(&ForwardInterfaceRequest))); } -bool WorkerThread::ShouldScheduleToTerminateExecution(const MutexLocker& lock) { +void WorkerThread::ScheduleToTerminateScriptExecution() { + DCHECK(!forcible_termination_task_handle_.IsActive()); + forcible_termination_task_handle_ = + parent_frame_task_runners_->Get(TaskType::kUnspecedTimer) + ->PostDelayedCancellableTask( + BLINK_FROM_HERE, + WTF::Bind(&WorkerThread::EnsureScriptExecutionTerminates, + WTF::Unretained(this), + ExitCode::kAsyncForciblyTerminated), + forcible_termination_delay_); +} + +bool WorkerThread::ShouldTerminateScriptExecution(const MutexLocker& lock) { DCHECK(IsMainThread()); DCHECK(IsThreadStateMutexLocked(lock)); @@ -372,7 +373,7 @@ void WorkerThread::EnsureScriptExecutionTerminates(ExitCode exit_code) { DCHECK(IsMainThread()); MutexLocker lock(thread_state_mutex_); - if (!ShouldScheduleToTerminateExecution(lock)) + if (!ShouldTerminateScriptExecution(lock)) return; DCHECK(exit_code == ExitCode::kSyncForciblyTerminated ||
diff --git a/third_party/WebKit/Source/core/workers/WorkerThread.h b/third_party/WebKit/Source/core/workers/WorkerThread.h index 831b3beb..4437bc7 100644 --- a/third_party/WebKit/Source/core/workers/WorkerThread.h +++ b/third_party/WebKit/Source/core/workers/WorkerThread.h
@@ -202,8 +202,7 @@ private: friend class WorkerThreadTest; - FRIEND_TEST_ALL_PREFIXES(WorkerThreadTest, - ShouldScheduleToTerminateExecution); + FRIEND_TEST_ALL_PREFIXES(WorkerThreadTest, ShouldTerminateScriptExecution); FRIEND_TEST_ALL_PREFIXES( WorkerThreadTest, Terminate_WhileDebuggerTaskIsRunningOnInitialization); @@ -221,11 +220,14 @@ kReadyToShutdown, }; - // Returns true if we should synchronously terminate or schedule to - // terminate the worker execution so that a shutdown task can be handled by - // the thread event loop. This must be called with |m_threadStateMutex| - // acquired. - bool ShouldScheduleToTerminateExecution(const MutexLocker&); + // Posts a delayed task to forcibly terminate script execution in case the + // normal shutdown sequence does not start within a certain time period. + void ScheduleToTerminateScriptExecution(); + + // Returns true if we should synchronously terminate the script execution so + // that a shutdown task can be handled by the thread event loop. This must be + // called with |m_threadStateMutex| acquired. + bool ShouldTerminateScriptExecution(const MutexLocker&); // Terminates worker script execution if the worker thread is running and not // already shutting down. Does not terminate if a debugger task is running,
diff --git a/third_party/WebKit/Source/core/workers/WorkerThreadTest.cpp b/third_party/WebKit/Source/core/workers/WorkerThreadTest.cpp index 9d06019d..a559965 100644 --- a/third_party/WebKit/Source/core/workers/WorkerThreadTest.cpp +++ b/third_party/WebKit/Source/core/workers/WorkerThreadTest.cpp
@@ -144,22 +144,22 @@ Persistent<MockWorkerThreadLifecycleObserver> lifecycle_observer_; }; -TEST_F(WorkerThreadTest, ShouldScheduleToTerminateExecution) { +TEST_F(WorkerThreadTest, ShouldTerminateScriptExecution) { using ThreadState = WorkerThread::ThreadState; MutexLocker dummy_lock(worker_thread_->thread_state_mutex_); EXPECT_EQ(ThreadState::kNotStarted, worker_thread_->thread_state_); - EXPECT_FALSE(worker_thread_->ShouldScheduleToTerminateExecution(dummy_lock)); + EXPECT_FALSE(worker_thread_->ShouldTerminateScriptExecution(dummy_lock)); worker_thread_->SetThreadState(dummy_lock, ThreadState::kRunning); EXPECT_FALSE(worker_thread_->running_debugger_task_); - EXPECT_TRUE(worker_thread_->ShouldScheduleToTerminateExecution(dummy_lock)); + EXPECT_TRUE(worker_thread_->ShouldTerminateScriptExecution(dummy_lock)); worker_thread_->running_debugger_task_ = true; - EXPECT_FALSE(worker_thread_->ShouldScheduleToTerminateExecution(dummy_lock)); + EXPECT_FALSE(worker_thread_->ShouldTerminateScriptExecution(dummy_lock)); worker_thread_->SetThreadState(dummy_lock, ThreadState::kReadyToShutdown); - EXPECT_FALSE(worker_thread_->ShouldScheduleToTerminateExecution(dummy_lock)); + EXPECT_FALSE(worker_thread_->ShouldTerminateScriptExecution(dummy_lock)); // This is necessary to satisfy DCHECK in the dtor of WorkerThread. worker_thread_->SetExitCode(dummy_lock, ExitCode::kGracefullyTerminated); @@ -215,14 +215,11 @@ // There are two possible cases depending on timing: // (1) If the thread hasn't been initialized on the worker thread yet, - // terminateAndWait() should wait for initialization and shut down the - // thread immediately after that. + // TerminateAllWorkersForTesting() should wait for initialization and shut + // down the thread immediately after that. // (2) If the thread has already been initialized on the worker thread, - // terminateAndWait() should synchronously forcibly terminates the worker - // execution. - // TODO(nhiroki): Make this test deterministically pass through the case 1), - // that is, terminateAndWait() is called before initializeOnWorkerThread(). - // Then, rename this test to SyncTerminate_BeforeInitialization. + // TerminateAllWorkersForTesting() should synchronously forcibly terminates + // the worker execution. WorkerThread::TerminateAllWorkersForTesting(); ExitCode exit_code = GetExitCode(); EXPECT_TRUE(ExitCode::kGracefullyTerminated == exit_code || @@ -237,17 +234,17 @@ StartWithSourceCodeNotToFinish(); reporting_proxy_->WaitUntilScriptEvaluation(); - // terminate() schedules a force termination task. + // Terminate() schedules a forcible termination task. worker_thread_->Terminate(); EXPECT_TRUE(IsForcibleTerminationTaskScheduled()); EXPECT_EQ(ExitCode::kNotTerminated, GetExitCode()); - // Multiple terminate() calls should not take effect. + // Multiple Terminate() calls should not take effect. worker_thread_->Terminate(); worker_thread_->Terminate(); EXPECT_EQ(ExitCode::kNotTerminated, GetExitCode()); - // Wait until the force termination task runs. + // Wait until the forcible termination task runs. testing::RunDelayedTasks(kDelay); worker_thread_->WaitForShutdownForTesting(); EXPECT_EQ(ExitCode::kAsyncForciblyTerminated, GetExitCode()); @@ -258,7 +255,8 @@ StartWithSourceCodeNotToFinish(); reporting_proxy_->WaitUntilScriptEvaluation(); - // terminateAndWait() synchronously terminates the worker execution. + // TerminateAllWorkersForTesting() synchronously terminates the worker + // execution. WorkerThread::TerminateAllWorkersForTesting(); EXPECT_EQ(ExitCode::kSyncForciblyTerminated, GetExitCode()); } @@ -271,18 +269,22 @@ StartWithSourceCodeNotToFinish(); reporting_proxy_->WaitUntilScriptEvaluation(); - // terminate() schedules a force termination task. + // Terminate() schedules a forcible termination task. worker_thread_->Terminate(); EXPECT_TRUE(IsForcibleTerminationTaskScheduled()); EXPECT_EQ(ExitCode::kNotTerminated, GetExitCode()); - // terminateAndWait() should overtake the scheduled force termination task. + // TerminateAllWorkersForTesting() should overtake the scheduled forcible + // termination task. WorkerThread::TerminateAllWorkersForTesting(); EXPECT_FALSE(IsForcibleTerminationTaskScheduled()); EXPECT_EQ(ExitCode::kSyncForciblyTerminated, GetExitCode()); } TEST_F(WorkerThreadTest, Terminate_WhileDebuggerTaskIsRunningOnInitialization) { + constexpr TimeDelta kDelay = TimeDelta::FromMilliseconds(10); + SetForcibleTerminationDelay(kDelay); + EXPECT_CALL(*reporting_proxy_, DidCreateWorkerGlobalScope(_)).Times(1); EXPECT_CALL(*reporting_proxy_, DidInitializeWorkerContext()).Times(1); EXPECT_CALL(*reporting_proxy_, WillDestroyWorkerGlobalScope()).Times(1); @@ -320,20 +322,19 @@ testing::EnterRunLoop(); EXPECT_TRUE(worker_thread_->running_debugger_task_); - // terminate() should not schedule a force termination task because there is - // a running debugger task. + // Terminate() schedules a forcible termination task. worker_thread_->Terminate(); + EXPECT_TRUE(IsForcibleTerminationTaskScheduled()); + EXPECT_EQ(ExitCode::kNotTerminated, GetExitCode()); + + // Wait until the task runs. It shouldn't terminate the script execution + // because of the running debugger task. + testing::RunDelayedTasks(kDelay); EXPECT_FALSE(IsForcibleTerminationTaskScheduled()); EXPECT_EQ(ExitCode::kNotTerminated, GetExitCode()); - // Multiple terminate() calls should not take effect. - worker_thread_->Terminate(); - worker_thread_->Terminate(); - EXPECT_FALSE(IsForcibleTerminationTaskScheduled()); - EXPECT_EQ(ExitCode::kNotTerminated, GetExitCode()); - - // Focible script termination request should also respect the running debugger - // task. + // Forcible script termination request should also respect the running + // debugger task. worker_thread_->EnsureScriptExecutionTerminates( ExitCode::kSyncForciblyTerminated); EXPECT_EQ(ExitCode::kNotTerminated, GetExitCode()); @@ -345,6 +346,9 @@ } TEST_F(WorkerThreadTest, Terminate_WhileDebuggerTaskIsRunning) { + constexpr TimeDelta kDelay = TimeDelta::FromMilliseconds(10); + SetForcibleTerminationDelay(kDelay); + ExpectReportingCalls(); Start(); worker_thread_->WaitForInit(); @@ -360,20 +364,19 @@ testing::EnterRunLoop(); EXPECT_TRUE(worker_thread_->running_debugger_task_); - // terminate() should not schedule a force termination task because there is - // a running debugger task. + // Terminate() schedules a forcible termination task. worker_thread_->Terminate(); + EXPECT_TRUE(IsForcibleTerminationTaskScheduled()); + EXPECT_EQ(ExitCode::kNotTerminated, GetExitCode()); + + // Wait until the task runs. It shouldn't terminate the script execution + // because of the running debugger task. + testing::RunDelayedTasks(kDelay); EXPECT_FALSE(IsForcibleTerminationTaskScheduled()); EXPECT_EQ(ExitCode::kNotTerminated, GetExitCode()); - // Multiple terminate() calls should not take effect. - worker_thread_->Terminate(); - worker_thread_->Terminate(); - EXPECT_FALSE(IsForcibleTerminationTaskScheduled()); - EXPECT_EQ(ExitCode::kNotTerminated, GetExitCode()); - - // Focible script termination request should also respect the running debugger - // task. + // Forcible script termination request should also respect the running + // debugger task. worker_thread_->EnsureScriptExecutionTerminates( ExitCode::kSyncForciblyTerminated); EXPECT_EQ(ExitCode::kNotTerminated, GetExitCode());
diff --git a/third_party/WebKit/Source/modules/accessibility/AXLayoutObject.cpp b/third_party/WebKit/Source/modules/accessibility/AXLayoutObject.cpp index a0d2f7a..3868b61 100644 --- a/third_party/WebKit/Source/modules/accessibility/AXLayoutObject.cpp +++ b/third_party/WebKit/Source/modules/accessibility/AXLayoutObject.cpp
@@ -1279,7 +1279,11 @@ } bool AXLayoutObject::AriaHasPopup() const { - return ElementAttributeValue(aria_haspopupAttr); + const AtomicString& has_popup = + GetAOMPropertyOrARIAAttribute(AOMStringProperty::kHasPopUp); + + return !has_popup.IsNull() && !has_popup.IsEmpty() && + !EqualIgnoringASCIICase(has_popup, "false"); } bool AXLayoutObject::AriaRoleHasPresentationalChildren() const {
diff --git a/third_party/WebKit/Source/modules/accessibility/InspectorAccessibilityAgent.cpp b/third_party/WebKit/Source/modules/accessibility/InspectorAccessibilityAgent.cpp index 6ec4a83..fa0c1f7 100644 --- a/third_party/WebKit/Source/modules/accessibility/InspectorAccessibilityAgent.cpp +++ b/third_party/WebKit/Source/modules/accessibility/InspectorAccessibilityAgent.cpp
@@ -165,8 +165,8 @@ CreateProperty(AXWidgetAttributesEnum::Autocomplete, CreateValue(autocomplete, AXValueTypeEnum::Token))); - if (ax_object.HasAttribute(HTMLNames::aria_haspopupAttr)) { - bool has_popup = ax_object.AriaHasPopup(); + bool has_popup = ax_object.AriaHasPopup(); + if (has_popup || ax_object.HasAttribute(HTMLNames::aria_haspopupAttr)) { properties.addItem(CreateProperty(AXWidgetAttributesEnum::Haspopup, CreateBooleanValue(has_popup))); }
diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index c904b1d0..0e249e8 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/tools/metrics/histograms/histograms.xml
@@ -15406,8 +15406,11 @@ <summary>The start result of download attempts.</summary> </histogram> -<histogram name="Download.Service.StartUpStatus" +<histogram base="true" name="Download.Service.StartUpStatus" enum="Download.Service.StartUpResult"> +<!-- Name completed by histogram_suffixes + name="Download.Service.StartUpStep" --> + <owner>xingliu@chromium.org</owner> <summary>The start up result of the download service.</summary> </histogram> @@ -69043,6 +69046,16 @@ </summary> </histogram> +<histogram name="Search.ContextualSearchTapOnWordMiddleSeen" + enum="ContextualSearchResultsSeen"> + <owner>donnd@chromium.org</owner> + <owner>twellington@chromium.org</owner> + <summary> + Whether results were seen for a Tap that was on the middle part of a word. + Recorded when the UX is hidden. Implemented for Android. + </summary> +</histogram> + <histogram name="Search.ContextualSearchTapShortDurationSeen" enum="ContextualSearchResultsSeen"> <owner>donnd@chromium.org</owner> @@ -91226,6 +91239,12 @@ <affected-histogram name="Download.Service.Db.Records"/> </histogram_suffixes> +<histogram_suffixes name="Download.Service.StartUpStep" separator="."> + <suffix name="Initialization" label="The initialization start up step."/> + <suffix name="Recovery" label="The recovery start up step."/> + <affected-histogram name="Download.Service.StartUpStatus"/> +</histogram_suffixes> + <histogram_suffixes name="Download.Service.TaskType" separator="."> <suffix name="DownloadTask" label="Download task."/> <suffix name="CleanUpTask" label="Clean up task."/>
diff --git a/ui/app_list/views/app_list_view_unittest.cc b/ui/app_list/views/app_list_view_unittest.cc index 8bbca86..c7b25e51 100644 --- a/ui/app_list/views/app_list_view_unittest.cc +++ b/ui/app_list/views/app_list_view_unittest.cc
@@ -9,16 +9,13 @@ #include <string> #include <vector> -#include "base/command_line.h" #include "base/macros.h" #include "base/memory/ptr_util.h" #include "base/run_loop.h" #include "base/strings/string_util.h" #include "base/strings/utf_string_conversions.h" -#include "build/build_config.h" #include "testing/gtest/include/gtest/gtest.h" #include "ui/app_list/app_list_constants.h" -#include "ui/app_list/app_list_switches.h" #include "ui/app_list/pagination_model.h" #include "ui/app_list/search_box_model.h" #include "ui/app_list/test/app_list_test_model.h" @@ -38,16 +35,16 @@ #include "ui/app_list/views/tile_item_view.h" #include "ui/events/event_utils.h" #include "ui/views/controls/textfield/textfield.h" -#include "ui/views/test/test_views_delegate.h" #include "ui/views/test/views_test_base.h" -#include "ui/views/views_delegate.h" -#include "ui/views/widget/desktop_aura/desktop_native_widget_aura.h" namespace app_list { namespace test { namespace { +// Choose a set that is 3 regular app list pages and 2 landscape app list pages. +constexpr int kInitialItems = 34; + template <class T> size_t GetVisibleViews(const std::vector<T*>& tiles) { size_t count = 0; @@ -58,6 +55,15 @@ return count; } +// A standard set of checks on a view, e.g., ensuring it is drawn and visible. +void CheckView(views::View* subview) { + ASSERT_TRUE(subview); + EXPECT_TRUE(subview->parent()); + EXPECT_TRUE(subview->visible()); + EXPECT_TRUE(subview->IsDrawn()); + EXPECT_FALSE(subview->bounds().IsEmpty()); +} + void SimulateClick(views::View* view) { gfx::Point center = view->GetLocalBounds().CenterPoint(); view->OnMousePressed(ui::MouseEvent( @@ -68,9 +74,6 @@ ui::EF_LEFT_MOUSE_BUTTON, ui::EF_LEFT_MOUSE_BUTTON)); } -// Choose a set that is 3 regular app list pages and 2 landscape app list pages. -const int kInitialItems = 34; - class TestStartPageSearchResult : public TestSearchResult { public: TestStartPageSearchResult() { set_display_type(DISPLAY_RECOMMENDATION); } @@ -80,174 +83,98 @@ DISALLOW_COPY_AND_ASSIGN(TestStartPageSearchResult); }; -// Allows the same tests to run with different contexts: either an Ash-style -// root window or a desktop window tree host. -class AppListViewTestContext { +class AppListViewTest : public views::ViewsTestBase { public: - explicit AppListViewTestContext(gfx::NativeView parent); - ~AppListViewTestContext(); + AppListViewTest() = default; + ~AppListViewTest() override = default; - // Test displaying the app list and performs a standard set of checks on its - // top level views. Then closes the window. - void RunDisplayTest(); + // testing::Test overrides: + void SetUp() override { + views::ViewsTestBase::SetUp(); - // Hides and reshows the app list with a folder open, expecting the main grid - // view to be shown. - void RunReshowWithOpenFolderTest(); + gfx::NativeView parent = GetContext(); + delegate_.reset(new AppListTestViewDelegate); + view_ = new AppListView(delegate_.get()); - // Tests that pressing the search box's back button navigates correctly. - void RunBackTest(); - - // Tests displaying of the app list and shows the start page. - void RunStartPageTest(); - - // Tests switching rapidly between multiple pages of the launcher. - void RunPageSwitchingAnimationTest(); - - // Tests displaying of the search results. - void RunSearchResultsTest(); - - // Tests displaying the app list overlay. - void RunAppListOverlayTest(); - - // A standard set of checks on a view, e.g., ensuring it is drawn and visible. - static void CheckView(views::View* subview); - - // Invoked when the Widget is closing, and the view it contains is about to - // be torn down. This only occurs in a run loop and will be used as a signal - // to quit. - void NativeWidgetClosing() { - view_ = NULL; - run_loop_->Quit(); + view_->Initialize(parent, 0, false, false); + // Initialize around a point that ensures the window is wholly shown. + gfx::Size size = view_->bounds().size(); + view_->MaybeSetAnchorPoint(gfx::Point(size.width() / 2, size.height() / 2)); } - private: + // testing::Test overrides: + void TearDown() override { + view_->GetWidget()->Close(); + views::ViewsTestBase::TearDown(); + } + + protected: // Switches the launcher to |state| and lays out to ensure all launcher pages // are in the correct position. Checks that the state is where it should be // and returns false on failure. - bool SetAppListState(AppListModel::State state); + bool SetAppListState(AppListModel::State state) { + ContentsView* contents_view = view_->app_list_main_view()->contents_view(); + contents_view->SetActiveState(state); + contents_view->Layout(); + return IsStateShown(state); + } // Returns true if all of the pages are in their correct position for |state|. - bool IsStateShown(AppListModel::State state); + bool IsStateShown(AppListModel::State state) { + ContentsView* contents_view = view_->app_list_main_view()->contents_view(); + bool success = true; + for (int i = 0; i < contents_view->NumLauncherPages(); ++i) { + success = success && + (contents_view->GetPageView(i)->GetPageBoundsForState(state) == + contents_view->GetPageView(i)->bounds()); + } + return success && state == delegate_->GetTestModel()->state(); + } // Shows the app list and waits until a paint occurs. - void Show(); + void Show() { + view_->GetWidget()->Show(); + base::RunLoop run_loop; + AppListViewTestApi test_api(view_); + test_api.SetNextPaintCallback(run_loop.QuitClosure()); + run_loop.Run(); - // Closes the app list. This sets |view_| to NULL. - void Close(); + EXPECT_TRUE(view_->GetWidget()->IsVisible()); + } // Checks the search box widget is at |expected| in the contents view's // coordinate space. - bool CheckSearchBoxWidget(const gfx::Rect& expected); + bool CheckSearchBoxWidget(const gfx::Rect& expected) { + ContentsView* contents_view = view_->app_list_main_view()->contents_view(); + // Adjust for the search box view's shadow. + gfx::Rect expected_with_shadow = + view_->app_list_main_view() + ->search_box_view() + ->GetViewBoundsForSearchBoxContentsBounds(expected); + gfx::Point point = expected_with_shadow.origin(); + views::View::ConvertPointToScreen(contents_view, &point); + + return gfx::Rect(point, expected_with_shadow.size()) == + view_->search_box_widget()->GetWindowBoundsInScreen(); + } // Gets the PaginationModel owned by |view_|. - PaginationModel* GetPaginationModel(); + PaginationModel* GetPaginationModel() const { + return view_->GetAppsPaginationModel(); + } - std::unique_ptr<base::RunLoop> run_loop_; - app_list::AppListView* view_; // Owned by native widget. - std::unique_ptr<app_list::test::AppListTestViewDelegate> delegate_; - - DISALLOW_COPY_AND_ASSIGN(AppListViewTestContext); -}; - -// Extend the regular AppListTestViewDelegate to communicate back to the test -// context. Note the test context doesn't simply inherit this, because the -// delegate is owned by the view. -class UnitTestViewDelegate : public app_list::test::AppListTestViewDelegate { - public: - explicit UnitTestViewDelegate(AppListViewTestContext* parent) - : parent_(parent) {} - - // Overridden from app_list::test::AppListTestViewDelegate: - void ViewClosing() override { parent_->NativeWidgetClosing(); } + AppListView* view_ = nullptr; // Owned by native widget. + std::unique_ptr<AppListTestViewDelegate> delegate_; private: - AppListViewTestContext* parent_; - - DISALLOW_COPY_AND_ASSIGN(UnitTestViewDelegate); + DISALLOW_COPY_AND_ASSIGN(AppListViewTest); }; -AppListViewTestContext::AppListViewTestContext(gfx::NativeView parent) { - delegate_.reset(new UnitTestViewDelegate(this)); - view_ = new app_list::AppListView(delegate_.get()); +} // namespace - // Initialize centered around a point that ensures the window is wholly shown. - view_->Initialize(parent, 0, false, false); - view_->MaybeSetAnchorPoint(gfx::Point(300, 300)); -} - -AppListViewTestContext::~AppListViewTestContext() { - // The view observes the PaginationModel which is about to get destroyed, so - // if the view is not already deleted by the time this destructor is called, - // there will be problems. - EXPECT_FALSE(view_); -} - -// static -void AppListViewTestContext::CheckView(views::View* subview) { - ASSERT_TRUE(subview); - EXPECT_TRUE(subview->parent()); - EXPECT_TRUE(subview->visible()); - EXPECT_TRUE(subview->IsDrawn()); - EXPECT_FALSE(subview->bounds().IsEmpty()); -} - -bool AppListViewTestContext::SetAppListState(AppListModel::State state) { - ContentsView* contents_view = view_->app_list_main_view()->contents_view(); - contents_view->SetActiveState(state); - contents_view->Layout(); - return IsStateShown(state); -} - -bool AppListViewTestContext::IsStateShown(AppListModel::State state) { - ContentsView* contents_view = view_->app_list_main_view()->contents_view(); - bool success = true; - for (int i = 0; i < contents_view->NumLauncherPages(); ++i) { - success = success && - (contents_view->GetPageView(i)->GetPageBoundsForState(state) == - contents_view->GetPageView(i)->bounds()); - } - return success && state == delegate_->GetTestModel()->state(); -} - -void AppListViewTestContext::Show() { - view_->GetWidget()->Show(); - run_loop_.reset(new base::RunLoop); - AppListViewTestApi test_api(view_); - test_api.SetNextPaintCallback(run_loop_->QuitClosure()); - run_loop_->Run(); - - EXPECT_TRUE(view_->GetWidget()->IsVisible()); -} - -void AppListViewTestContext::Close() { - view_->GetWidget()->Close(); - run_loop_.reset(new base::RunLoop); - run_loop_->Run(); - - // |view_| should have been deleted and set to NULL via ViewClosing(). - EXPECT_FALSE(view_); -} - -bool AppListViewTestContext::CheckSearchBoxWidget(const gfx::Rect& expected) { - ContentsView* contents_view = view_->app_list_main_view()->contents_view(); - // Adjust for the search box view's shadow. - gfx::Rect expected_with_shadow = - view_->app_list_main_view() - ->search_box_view() - ->GetViewBoundsForSearchBoxContentsBounds(expected); - gfx::Point point = expected_with_shadow.origin(); - views::View::ConvertPointToScreen(contents_view, &point); - - return gfx::Rect(point, expected_with_shadow.size()) == - view_->search_box_widget()->GetWindowBoundsInScreen(); -} - -PaginationModel* AppListViewTestContext::GetPaginationModel() { - return view_->GetAppsPaginationModel(); -} - -void AppListViewTestContext::RunDisplayTest() { +// Tests displaying the app list and performs a standard set of checks on its +// top level views. Then closes the window. +TEST_F(AppListViewTest, DisplayTest) { EXPECT_FALSE(view_->GetWidget()->IsVisible()); EXPECT_EQ(-1, GetPaginationModel()->total_pages()); delegate_->GetTestModel()->PopulateApps(kInitialItems); @@ -270,11 +197,11 @@ AppListModel::State expected = AppListModel::STATE_START; EXPECT_TRUE(main_view->contents_view()->IsStateActive(expected)); EXPECT_EQ(expected, delegate_->GetTestModel()->state()); - - Close(); } -void AppListViewTestContext::RunReshowWithOpenFolderTest() { +// Tests that the main grid view is shown after hiding and reshowing the app +// list with a folder view open. This is a regression test for crbug.com/357058. +TEST_F(AppListViewTest, ReshowWithOpenFolderTest) { EXPECT_FALSE(view_->GetWidget()->IsVisible()); EXPECT_EQ(-1, GetPaginationModel()->total_pages()); @@ -314,63 +241,10 @@ EXPECT_NO_FATAL_FAILURE(CheckView(main_view)); EXPECT_NO_FATAL_FAILURE(CheckView(container_view->apps_grid_view())); EXPECT_FALSE(container_view->app_list_folder_view()->visible()); - - Close(); } -void AppListViewTestContext::RunBackTest() { - EXPECT_FALSE(view_->GetWidget()->IsVisible()); - EXPECT_EQ(-1, GetPaginationModel()->total_pages()); - - Show(); - - AppListMainView* main_view = view_->app_list_main_view(); - ContentsView* contents_view = main_view->contents_view(); - SearchBoxView* search_box_view = main_view->search_box_view(); - - // Show the apps grid. - SetAppListState(AppListModel::STATE_APPS); - EXPECT_NO_FATAL_FAILURE(CheckView(search_box_view->back_button())); - - // The back button should return to the start page. - EXPECT_TRUE(contents_view->Back()); - contents_view->Layout(); - EXPECT_TRUE(IsStateShown(AppListModel::STATE_START)); - EXPECT_FALSE(search_box_view->back_button()->visible()); - - // Show the apps grid again. - SetAppListState(AppListModel::STATE_APPS); - EXPECT_NO_FATAL_FAILURE(CheckView(search_box_view->back_button())); - - // Pressing ESC should return to the start page. - view_->AcceleratorPressed(ui::Accelerator(ui::VKEY_ESCAPE, ui::EF_NONE)); - contents_view->Layout(); - EXPECT_TRUE(IsStateShown(AppListModel::STATE_START)); - EXPECT_FALSE(search_box_view->back_button()->visible()); - - // Pressing ESC from the start page should close the app list. - EXPECT_EQ(0, delegate_->dismiss_count()); - view_->AcceleratorPressed(ui::Accelerator(ui::VKEY_ESCAPE, ui::EF_NONE)); - EXPECT_EQ(1, delegate_->dismiss_count()); - - // Show the search results. - base::string16 new_search_text = base::UTF8ToUTF16("apple"); - search_box_view->search_box()->SetText(base::string16()); - search_box_view->search_box()->InsertText(new_search_text); - contents_view->Layout(); - EXPECT_TRUE(IsStateShown(AppListModel::STATE_SEARCH_RESULTS)); - EXPECT_NO_FATAL_FAILURE(CheckView(search_box_view->back_button())); - - // The back button should return to the start page. - EXPECT_TRUE(contents_view->Back()); - contents_view->Layout(); - EXPECT_TRUE(IsStateShown(AppListModel::STATE_START)); - EXPECT_FALSE(search_box_view->back_button()->visible()); - - Close(); -} - -void AppListViewTestContext::RunStartPageTest() { +// Tests that the start page view operates correctly. +TEST_F(AppListViewTest, StartPageTest) { EXPECT_FALSE(view_->GetWidget()->IsVisible()); EXPECT_EQ(-1, GetPaginationModel()->total_pages()); AppListTestModel* model = delegate_->GetTestModel(); @@ -423,13 +297,10 @@ EXPECT_EQ(0u, GetVisibleViews(start_page_view->tile_views())); EXPECT_TRUE(SetAppListState(AppListModel::STATE_START)); EXPECT_EQ(1u, GetVisibleViews(start_page_view->tile_views())); - - Close(); } -void AppListViewTestContext::RunPageSwitchingAnimationTest() { - Show(); - +// Tests switching rapidly between multiple pages of the launcher. +TEST_F(AppListViewTest, PageSwitchingAnimationTest) { AppListMainView* main_view = view_->app_list_main_view(); // Checks on the main view. EXPECT_NO_FATAL_FAILURE(CheckView(main_view)); @@ -454,11 +325,10 @@ // Call Layout(). Should jump to the third page. contents_view->Layout(); IsStateShown(AppListModel::STATE_APPS); - - Close(); } -void AppListViewTestContext::RunSearchResultsTest() { +// Tests that the correct views are displayed for showing search results. +TEST_F(AppListViewTest, SearchResultsTest) { EXPECT_FALSE(view_->GetWidget()->IsVisible()); EXPECT_EQ(-1, GetPaginationModel()->total_pages()); AppListTestModel* model = delegate_->GetTestModel(); @@ -515,11 +385,61 @@ contents_view->Layout(); EXPECT_TRUE(IsStateShown(AppListModel::STATE_SEARCH_RESULTS)); EXPECT_TRUE(CheckSearchBoxWidget(contents_view->GetDefaultSearchBoxBounds())); - - Close(); } -void AppListViewTestContext::RunAppListOverlayTest() { +// Tests that the back button navigates through the app list correctly. +TEST_F(AppListViewTest, BackTest) { + EXPECT_FALSE(view_->GetWidget()->IsVisible()); + EXPECT_EQ(-1, GetPaginationModel()->total_pages()); + + Show(); + + AppListMainView* main_view = view_->app_list_main_view(); + ContentsView* contents_view = main_view->contents_view(); + SearchBoxView* search_box_view = main_view->search_box_view(); + + // Show the apps grid. + SetAppListState(AppListModel::STATE_APPS); + EXPECT_NO_FATAL_FAILURE(CheckView(search_box_view->back_button())); + + // The back button should return to the start page. + EXPECT_TRUE(contents_view->Back()); + contents_view->Layout(); + EXPECT_TRUE(IsStateShown(AppListModel::STATE_START)); + EXPECT_FALSE(search_box_view->back_button()->visible()); + + // Show the apps grid again. + SetAppListState(AppListModel::STATE_APPS); + EXPECT_NO_FATAL_FAILURE(CheckView(search_box_view->back_button())); + + // Pressing ESC should return to the start page. + view_->AcceleratorPressed(ui::Accelerator(ui::VKEY_ESCAPE, ui::EF_NONE)); + contents_view->Layout(); + EXPECT_TRUE(IsStateShown(AppListModel::STATE_START)); + EXPECT_FALSE(search_box_view->back_button()->visible()); + + // Pressing ESC from the start page should close the app list. + EXPECT_EQ(0, delegate_->dismiss_count()); + view_->AcceleratorPressed(ui::Accelerator(ui::VKEY_ESCAPE, ui::EF_NONE)); + EXPECT_EQ(1, delegate_->dismiss_count()); + + // Show the search results. + base::string16 new_search_text = base::UTF8ToUTF16("apple"); + search_box_view->search_box()->SetText(base::string16()); + search_box_view->search_box()->InsertText(new_search_text); + contents_view->Layout(); + EXPECT_TRUE(IsStateShown(AppListModel::STATE_SEARCH_RESULTS)); + EXPECT_NO_FATAL_FAILURE(CheckView(search_box_view->back_button())); + + // The back button should return to the start page. + EXPECT_TRUE(contents_view->Back()); + contents_view->Layout(); + EXPECT_TRUE(IsStateShown(AppListModel::STATE_START)); + EXPECT_FALSE(search_box_view->back_button()->visible()); +} + +// Tests that the correct views are displayed for showing search results. +TEST_F(AppListViewTest, AppListOverlayTest) { Show(); AppListMainView* main_view = view_->app_list_main_view(); @@ -534,159 +454,6 @@ EXPECT_TRUE(search_box_view->enabled()); EXPECT_EQ(search_box_view->search_box(), view_->GetWidget()->GetFocusManager()->GetFocusedView()); - - Close(); -} - -class AppListViewTestAura : public views::ViewsTestBase { - public: - AppListViewTestAura() {} - ~AppListViewTestAura() override {} - - // testing::Test overrides: - void SetUp() override { - views::ViewsTestBase::SetUp(); - - // On Ash (only) the app list is placed into an aura::Window "container", - // which is also used to determine the context. In tests, use the ash root - // window as the parent. This only works on aura where the root window is a - // NativeView as well as a NativeWindow. - gfx::NativeView container = NULL; - container = GetContext(); - test_context_.reset(new AppListViewTestContext(container)); - } - - void TearDown() override { - test_context_.reset(); - views::ViewsTestBase::TearDown(); - } - - protected: - std::unique_ptr<AppListViewTestContext> test_context_; - - private: - DISALLOW_COPY_AND_ASSIGN(AppListViewTestAura); -}; - -class AppListViewTestDesktop : public views::ViewsTestBase { - public: - AppListViewTestDesktop() {} - ~AppListViewTestDesktop() override {} - - // testing::Test overrides: - void SetUp() override { - set_views_delegate(base::MakeUnique<AppListViewTestViewsDelegate>(this)); - views::ViewsTestBase::SetUp(); - test_context_.reset(new AppListViewTestContext(NULL)); - } - - void TearDown() override { - test_context_.reset(); - views::ViewsTestBase::TearDown(); - } - - protected: - std::unique_ptr<AppListViewTestContext> test_context_; - - private: - class AppListViewTestViewsDelegate : public views::TestViewsDelegate { - public: - explicit AppListViewTestViewsDelegate(AppListViewTestDesktop* parent) - : parent_(parent) - { - } - - // Overridden from views::ViewsDelegate: - void OnBeforeWidgetInit( - views::Widget::InitParams* params, - views::internal::NativeWidgetDelegate* delegate) override; - - private: - AppListViewTestDesktop* parent_; - - DISALLOW_COPY_AND_ASSIGN(AppListViewTestViewsDelegate); - }; - - DISALLOW_COPY_AND_ASSIGN(AppListViewTestDesktop); -}; - -void AppListViewTestDesktop::AppListViewTestViewsDelegate::OnBeforeWidgetInit( - views::Widget::InitParams* params, - views::internal::NativeWidgetDelegate* delegate) { -// Mimic the logic in ChromeViewsDelegate::OnBeforeWidgetInit(). Except, for -// ChromeOS, use the root window from the AuraTestHelper rather than depending -// on ash::Shell:GetPrimaryRootWindow(). Also assume non-ChromeOS is never the -// Ash desktop, as that is covered by AppListViewTestAura. - if (!params->parent && !params->context) - params->context = parent_->GetContext(); -} - -} // namespace - -// Tests showing the app list with basic test model in an ash-style root window. -TEST_F(AppListViewTestAura, Display) { - EXPECT_NO_FATAL_FAILURE(test_context_->RunDisplayTest()); -} - -// Tests showing the app list on the desktop. Note on ChromeOS, this will still -// use the regular root window. -TEST_F(AppListViewTestDesktop, Display) { - EXPECT_NO_FATAL_FAILURE(test_context_->RunDisplayTest()); -} - -// Tests that the main grid view is shown after hiding and reshowing the app -// list with a folder view open. This is a regression test for crbug.com/357058. -TEST_F(AppListViewTestAura, ReshowWithOpenFolder) { - EXPECT_NO_FATAL_FAILURE(test_context_->RunReshowWithOpenFolderTest()); -} - -TEST_F(AppListViewTestDesktop, ReshowWithOpenFolder) { - EXPECT_NO_FATAL_FAILURE(test_context_->RunReshowWithOpenFolderTest()); -} - -// Tests that the start page view operates correctly. -TEST_F(AppListViewTestAura, StartPageTest) { - EXPECT_NO_FATAL_FAILURE(test_context_->RunStartPageTest()); -} - -TEST_F(AppListViewTestDesktop, StartPageTest) { - EXPECT_NO_FATAL_FAILURE(test_context_->RunStartPageTest()); -} - -// Tests that the start page view operates correctly. -TEST_F(AppListViewTestAura, PageSwitchingAnimationTest) { - EXPECT_NO_FATAL_FAILURE(test_context_->RunPageSwitchingAnimationTest()); -} - -TEST_F(AppListViewTestDesktop, PageSwitchingAnimationTest) { - EXPECT_NO_FATAL_FAILURE(test_context_->RunPageSwitchingAnimationTest()); -} - -// Tests that the correct views are displayed for showing search results. -TEST_F(AppListViewTestAura, SearchResultsTest) { - EXPECT_NO_FATAL_FAILURE(test_context_->RunSearchResultsTest()); -} - -TEST_F(AppListViewTestDesktop, SearchResultsTest) { - EXPECT_NO_FATAL_FAILURE(test_context_->RunSearchResultsTest()); -} - -// Tests that the back button navigates through the app list correctly. -TEST_F(AppListViewTestAura, BackTest) { - EXPECT_NO_FATAL_FAILURE(test_context_->RunBackTest()); -} - -TEST_F(AppListViewTestDesktop, BackTest) { - EXPECT_NO_FATAL_FAILURE(test_context_->RunBackTest()); -} - -// Tests that the correct views are displayed for showing search results. -TEST_F(AppListViewTestAura, AppListOverlayTest) { - EXPECT_NO_FATAL_FAILURE(test_context_->RunAppListOverlayTest()); -} - -TEST_F(AppListViewTestDesktop, AppListOverlayTest) { - EXPECT_NO_FATAL_FAILURE(test_context_->RunAppListOverlayTest()); } } // namespace test
diff --git a/ui/base/ime/input_method_chromeos.cc b/ui/base/ime/input_method_chromeos.cc index 695e23ed..de201a30 100644 --- a/ui/base/ime/input_method_chromeos.cc +++ b/ui/base/ime/input_method_chromeos.cc
@@ -59,7 +59,7 @@ ui::EventDispatchDetails InputMethodChromeOS::DispatchKeyEvent( ui::KeyEvent* event, - std::unique_ptr<AckCallback> ack_callback) { + AckCallback ack_callback) { DCHECK(event->IsKeyEvent()); DCHECK(!(event->flags() & ui::EF_IS_SYNTHESIZED)); @@ -80,7 +80,7 @@ // generates some IME event, dispatch_details = ProcessKeyEventPostIME(event, true); if (ack_callback) - ack_callback->Run(true); + std::move(ack_callback).Run(true); return dispatch_details; } dispatch_details = ProcessUnfilteredKeyPressEvent(event); @@ -88,7 +88,7 @@ dispatch_details = DispatchKeyEventPostIME(event); } if (ack_callback) - ack_callback->Run(false); + std::move(ack_callback).Run(false); return dispatch_details; } @@ -112,17 +112,16 @@ return false; } -void InputMethodChromeOS::KeyEventDoneCallback( - ui::KeyEvent* event, - std::unique_ptr<AckCallback> ack_callback, - bool is_handled) { +void InputMethodChromeOS::KeyEventDoneCallback(ui::KeyEvent* event, + AckCallback ack_callback, + bool is_handled) { ignore_result( ProcessKeyEventDone(event, std::move(ack_callback), is_handled)); } ui::EventDispatchDetails InputMethodChromeOS::ProcessKeyEventDone( ui::KeyEvent* event, - std::unique_ptr<AckCallback> ack_callback, + AckCallback ack_callback, bool is_handled) { DCHECK(event); if (event->type() == ET_KEY_PRESSED) { @@ -138,7 +137,7 @@ } if (ack_callback) - ack_callback->Run(is_handled); + std::move(ack_callback).Run(is_handled); ui::EventDispatchDetails details; if (event->type() == ET_KEY_PRESSED || event->type() == ET_KEY_RELEASED) @@ -150,7 +149,7 @@ ui::EventDispatchDetails InputMethodChromeOS::DispatchKeyEvent( ui::KeyEvent* event) { - return DispatchKeyEvent(event, nullptr); + return DispatchKeyEvent(event, AckCallback()); } void InputMethodChromeOS::OnTextInputTypeChanged(
diff --git a/ui/base/ime/input_method_chromeos.h b/ui/base/ime/input_method_chromeos.h index 3dc5a3b..8c375a6 100644 --- a/ui/base/ime/input_method_chromeos.h +++ b/ui/base/ime/input_method_chromeos.h
@@ -28,10 +28,9 @@ explicit InputMethodChromeOS(internal::InputMethodDelegate* delegate); ~InputMethodChromeOS() override; - using AckCallback = base::Callback<void(bool)>; - ui::EventDispatchDetails DispatchKeyEvent( - ui::KeyEvent* event, - std::unique_ptr<AckCallback> ack_callback); + using AckCallback = base::OnceCallback<void(bool)>; + ui::EventDispatchDetails DispatchKeyEvent(ui::KeyEvent* event, + AckCallback ack_callback); // Overridden from InputMethod: bool OnUntranslatedIMEMessage(const base::NativeEvent& event, @@ -110,12 +109,12 @@ // Callback function for IMEEngineHandlerInterface::ProcessKeyEvent. void KeyEventDoneCallback(ui::KeyEvent* event, - std::unique_ptr<AckCallback> ack_callback, + AckCallback ack_callback, bool is_handled); - ui::EventDispatchDetails ProcessKeyEventDone( - ui::KeyEvent* event, - std::unique_ptr<AckCallback> ack_callback, - bool is_handled) WARN_UNUSED_RESULT; + ui::EventDispatchDetails ProcessKeyEventDone(ui::KeyEvent* event, + AckCallback ack_callback, + bool is_handled) + WARN_UNUSED_RESULT; // Returns whether an non-password input field is focused. bool IsNonPasswordInputFieldFocused();
diff --git a/ui/file_manager/file_manager/foreground/css/file_manager.css b/ui/file_manager/file_manager/foreground/css/file_manager.css index 46259a7..91b20ea 100644 --- a/ui/file_manager/file_manager/foreground/css/file_manager.css +++ b/ui/file_manager/file_manager/foreground/css/file_manager.css
@@ -363,18 +363,6 @@ url(../images/files/ui/2x/share.png) 2x); } -#share-button > .icon { - background-image: -webkit-image-set( - url(../images/files/ui/person_add_white.png) 1x, - url(../images/files/ui/2x/person_add_white.png) 2x); -} - -body.check-select #share-button > .icon { - background-image: -webkit-image-set( - url(../images/files/ui/person_add.png) 1x, - url(../images/files/ui/2x/person_add.png) 2x); -} - #delete-button > .icon { background-image: -webkit-image-set( url(../images/files/ui/delete_white.png) 1x, @@ -1655,6 +1643,12 @@ padding-left: 32px; } +#share-menu cr-menu-item[command='#share'] { + background-image: -webkit-image-set( + url(../images/files/ui/person_add.png) 1x, + url(../images/files/ui/2x/person_add.png) 2x); +} + .detail-name > * { align-items: center; display: flex;
diff --git a/ui/file_manager/file_manager/foreground/images/files/ui/2x/person_add_white.png b/ui/file_manager/file_manager/foreground/images/files/ui/2x/person_add_white.png deleted file mode 100644 index 0f2a111..0000000 --- a/ui/file_manager/file_manager/foreground/images/files/ui/2x/person_add_white.png +++ /dev/null Binary files differ
diff --git a/ui/file_manager/file_manager/foreground/images/files/ui/person_add_white.png b/ui/file_manager/file_manager/foreground/images/files/ui/person_add_white.png deleted file mode 100644 index 152e94b..0000000 --- a/ui/file_manager/file_manager/foreground/images/files/ui/person_add_white.png +++ /dev/null Binary files differ
diff --git a/ui/file_manager/file_manager/foreground/js/file_tasks.js b/ui/file_manager/file_manager/foreground/js/file_tasks.js index 05c41f8c..8e0fc74 100644 --- a/ui/file_manager/file_manager/foreground/js/file_tasks.js +++ b/ui/file_manager/file_manager/foreground/js/file_tasks.js
@@ -743,11 +743,29 @@ * @param {!Array<!Object>} tasks */ FileTasks.prototype.updateShareMenuButton_ = function(shareMenuButton, tasks) { - shareMenuButton.hidden = tasks.length == 0; - if (tasks.length == 0) - return; + var driveShareCommand = + shareMenuButton.menu.querySelector('cr-menu-item[command="#share"]'); + var driveShareCommandSeparator = + shareMenuButton.menu.querySelector('#drive-share-separator'); - shareMenuButton.menu.clear(); + shareMenuButton.hidden = driveShareCommand.disabled && tasks.length == 0; + + // Show the separator if Drive share command is enabled and there is at least + // one other share actions. + driveShareCommandSeparator.hidden = + driveShareCommand.disabled || tasks.length == 0; + + // Clear menu items except for drive share menu and a separator for it. + // As querySelectorAll() returns live NodeList, we need to copy elements to + // Array object to modify DOM in the for loop. + var itemsToRemove = [].slice.call( + shareMenuButton.menu.querySelectorAll('cr-menu-item[command~="share"]')); + for (var i = 0; i < itemsToRemove.length; i++) { + var item = itemsToRemove[i]; + item.parentNode.removeChild(item); + } + + // Add menu items for the new tasks. var items = this.createItems_(tasks); for (var i = 0; i < items.length; i++) { var menuitem = shareMenuButton.menu.addMenuItem(items[i]);
diff --git a/ui/file_manager/file_manager/foreground/js/task_controller.js b/ui/file_manager/file_manager/foreground/js/task_controller.js index ff34eb6..b6862c0 100644 --- a/ui/file_manager/file_manager/foreground/js/task_controller.js +++ b/ui/file_manager/file_manager/foreground/js/task_controller.js
@@ -148,6 +148,11 @@ * @private */ TaskController.prototype.onTaskItemClicked_ = function(event) { + // If the clicked target has an associated command, the click event should not + // be handled here since it is handled as a command. + if (event.target && event.target.command) + return; + // 'select' event from ComboButton has the item as event.item. // 'activate' event from cr.ui.MenuButton has the item as event.target.data. var item = event.item || event.target.data;
diff --git a/ui/file_manager/file_manager/main.html b/ui/file_manager/file_manager/main.html index 0075dea..54dd735 100644 --- a/ui/file_manager/file_manager/main.html +++ b/ui/file_manager/file_manager/main.html
@@ -263,6 +263,8 @@ <cr-menu id="share-menu" class="chrome-menu files-menu" menu-item-selector="cr-menu-item, hr"> + <cr-menu-item command="#share"></cr-menu-item> + <hr id="drive-share-separator"> </cr-menu> <cr-menu id="add-new-services-menu" @@ -301,14 +303,6 @@ <files-toggle-ripple></files-toggle-ripple> <div class="icon"></div> </button> - <button id="share-button" class="icon-button" command="#share" - tabindex="11" hidden - i18n-values="aria-label:SHARE_BUTTON_LABEL" - visibleif="full-page" - has-tooltip> - <files-ripple></files-ripple> - <div class="icon"></div> - </button> <button id="delete-button" class="icon-button" tabindex="12" hidden i18n-values="aria-label:DELETE_BUTTON_LABEL" visibleif="full-page"
diff --git a/ui/file_manager/integration_tests/file_manager/share_dialog.js b/ui/file_manager/integration_tests/file_manager/share_dialog.js index 0ac284a..44d1249a 100644 --- a/ui/file_manager/integration_tests/file_manager/share_dialog.js +++ b/ui/file_manager/integration_tests/file_manager/share_dialog.js
@@ -24,15 +24,29 @@ // Wait for the share button. function(result) { chrome.test.assertTrue(result); - remoteCall.waitForElement(appId, '#share-button:not([disabled])'). - then(this.next); + remoteCall.waitForElement(appId, '#share-menu-button:not([disabled])') + .then(this.next); + }, + // Open share options menu + function(result) { + chrome.test.assertTrue(!!result); + remoteCall.callRemoteTestUtil( + 'fakeMouseClick', appId, ['#share-menu-button'], this.next); + }, + // Wait until the "Share with others" item is shown. + function(result) { + chrome.test.assertTrue(result); + remoteCall + .waitForElement( + appId, 'cr-menu-item[command="#share"]:not([disabled]') + .then(this.next); }, // Invoke the share dialog. function(result) { - remoteCall.callRemoteTestUtil('fakeMouseClick', - appId, - ['#share-button'], - this.next); + chrome.test.assertTrue(!!result); + remoteCall.callRemoteTestUtil( + 'fakeMouseClick', appId, ['cr-menu-item[command="#share"]'], + this.next); }, // Wait until the share dialog's contents are shown. function(result) {
diff --git a/ui/file_manager/integration_tests/file_manager/tab_index.js b/ui/file_manager/integration_tests/file_manager/tab_index.js index 04e14800..f32f5fc 100644 --- a/ui/file_manager/integration_tests/file_manager/tab_index.js +++ b/ui/file_manager/integration_tests/file_manager/tab_index.js
@@ -164,47 +164,61 @@ }, function(elements) { remoteCall.callRemoteTestUtil('getActiveElement', appId, [], this.next); - }, function(element) { + }, + function(element) { chrome.test.assertEq('list', element.attributes['class']); // Select the directory named 'photos'. remoteCall.callRemoteTestUtil( 'selectFile', appId, ['photos']).then(this.next); - }, function(result) { + }, + function(result) { chrome.test.assertTrue(result); - Promise.all([ - remoteCall.waitForElement( - appId, ['#share-button:not([hidden]):not([disabled])']), - remoteCall.waitForElement( - appId, ['#delete-button:not([hidden]):not([disabled])']), - ]).then(this.next); - // Press the Tab key. - }, function(elements) { - remoteCall.checkNextTabFocus(appId, 'share-button').then(this.next); - }, function(result) { + Promise + .all([ + remoteCall.waitForElement( + appId, ['#share-menu-button:not([hidden]):not([disabled])']), + remoteCall.waitForElement( + appId, ['#delete-button:not([hidden]):not([disabled])']), + ]) + .then(this.next); + // Press the Tab key. + }, + function(elements) { + remoteCall.checkNextTabFocus(appId, 'share-menu-button').then(this.next); + }, + function(result) { chrome.test.assertTrue(result); remoteCall.checkNextTabFocus(appId, 'delete-button').then(this.next); - }, function(result) { + }, + function(result) { chrome.test.assertTrue(result); remoteCall.checkNextTabFocus(appId, 'search-button').then(this.next); - }, function(result) { + }, + function(result) { chrome.test.assertTrue(result); remoteCall.checkNextTabFocus(appId, 'view-button').then(this.next); - }, function(result) { + }, + function(result) { chrome.test.assertTrue(result); remoteCall.checkNextTabFocus(appId, 'sort-button').then(this.next); - }, function(result) { + }, + function(result) { chrome.test.assertTrue(result); remoteCall.checkNextTabFocus(appId, 'gear-button').then(this.next); - }, function(result) { + }, + function(result) { chrome.test.assertTrue(result); remoteCall.checkNextTabFocus(appId, 'directory-tree').then(this.next); - }, function(result) { + }, + function(result) { chrome.test.assertTrue(result); remoteCall.checkNextTabFocus(appId, 'drive-welcome-link').then(this.next); - }, function(result) { + }, + function(result) { chrome.test.assertTrue(result); remoteCall.checkNextTabFocus(appId, 'file-list').then(this.next); - }, function(result) { + }, + function(result) { chrome.test.assertTrue(result); checkIfNoErrorsOccured(this.next); }
