diff --git a/DEPS b/DEPS index b43a6bbd..ec626118 100644 --- a/DEPS +++ b/DEPS
@@ -63,7 +63,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': '19ada67df35d8d480d47637b98179d651ff3742f', + 'v8_revision': '7382dc2601c2f6793411579e9211397acd347deb', # 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/base/cancelable_callback.h b/base/cancelable_callback.h index 13cbd0c..a98101a 100644 --- a/base/cancelable_callback.h +++ b/base/cancelable_callback.h
@@ -56,28 +56,24 @@ #include "base/memory/weak_ptr.h" namespace base { +namespace internal { -template <typename Sig> -class CancelableCallback; - -template <typename... A> -class CancelableCallback<void(A...)> { +template <typename CallbackType> +class CancelableCallbackImpl { public: - CancelableCallback() : weak_factory_(this) {} + CancelableCallbackImpl() : weak_ptr_factory_(this) {} // |callback| must not be null. - explicit CancelableCallback(const base::Callback<void(A...)>& callback) - : callback_(callback), weak_factory_(this) { - DCHECK(!callback.is_null()); - InitializeForwarder(); + explicit CancelableCallbackImpl(CallbackType callback) + : callback_(std::move(callback)), weak_ptr_factory_(this) { + DCHECK(callback_); } - ~CancelableCallback() {} + ~CancelableCallbackImpl() = default; // Cancels and drops the reference to the wrapped callback. void Cancel() { - weak_factory_.InvalidateWeakPtrs(); - forwarder_.Reset(); + weak_ptr_factory_.InvalidateWeakPtrs(); callback_.Reset(); } @@ -88,48 +84,72 @@ // Sets |callback| as the closure that may be cancelled. |callback| may not // be null. Outstanding and any previously wrapped callbacks are cancelled. - void Reset(const base::Callback<void(A...)>& callback) { - DCHECK(!callback.is_null()); - + void Reset(CallbackType callback) { + DCHECK(callback); // Outstanding tasks (e.g., posted to a message loop) must not be called. Cancel(); - - // |forwarder_| is no longer valid after Cancel(), so re-bind. - InitializeForwarder(); - - callback_ = callback; + callback_ = std::move(callback); } // Returns a callback that can be disabled by calling Cancel(). - const base::Callback<void(A...)>& callback() const { - return forwarder_; + CallbackType callback() const { + if (!callback_) + return CallbackType(); + CallbackType forwarder; + MakeForwarder(&forwarder); + return forwarder; } private: - void Forward(A... args) const { - callback_.Run(std::forward<A>(args)...); + template <typename... Args> + void MakeForwarder(RepeatingCallback<void(Args...)>* out) const { + using ForwarderType = void (CancelableCallbackImpl::*)(Args...); + ForwarderType forwarder = &CancelableCallbackImpl::ForwardRepeating; + *out = BindRepeating(forwarder, weak_ptr_factory_.GetWeakPtr()); } - // Helper method to bind |forwarder_| using a weak pointer from - // |weak_factory_|. - void InitializeForwarder() { - forwarder_ = base::Bind(&CancelableCallback<void(A...)>::Forward, - weak_factory_.GetWeakPtr()); + template <typename... Args> + void MakeForwarder(OnceCallback<void(Args...)>* out) const { + using ForwarderType = void (CancelableCallbackImpl::*)(Args...); + ForwarderType forwarder = &CancelableCallbackImpl::ForwardOnce; + *out = BindOnce(forwarder, weak_ptr_factory_.GetWeakPtr()); } - // The wrapper closure. - base::Callback<void(A...)> forwarder_; + template <typename... Args> + void ForwardRepeating(Args... args) { + callback_.Run(std::forward<Args>(args)...); + } + + template <typename... Args> + void ForwardOnce(Args... args) { + weak_ptr_factory_.InvalidateWeakPtrs(); + std::move(callback_).Run(std::forward<Args>(args)...); + } // The stored closure that may be cancelled. - base::Callback<void(A...)> callback_; + CallbackType callback_; + mutable base::WeakPtrFactory<CancelableCallbackImpl> weak_ptr_factory_; - // Used to ensure Forward() is not run when this object is destroyed. - base::WeakPtrFactory<CancelableCallback<void(A...)>> weak_factory_; - - DISALLOW_COPY_AND_ASSIGN(CancelableCallback); + DISALLOW_COPY_AND_ASSIGN(CancelableCallbackImpl); }; -typedef CancelableCallback<void(void)> CancelableClosure; +} // namespace internal + +// Consider using base::WeakPtr directly instead of base::CancelableCallback for +// the task cancellation. +template <typename Signature> +using CancelableOnceCallback = + internal::CancelableCallbackImpl<OnceCallback<Signature>>; +using CancelableOnceClosure = CancelableOnceCallback<void()>; + +template <typename Signature> +using CancelableRepeatingCallback = + internal::CancelableCallbackImpl<RepeatingCallback<Signature>>; +using CancelableRepeatingClosure = CancelableOnceCallback<void()>; + +template <typename Signature> +using CancelableCallback = CancelableRepeatingCallback<Signature>; +using CancelableClosure = CancelableCallback<void()>; } // namespace base
diff --git a/chrome/browser/apps/guest_view/web_view_browsertest.cc b/chrome/browser/apps/guest_view/web_view_browsertest.cc index 9a4182a..07fe429 100644 --- a/chrome/browser/apps/guest_view/web_view_browsertest.cc +++ b/chrome/browser/apps/guest_view/web_view_browsertest.cc
@@ -1778,6 +1778,13 @@ } // Test makes sure that interstitial pages renders in <webview>. +// Flaky on Win dbg: crbug.com/779973 +#if defined(OS_WIN) && !defined(NDEBUG) +#define MAYBE_InterstitialPage DISABLED_InterstitialPage +#else +#define MAYBE_InterstitialPage InterstitialPage +#endif + IN_PROC_BROWSER_TEST_P(WebViewTest, InterstitialPage) { // This test tests that a inner WebContents' InterstitialPage is properly // connected to an outer WebContents through a CrossProcessFrameConnector, it @@ -1801,6 +1808,13 @@ // Test makes sure that interstitial pages are registered in the // RenderWidgetHostInputEventRouter when inside a <webview>. +// Flaky on Win dbg: crbug.com/779973 +#if defined(OS_WIN) && !defined(NDEBUG) +#define MAYBE_InterstitialPageRouteEvents DISABLED_InterstitialPageRouteEvents +#else +#define MAYBE_InterstitialPageRouteEvents InterstitialPageRouteEvents +#endif + IN_PROC_BROWSER_TEST_P(WebViewTest, InterstitialPageRouteEvents) { // This test tests that a inner WebContents' InterstitialPage is properly // connected to an outer WebContents through a CrossProcessFrameConnector, it @@ -1824,6 +1838,14 @@ // Test makes sure that interstitial pages will receive input events and can be // focused. +// Flaky on Win dbg: crbug.com/779973 +#if defined(OS_WIN) && !defined(NDEBUG) +#define MAYBE_InterstitialPageFocusedWidget \ + DISABLED_InterstitialPageFocusedWidget +#else +#define MAYBE_InterstitialPageFocusedWidget InterstitialPageFocusedWidget +#endif + IN_PROC_BROWSER_TEST_P(WebViewTest, InterstitialPageFocusedWidget) { // This test tests that a inner WebContents' InterstitialPage is properly // connected to an outer WebContents through a CrossProcessFrameConnector, it @@ -1877,6 +1899,13 @@ // Test makes sure that the browser does not crash when a <webview> navigates // out of an interstitial. +// Flaky on Win dbg: crbug.com/779973 +#if defined(OS_WIN) && !defined(NDEBUG) +#define MAYBE_InterstitialPageDetach DISABLED_InterstitialPageDetach +#else +#define MAYBE_InterstitialPageDetach InterstitialPageDetach +#endif + IN_PROC_BROWSER_TEST_P(WebViewTest, InterstitialPageDetach) { InterstitialTestHelper(); @@ -1894,6 +1923,13 @@ // This test makes sure the browser process does not crash if app is closed // while an interstitial page is being shown in guest. +// Flaky on Win dbg: crbug.com/779973 +#if defined(OS_WIN) && !defined(NDEBUG) +#define MAYBE_InterstitialTeardown DISABLED_InterstitialTeardown +#else +#define MAYBE_InterstitialTeardown InterstitialTeardown +#endif + IN_PROC_BROWSER_TEST_P(WebViewTest, InterstitialTeardown) { InterstitialTestHelper();
diff --git a/chrome/browser/chrome_content_gpu_manifest_overlay.json b/chrome/browser/chrome_content_gpu_manifest_overlay.json index 6398526a..0d7c7c1 100644 --- a/chrome/browser/chrome_content_gpu_manifest_overlay.json +++ b/chrome/browser/chrome_content_gpu_manifest_overlay.json
@@ -4,12 +4,12 @@ "service_manager:connector": { "provides": { "browser": [ + "arc::mojom::ProtectedBufferManager", "arc::mojom::VideoDecodeAccelerator", "arc::mojom::VideoDecodeClient", "arc::mojom::VideoEncodeAccelerator", "arc::mojom::VideoEncodeClient", "chrome::mojom::ResourceUsageReporter", - "media::mojom::ProtectedBufferManager", "profiling::mojom::ProfilingClient" ] }
diff --git a/chrome/browser/chromeos/arc/oemcrypto/arc_oemcrypto_bridge.cc b/chrome/browser/chromeos/arc/oemcrypto/arc_oemcrypto_bridge.cc index 030cad8..4505e33 100644 --- a/chrome/browser/chromeos/arc/oemcrypto/arc_oemcrypto_bridge.cc +++ b/chrome/browser/chromeos/arc/oemcrypto/arc_oemcrypto_bridge.cc
@@ -149,7 +149,7 @@ mojom::OemCryptoServiceRequest request) { // Get the Mojo interface from the GPU for dealing with secure buffers and // pass that to the daemon as well in our Connect call. - media::mojom::ProtectedBufferManagerPtr gpu_buffer_manager; + mojom::ProtectedBufferManagerPtr gpu_buffer_manager; content::BindInterfaceInGpuProcess(mojo::MakeRequest(&gpu_buffer_manager)); oemcrypto_host_daemon_ptr_->Connect(std::move(request), std::move(gpu_buffer_manager));
diff --git a/chrome/browser/devtools/device/devtools_android_bridge.cc b/chrome/browser/devtools/device/devtools_android_bridge.cc index 583b4616..66de4522 100644 --- a/chrome/browser/devtools/device/devtools_android_bridge.cc +++ b/chrome/browser/devtools/device/devtools_android_bridge.cc
@@ -297,8 +297,7 @@ const base::Callback<void(int)>& callback) { DCHECK_CURRENTLY_ON(BrowserThread::UI); - if (device_count_listeners_.empty() || - !callback.Equals(device_count_callback_.callback())) + if (device_count_listeners_.empty() || callback.IsCancelled()) return; UsbDeviceProvider::CountDevices(callback);
diff --git a/chrome/browser/resource_coordinator/browser_child_process_watcher.cc b/chrome/browser/resource_coordinator/browser_child_process_watcher.cc index e91d89b..e0c86ec 100644 --- a/chrome/browser/resource_coordinator/browser_child_process_watcher.cc +++ b/chrome/browser/resource_coordinator/browser_child_process_watcher.cc
@@ -8,6 +8,7 @@ #include "content/public/browser/child_process_data.h" #include "content/public/common/process_type.h" #include "content/public/common/service_manager_connection.h" +#include "services/resource_coordinator/public/cpp/process_resource_coordinator.h" #include "services/resource_coordinator/public/cpp/resource_coordinator_features.h" namespace resource_coordinator { @@ -27,17 +28,11 @@ if (data.process_type == content::PROCESS_TYPE_GPU) { gpu_process_resource_coordinator_ = - base::MakeUnique<resource_coordinator::ResourceCoordinatorInterface>( - content::ServiceManagerConnection::GetForProcess()->GetConnector(), - resource_coordinator::CoordinationUnitType::kProcess); + base::MakeUnique<resource_coordinator::ProcessResourceCoordinator>( + content::ServiceManagerConnection::GetForProcess()->GetConnector()); - base::ProcessId pid = base::GetProcId(data.handle); - gpu_process_resource_coordinator_->SetProperty( - resource_coordinator::mojom::PropertyType::kPID, pid); - - gpu_process_resource_coordinator_->SetProperty( - resource_coordinator::mojom::PropertyType::kLaunchTime, - base::Time::Now().ToTimeT()); + gpu_process_resource_coordinator_->SetLaunchTime(base::Time::Now()); + gpu_process_resource_coordinator_->SetPID(base::GetProcId(data.handle)); } }
diff --git a/chrome/browser/resource_coordinator/browser_child_process_watcher.h b/chrome/browser/resource_coordinator/browser_child_process_watcher.h index 8e637e1..8202859 100644 --- a/chrome/browser/resource_coordinator/browser_child_process_watcher.h +++ b/chrome/browser/resource_coordinator/browser_child_process_watcher.h
@@ -9,10 +9,11 @@ #include "base/macros.h" #include "content/public/browser/browser_child_process_observer.h" #include "content/public/common/service_manager_connection.h" -#include "services/resource_coordinator/public/cpp/resource_coordinator_interface.h" namespace resource_coordinator { +class ProcessResourceCoordinator; + class BrowserChildProcessWatcher : public content::BrowserChildProcessObserver { public: BrowserChildProcessWatcher(); @@ -31,7 +32,7 @@ void GPUProcessStopped(); - std::unique_ptr<resource_coordinator::ResourceCoordinatorInterface> + std::unique_ptr<resource_coordinator::ProcessResourceCoordinator> gpu_process_resource_coordinator_; DISALLOW_COPY_AND_ASSIGN(BrowserChildProcessWatcher);
diff --git a/chrome/browser/resource_coordinator/chrome_browser_main_extra_parts_resource_coordinator.cc b/chrome/browser/resource_coordinator/chrome_browser_main_extra_parts_resource_coordinator.cc index d8f9552..d96b9b4 100644 --- a/chrome/browser/resource_coordinator/chrome_browser_main_extra_parts_resource_coordinator.cc +++ b/chrome/browser/resource_coordinator/chrome_browser_main_extra_parts_resource_coordinator.cc
@@ -7,6 +7,7 @@ #include "base/process/process.h" #include "chrome/browser/resource_coordinator/browser_child_process_watcher.h" #include "content/public/common/service_manager_connection.h" +#include "services/resource_coordinator/public/cpp/process_resource_coordinator.h" #include "services/resource_coordinator/public/cpp/resource_coordinator_features.h" ChromeBrowserMainExtraPartsResourceCoordinator:: @@ -21,17 +22,11 @@ return; process_resource_coordinator_ = - base::MakeUnique<resource_coordinator::ResourceCoordinatorInterface>( - connection->GetConnector(), - resource_coordinator::CoordinationUnitType::kProcess); + base::MakeUnique<resource_coordinator::ProcessResourceCoordinator>( + connection->GetConnector()); - process_resource_coordinator_->SetProperty( - resource_coordinator::mojom::PropertyType::kPID, - base::Process::Current().Pid()); - - process_resource_coordinator_->SetProperty( - resource_coordinator::mojom::PropertyType::kLaunchTime, - base::Time::Now().ToTimeT()); + process_resource_coordinator_->SetLaunchTime(base::Time::Now()); + process_resource_coordinator_->SetPID(base::Process::Current().Pid()); browser_child_process_watcher_ = base::MakeUnique<resource_coordinator::BrowserChildProcessWatcher>();
diff --git a/chrome/browser/resource_coordinator/chrome_browser_main_extra_parts_resource_coordinator.h b/chrome/browser/resource_coordinator/chrome_browser_main_extra_parts_resource_coordinator.h index 2305c5b..b581afd 100644 --- a/chrome/browser/resource_coordinator/chrome_browser_main_extra_parts_resource_coordinator.h +++ b/chrome/browser/resource_coordinator/chrome_browser_main_extra_parts_resource_coordinator.h
@@ -9,7 +9,12 @@ #include "base/macros.h" #include "chrome/browser/chrome_browser_main_extra_parts.h" #include "chrome/browser/resource_coordinator/browser_child_process_watcher.h" -#include "services/resource_coordinator/public/cpp/resource_coordinator_interface.h" + +namespace resource_coordinator { + +class ProcessResourceCoordinator; + +} // namespace resource_coordinator class ChromeBrowserMainExtraPartsResourceCoordinator : public ChromeBrowserMainExtraParts { @@ -22,7 +27,7 @@ void ServiceManagerConnectionStarted( content::ServiceManagerConnection* connection) override; - std::unique_ptr<resource_coordinator::ResourceCoordinatorInterface> + std::unique_ptr<resource_coordinator::ProcessResourceCoordinator> process_resource_coordinator_; std::unique_ptr<resource_coordinator::BrowserChildProcessWatcher>
diff --git a/chrome/browser/resource_coordinator/resource_coordinator_render_process_probe.cc b/chrome/browser/resource_coordinator/resource_coordinator_render_process_probe.cc index 054273d..bfe17a82 100644 --- a/chrome/browser/resource_coordinator/resource_coordinator_render_process_probe.cc +++ b/chrome/browser/resource_coordinator/resource_coordinator_render_process_probe.cc
@@ -13,8 +13,8 @@ #include "content/public/browser/browser_thread.h" #include "content/public/browser/render_process_host.h" #include "content/public/common/service_manager_connection.h" +#include "services/resource_coordinator/public/cpp/process_resource_coordinator.h" #include "services/resource_coordinator/public/cpp/resource_coordinator_features.h" -#include "services/resource_coordinator/public/cpp/resource_coordinator_interface.h" #include "services/resource_coordinator/public/interfaces/coordination_unit.mojom.h" #if defined(OS_MACOSX) @@ -54,8 +54,8 @@ auto& render_process_info = render_process_info_map_entry.second; // TODO(oysteine): Move the multiplier used to avoid precision loss // into a shared location, when this property gets used. - render_process_info.host->GetProcessResourceCoordinator()->SetProperty( - mojom::PropertyType::kCPUUsage, render_process_info.cpu_usage * 1000); + render_process_info.host->GetProcessResourceCoordinator()->SetCPUUsage( + render_process_info.cpu_usage); } return true;
diff --git a/chrome/browser/resource_coordinator/resource_coordinator_web_contents_observer.cc b/chrome/browser/resource_coordinator/resource_coordinator_web_contents_observer.cc index 0454dc2..d835bc0 100644 --- a/chrome/browser/resource_coordinator/resource_coordinator_web_contents_observer.cc +++ b/chrome/browser/resource_coordinator/resource_coordinator_web_contents_observer.cc
@@ -20,6 +20,8 @@ #include "content/public/common/service_names.mojom.h" #include "services/metrics/public/cpp/ukm_recorder.h" #include "services/metrics/public/interfaces/ukm_interface.mojom.h" +#include "services/resource_coordinator/public/cpp/page_resource_coordinator.h" +#include "services/resource_coordinator/public/cpp/process_resource_coordinator.h" #include "services/resource_coordinator/public/cpp/resource_coordinator_features.h" #include "services/resource_coordinator/public/interfaces/coordination_unit.mojom.h" #include "services/resource_coordinator/public/interfaces/service_callbacks.mojom.h" @@ -37,14 +39,12 @@ content::ServiceManagerConnection::GetForProcess()->GetConnector(); page_resource_coordinator_ = - base::MakeUnique<resource_coordinator::ResourceCoordinatorInterface>( - connector, resource_coordinator::CoordinationUnitType::kPage); + base::MakeUnique<resource_coordinator::PageResourceCoordinator>( + connector); // Make sure to set the visibility property when we create // |page_resource_coordinator_|. - page_resource_coordinator_->SetProperty( - resource_coordinator::mojom::PropertyType::kVisible, - web_contents->IsVisible()); + page_resource_coordinator_->SetVisibility(web_contents->IsVisible()); connector->BindInterface(resource_coordinator::mojom::kServiceName, mojo::MakeRequest(&service_callbacks_)); @@ -114,13 +114,11 @@ } void ResourceCoordinatorWebContentsObserver::WasShown() { - page_resource_coordinator_->SetProperty( - resource_coordinator::mojom::PropertyType::kVisible, true); + page_resource_coordinator_->SetVisibility(true); } void ResourceCoordinatorWebContentsObserver::WasHidden() { - page_resource_coordinator_->SetProperty( - resource_coordinator::mojom::PropertyType::kVisible, false); + page_resource_coordinator_->SetVisibility(false); } void ResourceCoordinatorWebContentsObserver::WebContentsDestroyed() { @@ -145,8 +143,7 @@ if (navigation_handle->IsInMainFrame()) { UpdateUkmRecorder(navigation_handle->GetNavigationId()); ResetFlag(); - page_resource_coordinator_->SendEvent( - resource_coordinator::mojom::Event::kNavigationCommitted); + page_resource_coordinator_->OnMainFrameNavigationCommitted(); } content::RenderFrameHost* render_frame_host = @@ -154,11 +151,11 @@ auto* frame_resource_coordinator = render_frame_host->GetFrameResourceCoordinator(); - page_resource_coordinator_->AddChild(*frame_resource_coordinator); + page_resource_coordinator_->AddFrame(*frame_resource_coordinator); auto* process_resource_coordinator = render_frame_host->GetProcess()->GetProcessResourceCoordinator(); - process_resource_coordinator->AddChild(*frame_resource_coordinator); + process_resource_coordinator->AddFrame(*frame_resource_coordinator); } void ResourceCoordinatorWebContentsObserver::TitleWasSet( @@ -167,8 +164,7 @@ first_time_title_set_ = true; return; } - page_resource_coordinator_->SendEvent( - resource_coordinator::mojom::Event::kTitleUpdated); + page_resource_coordinator_->OnTitleUpdated(); } void ResourceCoordinatorWebContentsObserver::DidUpdateFaviconURL( @@ -177,8 +173,7 @@ first_time_favicon_set_ = true; return; } - page_resource_coordinator_->SendEvent( - resource_coordinator::mojom::Event::kFaviconUpdated); + page_resource_coordinator_->OnFaviconUpdated(); } void ResourceCoordinatorWebContentsObserver::UpdateUkmRecorder( @@ -189,8 +184,7 @@ ukm_source_id_ = ukm::ConvertToSourceId(navigation_id, ukm::SourceIdType::NAVIGATION_ID); - page_resource_coordinator_->SetProperty( - resource_coordinator::mojom::PropertyType::kUKMSourceId, ukm_source_id_); + page_resource_coordinator_->SetUKMSourceId(ukm_source_id_); } void ResourceCoordinatorWebContentsObserver::ResetFlag() {
diff --git a/chrome/browser/resource_coordinator/resource_coordinator_web_contents_observer.h b/chrome/browser/resource_coordinator/resource_coordinator_web_contents_observer.h index 35df83d..580c268 100644 --- a/chrome/browser/resource_coordinator/resource_coordinator_web_contents_observer.h +++ b/chrome/browser/resource_coordinator/resource_coordinator_web_contents_observer.h
@@ -12,10 +12,13 @@ #include "content/public/browser/web_contents_observer.h" #include "content/public/browser/web_contents_user_data.h" #include "services/metrics/public/cpp/ukm_source_id.h" -#include "services/resource_coordinator/public/cpp/resource_coordinator_interface.h" #include "services/resource_coordinator/public/interfaces/service_callbacks.mojom.h" #include "url/gurl.h" +namespace resource_coordinator { +class PageResourceCoordinator; +} // namespace resource_coordinator + class ResourceCoordinatorWebContentsObserver : public content::WebContentsObserver, public content::WebContentsUserData< @@ -27,8 +30,7 @@ static bool IsEnabled(); - resource_coordinator::ResourceCoordinatorInterface* - page_resource_coordinator() { + resource_coordinator::PageResourceCoordinator* page_resource_coordinator() { return page_resource_coordinator_.get(); } @@ -58,7 +60,7 @@ friend class content::WebContentsUserData< ResourceCoordinatorWebContentsObserver>; - std::unique_ptr<resource_coordinator::ResourceCoordinatorInterface> + std::unique_ptr<resource_coordinator::PageResourceCoordinator> page_resource_coordinator_; ukm::SourceId ukm_source_id_ = ukm::kInvalidSourceId;
diff --git a/chrome/gpu/BUILD.gn b/chrome/gpu/BUILD.gn index 93bdd8a..4581690 100644 --- a/chrome/gpu/BUILD.gn +++ b/chrome/gpu/BUILD.gn
@@ -27,6 +27,10 @@ "gpu_arc_video_decode_accelerator.h", "gpu_arc_video_encode_accelerator.cc", "gpu_arc_video_encode_accelerator.h", + "protected_buffer_manager.cc", + "protected_buffer_manager.h", + "protected_buffer_manager_proxy.cc", + "protected_buffer_manager_proxy.h", ] } }
diff --git a/chrome/gpu/arc_video_decode_accelerator.h b/chrome/gpu/arc_video_decode_accelerator.h index c56d5df..bd1104e2 100644 --- a/chrome/gpu/arc_video_decode_accelerator.h +++ b/chrome/gpu/arc_video_decode_accelerator.h
@@ -72,6 +72,8 @@ struct Config { size_t num_input_buffers = 0; uint32_t input_pixel_format = 0; + // If true, only buffers created via AllocateProtectedBuffer() may be used. + bool secure_mode = false; // TODO(owenlin): Add output_pixel_format. For now only the native pixel // format of each VDA on Chromium is supported. }; @@ -111,10 +113,27 @@ // returns SUCCESS iff initialization is successful. virtual Result Initialize(const Config& config, Client* client) = 0; + // Allocates a new protected buffer on accelerator side for the given |port| + // and |index|, the contents of which will be inaccessible to the client. + // The protected buffer will remain valid for at least as long as the resource + // backing the passed |handle_fd| is not released (i.e. there is at least one + // reference on the file backing |handle_fd|. + // + // Usable only if the accelerator has been initialized to run in secure mode. + // Allocation for input will create a protected buffer of at least |size|; + // for output, |size| is ignored, and the currently configured output format + // is used instead to determine the required buffer size and format. + virtual bool AllocateProtectedBuffer(PortType port, + uint32_t index, + base::ScopedFD handle_fd, + size_t size) = 0; + // Assigns a shared memory to be used for the accelerator at the specified // port and index. A buffer must be successfully bound before it can be passed // to the accelerator via UseBuffer(). Already bound buffers may be reused // multiple times without additional bindings. + // Not allowed in secure_mode, where protected buffers have to be allocated + // instead. virtual void BindSharedMemory(PortType port, uint32_t index, base::ScopedFD ashmem_fd, @@ -125,6 +144,8 @@ // port and index. A buffer must be successfully bound before it can be // passed to the accelerator via UseBuffer(). Already bound buffers may be // reused multiple times without additional bindings. + // Not allowed in secure_mode, where protected buffers have to be allocated + // instead. virtual void BindDmabuf( PortType port, uint32_t index, @@ -134,6 +155,8 @@ // Passes a buffer to the accelerator. For input buffer, the accelerator // will process it. For output buffer, the accelerator will output content // to it. + // In secure mode, |port| and |index| must correspond to a protected buffer + // allocated using AllocateProtectedBuffer(). virtual void UseBuffer(PortType port, uint32_t index, const BufferMetadata& metadata) = 0;
diff --git a/chrome/gpu/chrome_arc_video_decode_accelerator.cc b/chrome/gpu/chrome_arc_video_decode_accelerator.cc index 16715b663..59617a5 100644 --- a/chrome/gpu/chrome_arc_video_decode_accelerator.cc +++ b/chrome/gpu/chrome_arc_video_decode_accelerator.cc
@@ -10,7 +10,9 @@ #include "base/numerics/safe_math.h" #include "base/run_loop.h" #include "base/unguessable_token.h" +#include "chrome/gpu/protected_buffer_manager.h" #include "media/base/video_frame.h" +#include "media/gpu/format_utils.h" #include "media/gpu/gpu_video_decode_accelerator_factory.h" namespace chromeos { @@ -41,27 +43,33 @@ ChromeArcVideoDecodeAccelerator::InputBufferInfo::InputBufferInfo() = default; -ChromeArcVideoDecodeAccelerator::InputBufferInfo::InputBufferInfo( - InputBufferInfo&& other) = default; - -ChromeArcVideoDecodeAccelerator::InputBufferInfo::~InputBufferInfo() = default; +ChromeArcVideoDecodeAccelerator::InputBufferInfo::~InputBufferInfo() { + if (shm_handle.OwnershipPassesToIPC()) + shm_handle.Close(); +} ChromeArcVideoDecodeAccelerator::OutputBufferInfo::OutputBufferInfo() = default; -ChromeArcVideoDecodeAccelerator::OutputBufferInfo::OutputBufferInfo( - OutputBufferInfo&& other) = default; - -ChromeArcVideoDecodeAccelerator::OutputBufferInfo::~OutputBufferInfo() = - default; +ChromeArcVideoDecodeAccelerator::OutputBufferInfo::~OutputBufferInfo() { + if (!gpu_memory_buffer_handle.is_null()) { + for (const auto& fd : gpu_memory_buffer_handle.native_pixmap_handle.fds) { + // Close the fd by wrapping it in a ScopedFD and letting + // it fall out of scope. + base::ScopedFD scoped_fd(fd.fd); + } + } +} ChromeArcVideoDecodeAccelerator::ChromeArcVideoDecodeAccelerator( - const gpu::GpuPreferences& gpu_preferences) + const gpu::GpuPreferences& gpu_preferences, + ProtectedBufferManager* protected_buffer_manager) : arc_client_(nullptr), next_bitstream_buffer_id_(0), output_pixel_format_(media::PIXEL_FORMAT_UNKNOWN), output_buffer_size_(0), requested_num_of_output_buffers_(0), - gpu_preferences_(gpu_preferences) {} + gpu_preferences_(gpu_preferences), + protected_buffer_manager_(protected_buffer_manager) {} ChromeArcVideoDecodeAccelerator::~ChromeArcVideoDecodeAccelerator() { DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); @@ -95,13 +103,20 @@ return ILLEGAL_STATE; } + arc_client_ = client; + if (client_count_ >= kMaxConcurrentClients) { LOG(WARNING) << "Reject to Initialize() due to too many clients: " << client_count_; return INSUFFICIENT_RESOURCES; } - arc_client_ = client; + if (config.secure_mode && !protected_buffer_manager_) { + DLOG(ERROR) << "Secure mode unsupported"; + return PLATFORM_FAILURE; + } + + secure_mode_ = config.secure_mode; if (config.num_input_buffers > kMaxBufferCount) { DLOG(ERROR) << "Request too many buffers: " << config.num_input_buffers; @@ -168,6 +183,60 @@ buffers_pending_import_.resize(number); } +bool ChromeArcVideoDecodeAccelerator::AllocateProtectedBuffer( + PortType port, + uint32_t index, + base::ScopedFD handle_fd, + size_t size) { + DVLOG(5) << "port=" << port << " index=" << index + << " handle=" << handle_fd.get() << " size=" << size; + DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); + + if (!secure_mode_) { + DLOG(ERROR) << "Not in secure mode"; + arc_client_->OnError(INVALID_ARGUMENT); + return false; + } + + if (!ValidatePortAndIndex(port, index)) { + arc_client_->OnError(INVALID_ARGUMENT); + return false; + } + + if (port == PORT_INPUT) { + auto protected_shmem = + protected_buffer_manager_->AllocateProtectedSharedMemory( + std::move(handle_fd), size); + if (!protected_shmem) { + DLOG(ERROR) << "Failed allocating protected shared memory"; + return false; + } + + auto input_info = std::make_unique<InputBufferInfo>(); + input_info->shm_handle = protected_shmem->shm_handle(); + input_info->protected_buffer_handle = std::move(protected_shmem); + input_buffer_info_[index] = std::move(input_info); + } else if (port == PORT_OUTPUT) { + auto protected_pixmap = + protected_buffer_manager_->AllocateProtectedNativePixmap( + std::move(handle_fd), + media::VideoPixelFormatToGfxBufferFormat(output_pixel_format_), + coded_size_); + if (!protected_pixmap) { + DLOG(ERROR) << "Failed allocating a protected pixmap"; + return false; + } + auto output_info = std::make_unique<OutputBufferInfo>(); + output_info->gpu_memory_buffer_handle.type = gfx::NATIVE_PIXMAP; + output_info->gpu_memory_buffer_handle.native_pixmap_handle = + CloneHandleForIPC(protected_pixmap->native_pixmap_handle()); + output_info->protected_buffer_handle = std::move(protected_pixmap); + buffers_pending_import_[index] = std::move(output_info); + } + + return true; +} + void ChromeArcVideoDecodeAccelerator::BindSharedMemory(PortType port, uint32_t index, base::ScopedFD ashmem_fd, @@ -176,6 +245,13 @@ DVLOG(5) << "ArcGVDA::BindSharedMemory, offset: " << offset << ", length: " << length; DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); + + if (secure_mode_) { + DLOG(ERROR) << "not allowed in secure mode"; + arc_client_->OnError(INVALID_ARGUMENT); + return; + } + if (!vda_) { DLOG(ERROR) << "VDA not initialized"; return; @@ -190,10 +266,14 @@ arc_client_->OnError(INVALID_ARGUMENT); return; } - InputBufferInfo* input_info = &input_buffer_info_[index]; - input_info->handle = std::move(ashmem_fd); + + auto input_info = std::make_unique<InputBufferInfo>(); + input_info->shm_handle = + base::SharedMemoryHandle(base::FileDescriptor(ashmem_fd.release(), true), + length, base::UnguessableToken::Create()); + DCHECK(input_info->shm_handle.OwnershipPassesToIPC()); input_info->offset = offset; - input_info->length = length; + input_buffer_info_[index] = std::move(input_info); } bool ChromeArcVideoDecodeAccelerator::VerifyDmabuf( @@ -241,6 +321,12 @@ const std::vector<::arc::VideoFramePlane>& planes) { DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); + if (secure_mode_) { + DLOG(ERROR) << "not allowed in secure mode"; + arc_client_->OnError(INVALID_ARGUMENT); + return; + } + if (!vda_) { DLOG(ERROR) << "VDA not initialized"; return; @@ -260,9 +346,19 @@ return; } - OutputBufferInfo& info = buffers_pending_import_[index]; - info.handle = std::move(dmabuf_fd); - info.planes = planes; +#if defined(USE_OZONE) + auto output_info = std::make_unique<OutputBufferInfo>(); + output_info->gpu_memory_buffer_handle.type = gfx::NATIVE_PIXMAP; + output_info->gpu_memory_buffer_handle.native_pixmap_handle.fds.emplace_back( + base::FileDescriptor(dmabuf_fd.release(), true)); + for (const auto& plane : planes) { + output_info->gpu_memory_buffer_handle.native_pixmap_handle.planes + .emplace_back(plane.stride, plane.offset, 0, 0); + } + buffers_pending_import_[index] = std::move(output_info); +#else + arc_client_->OnError(PLATFORM_FAILURE); +#endif } void ChromeArcVideoDecodeAccelerator::UseBuffer( @@ -283,41 +379,44 @@ } switch (port) { case PORT_INPUT: { - InputBufferInfo* input_info = &input_buffer_info_[index]; + auto& input_info = input_buffer_info_[index]; + if (!input_info) { + DLOG(ERROR) << "Buffer not initialized"; + arc_client_->OnError(INVALID_ARGUMENT); + return; + } + int32_t bitstream_buffer_id = next_bitstream_buffer_id_; // Mask against 30 bits, to avoid (undefined) wraparound on signed // integer. next_bitstream_buffer_id_ = (next_bitstream_buffer_id_ + 1) & 0x3FFFFFFF; - int dup_fd = HANDLE_EINTR(dup(input_info->handle.get())); - if (dup_fd < 0) { - DLOG(ERROR) << "dup() failed."; + + auto duplicated_handle = input_info->shm_handle.Duplicate(); + if (!duplicated_handle.IsValid()) { arc_client_->OnError(PLATFORM_FAILURE); return; } + CreateInputRecord(bitstream_buffer_id, index, metadata.timestamp); - base::UnguessableToken guid = base::UnguessableToken::Create(); - vda_->Decode(media::BitstreamBuffer( - bitstream_buffer_id, - base::SharedMemoryHandle(base::FileDescriptor(dup_fd, true), 0u, - guid), - metadata.bytes_used, input_info->offset)); + vda_->Decode( + media::BitstreamBuffer(bitstream_buffer_id, duplicated_handle, + metadata.bytes_used, input_info->offset)); break; } case PORT_OUTPUT: { - // is_valid() is true for the first time the buffer is passed to the VDA. + auto& output_info = buffers_pending_import_[index]; + if (!output_info) { + DLOG(ERROR) << "Buffer not initialized"; + arc_client_->OnError(INVALID_ARGUMENT); + return; + } + // is_null() is false the first time the buffer is passed to the VDA. // In that case, VDA needs to import the buffer first. - OutputBufferInfo& info = buffers_pending_import_[index]; - if (info.handle.is_valid()) { - gfx::GpuMemoryBufferHandle handle; -#if defined(USE_OZONE) - handle.native_pixmap_handle.fds.emplace_back( - base::FileDescriptor(info.handle.release(), true)); - for (const auto& plane : info.planes) { - handle.native_pixmap_handle.planes.emplace_back(plane.stride, - plane.offset, 0, 0); - } -#endif - vda_->ImportBufferForPicture(index, handle); + if (!output_info->gpu_memory_buffer_handle.is_null()) { + vda_->ImportBufferForPicture(index, + output_info->gpu_memory_buffer_handle); + // VDA takes ownership, so just clear out, don't close the handle. + output_info->gpu_memory_buffer_handle = gfx::GpuMemoryBufferHandle(); } else { vda_->ReusePictureBuffer(index); }
diff --git a/chrome/gpu/chrome_arc_video_decode_accelerator.h b/chrome/gpu/chrome_arc_video_decode_accelerator.h index a520398..2da28c2 100644 --- a/chrome/gpu/chrome_arc_video_decode_accelerator.h +++ b/chrome/gpu/chrome_arc_video_decode_accelerator.h
@@ -19,6 +19,9 @@ namespace chromeos { namespace arc { +class ProtectedBufferManager; +class ProtectedBufferHandle; + // This class is executed in the GPU process. It takes decoding requests from // ARC via IPC channels and translates and sends those requests to an // implementation of media::VideoDecodeAccelerator. It also returns the decoded @@ -28,8 +31,9 @@ public media::VideoDecodeAccelerator::Client, public base::SupportsWeakPtr<ChromeArcVideoDecodeAccelerator> { public: - explicit ChromeArcVideoDecodeAccelerator( - const gpu::GpuPreferences& gpu_preferences); + ChromeArcVideoDecodeAccelerator( + const gpu::GpuPreferences& gpu_preferences, + ProtectedBufferManager* protected_buffer_manager); ~ChromeArcVideoDecodeAccelerator() override; // Implementation of the ArcVideoDecodeAccelerator interface. @@ -37,6 +41,10 @@ const Config& config, ArcVideoDecodeAccelerator::Client* client) override; void SetNumberOfOutputBuffers(size_t number) override; + bool AllocateProtectedBuffer(PortType port, + uint32_t index, + base::ScopedFD handle_fd, + size_t size) override; void BindSharedMemory(PortType port, uint32_t index, base::ScopedFD ashmem_fd, @@ -80,28 +88,34 @@ // The information about the shared memory used as an input buffer. struct InputBufferInfo { - // The file handle to access the buffer. It is owned by this class and - // should be closed after use. - base::ScopedFD handle; + // SharedMemoryHandle for this buffer to be passed to accelerator. + // In non-secure mode, received via BindSharedMemory from the client, + // in secure mode, a handle for the SharedMemory in protected_shmem. + base::SharedMemoryHandle shm_handle; - // The offset of the payload to the beginning of the shared memory. + // Used only in secure mode; handle to the protected buffer backing + // this input buffer. + std::unique_ptr<ProtectedBufferHandle> protected_buffer_handle; + + // Offset to the payload from the beginning of the shared memory buffer. off_t offset = 0; - // The size of the payload in bytes. - size_t length = 0; - InputBufferInfo(); - InputBufferInfo(InputBufferInfo&& other); ~InputBufferInfo(); }; - // The information about the dmabuf used as an output buffer. + // The information about the native pixmap used as an output buffer. struct OutputBufferInfo { - base::ScopedFD handle; - std::vector<::arc::VideoFramePlane> planes; + // GpuMemoryBufferHandle for this buffer to be passed to accelerator. + // In non-secure mode, received via BindDmabuf from the client, + // in secure mode, a handle to the NativePixmap in protected_pixmap. + gfx::GpuMemoryBufferHandle gpu_memory_buffer_handle; + + // Used only in secure mode; handle to the protected buffer backing + // this output buffer. + std::unique_ptr<ProtectedBufferHandle> protected_buffer_handle; OutputBufferInfo(); - OutputBufferInfo(OutputBufferInfo&& other); ~OutputBufferInfo(); }; @@ -155,12 +169,12 @@ std::list<InputRecord> input_records_; // The details of the shared memory of each input buffers. - std::vector<InputBufferInfo> input_buffer_info_; + std::vector<std::unique_ptr<InputBufferInfo>> input_buffer_info_; // To keep those output buffers which have been bound by bindDmabuf() but // haven't been passed to VDA yet. Will call VDA::ImportBufferForPicture() // when those buffers are used for the first time. - std::vector<OutputBufferInfo> buffers_pending_import_; + std::vector<std::unique_ptr<OutputBufferInfo>> buffers_pending_import_; THREAD_CHECKER(thread_checker_); size_t output_buffer_size_; @@ -168,7 +182,10 @@ // The minimal number of requested output buffers. uint32_t requested_num_of_output_buffers_; + bool secure_mode_ = false; + gpu::GpuPreferences gpu_preferences_; + ProtectedBufferManager* protected_buffer_manager_; DISALLOW_COPY_AND_ASSIGN(ChromeArcVideoDecodeAccelerator); };
diff --git a/chrome/gpu/chrome_content_gpu_client.cc b/chrome/gpu/chrome_content_gpu_client.cc index bcd0060..ae7e058c 100644 --- a/chrome/gpu/chrome_content_gpu_client.cc +++ b/chrome/gpu/chrome_content_gpu_client.cc
@@ -22,8 +22,14 @@ #if defined(OS_CHROMEOS) #include "chrome/gpu/gpu_arc_video_decode_accelerator.h" #include "chrome/gpu/gpu_arc_video_encode_accelerator.h" +#include "chrome/gpu/protected_buffer_manager.h" +#include "chrome/gpu/protected_buffer_manager_proxy.h" #include "content/public/common/service_manager_connection.h" #include "services/service_manager/public/cpp/binder_registry.h" +#if defined(USE_OZONE) +#include "ui/ozone/public/ozone_platform.h" +#include "ui/ozone/public/surface_factory_ozone.h" +#endif #endif namespace { @@ -46,6 +52,10 @@ metrics::CallStackProfileParams::MAY_SHUFFLE))) { if (StackSamplingConfiguration::Get()->IsProfilerEnabledForCurrentProcess()) stack_sampling_profiler_.Start(); + +#if defined(OS_CHROMEOS) + protected_buffer_manager_.reset(new chromeos::arc::ProtectedBufferManager()); +#endif } ChromeContentGpuClient::~ChromeContentGpuClient() {} @@ -61,6 +71,10 @@ base::Bind(&ChromeContentGpuClient::CreateArcVideoEncodeAccelerator, base::Unretained(this)), base::ThreadTaskRunnerHandle::Get()); + registry->AddInterface( + base::Bind(&ChromeContentGpuClient::CreateProtectedBufferManager, + base::Unretained(this)), + base::ThreadTaskRunnerHandle::Get()); #endif } @@ -68,6 +82,13 @@ const gpu::GpuPreferences& gpu_preferences) { #if defined(OS_CHROMEOS) gpu_preferences_ = gpu_preferences; +#if defined(USE_OZONE) + ui::OzonePlatform::GetInstance() + ->GetSurfaceFactoryOzone() + ->SetGetProtectedNativePixmapDelegate(base::Bind( + &chromeos::arc::ProtectedBufferManager::GetProtectedNativePixmapFor, + base::Unretained(protected_buffer_manager_.get()))); +#endif #endif metrics::mojom::CallStackProfileCollectorPtr browser_interface; @@ -78,12 +99,11 @@ } #if defined(OS_CHROMEOS) - void ChromeContentGpuClient::CreateArcVideoDecodeAccelerator( ::arc::mojom::VideoDecodeAcceleratorRequest request) { mojo::MakeStrongBinding( base::MakeUnique<chromeos::arc::GpuArcVideoDecodeAccelerator>( - gpu_preferences_), + gpu_preferences_, protected_buffer_manager_.get()), std::move(request)); } @@ -94,4 +114,12 @@ gpu_preferences_), std::move(request)); } + +void ChromeContentGpuClient::CreateProtectedBufferManager( + ::arc::mojom::ProtectedBufferManagerRequest request) { + mojo::MakeStrongBinding( + base::MakeUnique<chromeos::arc::GpuArcProtectedBufferManagerProxy>( + protected_buffer_manager_.get()), + std::move(request)); +} #endif
diff --git a/chrome/gpu/chrome_content_gpu_client.h b/chrome/gpu/chrome_content_gpu_client.h index 78abc0e..bbc731f 100644 --- a/chrome/gpu/chrome_content_gpu_client.h +++ b/chrome/gpu/chrome_content_gpu_client.h
@@ -12,10 +12,16 @@ #include "content/public/gpu/content_gpu_client.h" #if defined(OS_CHROMEOS) +#include "components/arc/common/protected_buffer_manager.mojom.h" #include "components/arc/common/video_decode_accelerator.mojom.h" #include "components/arc/common/video_encode_accelerator.mojom.h" #include "gpu/command_buffer/service/gpu_preferences.h" +namespace chromeos { +namespace arc { +class ProtectedBufferManager; +} // namespace arc +} // namespace chromeos #endif class ChromeContentGpuClient : public content::ContentGpuClient { @@ -35,6 +41,9 @@ void CreateArcVideoEncodeAccelerator( ::arc::mojom::VideoEncodeAcceleratorRequest request); + + void CreateProtectedBufferManager( + ::arc::mojom::ProtectedBufferManagerRequest request); #endif // Used to profile process startup. @@ -42,6 +51,8 @@ #if defined(OS_CHROMEOS) gpu::GpuPreferences gpu_preferences_; + std::unique_ptr<chromeos::arc::ProtectedBufferManager> + protected_buffer_manager_; #endif DISALLOW_COPY_AND_ASSIGN(ChromeContentGpuClient);
diff --git a/chrome/gpu/gpu_arc_video_decode_accelerator.cc b/chrome/gpu/gpu_arc_video_decode_accelerator.cc index d53c3c0..b94aa8f 100644 --- a/chrome/gpu/gpu_arc_video_decode_accelerator.cc +++ b/chrome/gpu/gpu_arc_video_decode_accelerator.cc
@@ -98,6 +98,7 @@ chromeos::arc::ArcVideoDecodeAccelerator::Config result; result.num_input_buffers = input->num_input_buffers; result.input_pixel_format = input->input_pixel_format; + result.secure_mode = input->secure_mode; return result; } }; @@ -108,9 +109,12 @@ namespace arc { GpuArcVideoDecodeAccelerator::GpuArcVideoDecodeAccelerator( - const gpu::GpuPreferences& gpu_preferences) + const gpu::GpuPreferences& gpu_preferences, + ProtectedBufferManager* protected_buffer_manager) : gpu_preferences_(gpu_preferences), - accelerator_(new ChromeArcVideoDecodeAccelerator(gpu_preferences_)) {} + accelerator_(std::make_unique<ChromeArcVideoDecodeAccelerator>( + gpu_preferences_, + protected_buffer_manager)) {} GpuArcVideoDecodeAccelerator::~GpuArcVideoDecodeAccelerator() { DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); @@ -166,7 +170,9 @@ LOG(ERROR) << "only decoder is supported"; std::move(callback).Run( ::arc::mojom::VideoDecodeAccelerator::Result::INVALID_ARGUMENT); + return; } + client_ = std::move(client); ArcVideoDecodeAccelerator::Result result = accelerator_->Initialize( config.To<ArcVideoDecodeAccelerator::Config>(), this); @@ -197,6 +203,26 @@ return base::ScopedFD(platform_file); } +void GpuArcVideoDecodeAccelerator::AllocateProtectedBuffer( + ::arc::mojom::PortType port, + uint32_t index, + mojo::ScopedHandle handle, + uint64_t size, + AllocateProtectedBufferCallback callback) { + DVLOG(2) << "port=" << port << ", index=" << index << ", size=" << size; + + base::ScopedFD fd = UnwrapFdFromMojoHandle(std::move(handle)); + if (!fd.is_valid()) { + std::move(callback).Run(false); + return; + } + + bool result = accelerator_->AllocateProtectedBuffer( + static_cast<PortType>(port), index, std::move(fd), size); + + std::move(callback).Run(result); +} + void GpuArcVideoDecodeAccelerator::BindSharedMemory( ::arc::mojom::PortType port, uint32_t index,
diff --git a/chrome/gpu/gpu_arc_video_decode_accelerator.h b/chrome/gpu/gpu_arc_video_decode_accelerator.h index 078c17d1..daf6088 100644 --- a/chrome/gpu/gpu_arc_video_decode_accelerator.h +++ b/chrome/gpu/gpu_arc_video_decode_accelerator.h
@@ -19,6 +19,8 @@ namespace chromeos { namespace arc { +class ProtectedBufferManager; + // GpuArcVideoDecodeAccelerator manages life-cycle and IPC message translation // for ArcVideoDecodeAccelerator. // @@ -28,8 +30,9 @@ : public ::arc::mojom::VideoDecodeAccelerator, public ArcVideoDecodeAccelerator::Client { public: - explicit GpuArcVideoDecodeAccelerator( - const gpu::GpuPreferences& gpu_preferences); + GpuArcVideoDecodeAccelerator( + const gpu::GpuPreferences& gpu_preferences, + ProtectedBufferManager* protected_buffer_manager); ~GpuArcVideoDecodeAccelerator() override; private: @@ -46,6 +49,14 @@ void Initialize(::arc::mojom::VideoDecodeAcceleratorConfigPtr config, ::arc::mojom::VideoDecodeClientPtr client, InitializeCallback callback) override; + + void AllocateProtectedBuffer( + ::arc::mojom::PortType port, + uint32_t index, + mojo::ScopedHandle handle, + uint64_t size, + AllocateProtectedBufferCallback callback) override; + void BindSharedMemory(::arc::mojom::PortType port, uint32_t index, mojo::ScopedHandle ashmem_handle,
diff --git a/chrome/gpu/protected_buffer_manager.cc b/chrome/gpu/protected_buffer_manager.cc new file mode 100644 index 0000000..47bcdaf --- /dev/null +++ b/chrome/gpu/protected_buffer_manager.cc
@@ -0,0 +1,411 @@ +// 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 "chrome/gpu/protected_buffer_manager.h" + +#include "base/bits.h" +#include "base/logging.h" +#include "base/memory/ptr_util.h" +#include "base/memory/shared_memory.h" +#include "base/sys_info.h" +#include "mojo/public/cpp/system/buffer.h" +#include "mojo/public/cpp/system/platform_handle.h" +#include "ui/gfx/geometry/size.h" +#include "ui/ozone/public/ozone_platform.h" +#include "ui/ozone/public/surface_factory_ozone.h" + +#define VLOGF(level) VLOG(level) << __func__ << "(): " + +namespace chromeos { +namespace arc { + +namespace { +// Size of the pixmap to be used as the dummy handle for protected buffers. +constexpr gfx::Size kDummyBufferSize(32, 32); +} // namespace + +ProtectedBufferHandle::ProtectedBufferHandle( + base::OnceClosure destruction_cb, + const base::SharedMemoryHandle& shm_handle) + : shm_handle_(shm_handle), destruction_cb_(std::move(destruction_cb)) { + DCHECK(shm_handle_.IsValid()); + DCHECK(shm_handle.OwnershipPassesToIPC()); +} + +ProtectedBufferHandle::ProtectedBufferHandle( + base::OnceClosure destruction_cb, + const gfx::NativePixmapHandle& native_pixmap_handle) + : native_pixmap_handle_(native_pixmap_handle), + destruction_cb_(std::move(destruction_cb)) { + DCHECK(!native_pixmap_handle_.fds.empty()); + for (const auto& fd : native_pixmap_handle_.fds) + DCHECK(fd.auto_close); +} + +ProtectedBufferHandle::~ProtectedBufferHandle() { + if (shm_handle_.OwnershipPassesToIPC()) + shm_handle_.Close(); + + for (const auto& fd : native_pixmap_handle_.fds) { + // Close the fd by wrapping it in a ScopedFD and letting + // it fall out of scope. + base::ScopedFD scoped_fd(fd.fd); + } + + std::move(destruction_cb_).Run(); +} + +base::SharedMemoryHandle ProtectedBufferHandle::shm_handle() const { + base::SharedMemoryHandle handle = shm_handle_; + handle.SetOwnershipPassesToIPC(false); + return handle; +} + +gfx::NativePixmapHandle ProtectedBufferHandle::native_pixmap_handle() const { + return native_pixmap_handle_; +} + +class ProtectedBufferManager::ProtectedBuffer { + public: + virtual ~ProtectedBuffer() {} + + // Downcasting methods to return duplicated handles to the underlying + // protected buffers for each buffer type, or empty/null handles if not + // applicable. + virtual base::SharedMemoryHandle DuplicateSharedMemoryHandle() const { + return base::SharedMemoryHandle(); + } + virtual gfx::NativePixmapHandle DuplicateNativePixmapHandle() const { + return gfx::NativePixmapHandle(); + } + + // Downcasting method to return a scoped_refptr to the underlying + // NativePixmap, or null if not applicable. + virtual scoped_refptr<gfx::NativePixmap> GetNativePixmap() const { + return nullptr; + } + + protected: + explicit ProtectedBuffer(scoped_refptr<gfx::NativePixmap> dummy_handle) + : dummy_handle_(std::move(dummy_handle)) {} + + private: + scoped_refptr<gfx::NativePixmap> dummy_handle_; + + DISALLOW_COPY_AND_ASSIGN(ProtectedBuffer); +}; + +class ProtectedBufferManager::ProtectedSharedMemory + : public ProtectedBufferManager::ProtectedBuffer { + public: + ~ProtectedSharedMemory() override; + + // Allocate a ProtectedSharedMemory buffer of |size| bytes. + static std::unique_ptr<ProtectedSharedMemory> Create( + scoped_refptr<gfx::NativePixmap> dummy_handle, + size_t size); + + base::SharedMemoryHandle DuplicateSharedMemoryHandle() const override { + return base::SharedMemory::DuplicateHandle(shmem_->handle()); + } + + private: + explicit ProtectedSharedMemory(scoped_refptr<gfx::NativePixmap> dummy_handle); + + std::unique_ptr<base::SharedMemory> shmem_; +}; + +ProtectedBufferManager::ProtectedSharedMemory::ProtectedSharedMemory( + scoped_refptr<gfx::NativePixmap> dummy_handle) + : ProtectedBuffer(std::move(dummy_handle)) {} + +ProtectedBufferManager::ProtectedSharedMemory::~ProtectedSharedMemory() {} + +// static +std::unique_ptr<ProtectedBufferManager::ProtectedSharedMemory> +ProtectedBufferManager::ProtectedSharedMemory::Create( + scoped_refptr<gfx::NativePixmap> dummy_handle, + size_t size) { + std::unique_ptr<ProtectedSharedMemory> protected_shmem( + new ProtectedSharedMemory(std::move(dummy_handle))); + + size_t aligned_size = + base::bits::Align(size, base::SysInfo::VMAllocationGranularity()); + + mojo::ScopedSharedBufferHandle mojo_shared_buffer = + mojo::SharedBufferHandle::Create(aligned_size); + if (!mojo_shared_buffer->is_valid()) { + VLOGF(1) << "Failed to allocate shared memory"; + return nullptr; + } + + base::SharedMemoryHandle shm_handle; + MojoResult mojo_result = mojo::UnwrapSharedMemoryHandle( + std::move(mojo_shared_buffer), &shm_handle, nullptr, nullptr); + if (mojo_result != MOJO_RESULT_OK) { + VLOGF(1) << "Failed to unwrap a mojo shared memory handle"; + return nullptr; + } + + protected_shmem->shmem_ = + std::make_unique<base::SharedMemory>(shm_handle, false); + return protected_shmem; +} + +class ProtectedBufferManager::ProtectedNativePixmap + : public ProtectedBufferManager::ProtectedBuffer { + public: + ~ProtectedNativePixmap() override; + + // Allocate a ProtectedNativePixmap of |format| and |size|. + static std::unique_ptr<ProtectedNativePixmap> Create( + scoped_refptr<gfx::NativePixmap> dummy_handle, + gfx::BufferFormat format, + const gfx::Size& size); + + gfx::NativePixmapHandle DuplicateNativePixmapHandle() const override { + return native_pixmap_->ExportHandle(); + } + + scoped_refptr<gfx::NativePixmap> GetNativePixmap() const override { + return native_pixmap_; + } + + private: + explicit ProtectedNativePixmap(scoped_refptr<gfx::NativePixmap> dummy_handle); + + scoped_refptr<gfx::NativePixmap> native_pixmap_; +}; + +ProtectedBufferManager::ProtectedNativePixmap::ProtectedNativePixmap( + scoped_refptr<gfx::NativePixmap> dummy_handle) + : ProtectedBuffer(std::move(dummy_handle)) {} + +ProtectedBufferManager::ProtectedNativePixmap::~ProtectedNativePixmap() {} + +// static +std::unique_ptr<ProtectedBufferManager::ProtectedNativePixmap> +ProtectedBufferManager::ProtectedNativePixmap::Create( + scoped_refptr<gfx::NativePixmap> dummy_handle, + gfx::BufferFormat format, + const gfx::Size& size) { + std::unique_ptr<ProtectedNativePixmap> protected_pixmap( + new ProtectedNativePixmap(std::move(dummy_handle))); + + ui::OzonePlatform* platform = ui::OzonePlatform::GetInstance(); + ui::SurfaceFactoryOzone* factory = platform->GetSurfaceFactoryOzone(); + protected_pixmap->native_pixmap_ = + factory->CreateNativePixmap(gfx::kNullAcceleratedWidget, size, format, + gfx::BufferUsage::SCANOUT_VDA_WRITE); + + if (!protected_pixmap->native_pixmap_) { + VLOGF(1) << "Failed allocating a native pixmap"; + return nullptr; + } + + return protected_pixmap; +} + +ProtectedBufferManager::ProtectedBufferManager() : weak_factory_(this) { + VLOGF(2); + weak_this_ = weak_factory_.GetWeakPtr(); +} + +ProtectedBufferManager::~ProtectedBufferManager() { + VLOGF(2); +} + +std::unique_ptr<ProtectedBufferHandle> +ProtectedBufferManager::AllocateProtectedSharedMemory(base::ScopedFD dummy_fd, + size_t size) { + VLOGF(2) << "dummy_fd: " << dummy_fd.get() << ", size: " << size; + + // Import the |dummy_fd| to produce a unique id for it. + uint32_t id; + auto pixmap = ImportDummyFd(std::move(dummy_fd), &id); + if (!pixmap) + return nullptr; + + base::AutoLock lock(buffer_map_lock_); + + if (buffer_map_.find(id) != buffer_map_.end()) { + VLOGF(1) << "A protected buffer for this handle already exists"; + return nullptr; + } + + // Allocate a protected buffer and associate it with the dummy pixmap. + // The pixmap needs to be stored to ensure the id remains the same for + // the entire lifetime of the dummy pixmap. + auto protected_shmem = ProtectedSharedMemory::Create(pixmap, size); + if (!protected_shmem) { + VLOGF(1) << "Failed allocating a protected shared memory buffer"; + return nullptr; + } + + auto shm_handle = protected_shmem->DuplicateSharedMemoryHandle(); + if (!shm_handle.IsValid()) { + VLOGF(1) << "Failed duplicating SharedMemoryHandle"; + return nullptr; + } + + // Store the buffer in the buffer_map_, and return a handle to it to the + // client. The buffer will be permanently removed from the map when the + // handle is destroyed. + VLOGF(2) << "New protected shared memory buffer, handle id: " << id; + auto protected_buffer_handle = base::MakeUnique<ProtectedBufferHandle>( + base::BindOnce(&ProtectedBufferManager::RemoveEntry, weak_this_, id), + shm_handle); + + // This will always succeed as we find() first above. + buffer_map_.emplace(id, std::move(protected_shmem)); + + return protected_buffer_handle; +} + +std::unique_ptr<ProtectedBufferHandle> +ProtectedBufferManager::AllocateProtectedNativePixmap(base::ScopedFD dummy_fd, + gfx::BufferFormat format, + const gfx::Size& size) { + VLOGF(2) << "dummy_fd: " << dummy_fd.get() << ", format: " << (int)format + << ", size: " << size.ToString(); + + // Import the |dummy_fd| to produce a unique id for it. + uint32_t id = 0; + auto pixmap = ImportDummyFd(std::move(dummy_fd), &id); + if (!pixmap) + return nullptr; + + base::AutoLock lock(buffer_map_lock_); + + if (buffer_map_.find(id) != buffer_map_.end()) { + VLOGF(1) << "A protected buffer for this handle already exists"; + return nullptr; + } + + // Allocate a protected buffer and associate it with the dummy pixmap. + // The pixmap needs to be stored to ensure the id remains the same for + // the entire lifetime of the dummy pixmap. + auto protected_pixmap = ProtectedNativePixmap::Create(pixmap, format, size); + if (!protected_pixmap) { + VLOGF(1) << "Failed allocating a protected native pixmap"; + return nullptr; + } + + auto native_pixmap_handle = protected_pixmap->DuplicateNativePixmapHandle(); + if (native_pixmap_handle.planes.empty()) { + VLOGF(1) << "Failed duplicating NativePixmapHandle"; + return nullptr; + } + + // Store the buffer in the buffer_map_, and return a handle to it to the + // client. The buffer will be permanently removed from the map when the + // handle is destroyed. + VLOGF(2) << "New protected native pixmap, handle id: " << id; + auto protected_buffer_handle = base::MakeUnique<ProtectedBufferHandle>( + base::BindOnce(&ProtectedBufferManager::RemoveEntry, weak_this_, id), + native_pixmap_handle); + + // This will always succeed as we find() first above. + buffer_map_.emplace(id, std::move(protected_pixmap)); + + return protected_buffer_handle; +} + +base::SharedMemoryHandle +ProtectedBufferManager::GetProtectedSharedMemoryHandleFor( + base::ScopedFD dummy_fd) { + uint32_t id = 0; + auto pixmap = ImportDummyFd(std::move(dummy_fd), &id); + + base::AutoLock lock(buffer_map_lock_); + const auto& iter = buffer_map_.find(id); + if (iter == buffer_map_.end()) + return base::SharedMemoryHandle(); + + return iter->second->DuplicateSharedMemoryHandle(); +} + +gfx::NativePixmapHandle +ProtectedBufferManager::GetProtectedNativePixmapHandleFor( + base::ScopedFD dummy_fd) { + uint32_t id = 0; + auto pixmap = ImportDummyFd(std::move(dummy_fd), &id); + + base::AutoLock lock(buffer_map_lock_); + const auto& iter = buffer_map_.find(id); + if (iter == buffer_map_.end()) + return gfx::NativePixmapHandle(); + + return iter->second->DuplicateNativePixmapHandle(); +} + +scoped_refptr<gfx::NativePixmap> +ProtectedBufferManager::GetProtectedNativePixmapFor( + const gfx::NativePixmapHandle& handle) { + // Only the first fd is used for lookup. + if (handle.fds.empty()) + return nullptr; + + base::ScopedFD dummy_fd(HANDLE_EINTR(dup(handle.fds[0].fd))); + uint32_t id = 0; + auto pixmap = ImportDummyFd(std::move(dummy_fd), &id); + + base::AutoLock lock(buffer_map_lock_); + const auto& iter = buffer_map_.find(id); + if (iter == buffer_map_.end()) + return nullptr; + + auto native_pixmap = iter->second->GetNativePixmap(); + if (native_pixmap) { + for (const auto& fd : handle.fds) + base::ScopedFD scoped_fd(fd.fd); + } + + return native_pixmap; +} + +scoped_refptr<gfx::NativePixmap> ProtectedBufferManager::ImportDummyFd( + base::ScopedFD dummy_fd, + uint32_t* id) const { + // 0 is an invalid handle id. + *id = 0; + + // Import dummy_fd to acquire its unique id. + // CreateNativePixmapFromHandle() takes ownership and will close the handle + // also on failure. + gfx::NativePixmapHandle pixmap_handle; + pixmap_handle.fds.emplace_back( + base::FileDescriptor(dummy_fd.release(), true)); + pixmap_handle.planes.emplace_back(gfx::NativePixmapPlane()); + ui::OzonePlatform* platform = ui::OzonePlatform::GetInstance(); + ui::SurfaceFactoryOzone* factory = platform->GetSurfaceFactoryOzone(); + scoped_refptr<gfx::NativePixmap> pixmap = + factory->CreateNativePixmapForProtectedBufferHandle( + gfx::kNullAcceleratedWidget, kDummyBufferSize, gfx::BufferFormat::R_8, + pixmap_handle); + if (!pixmap) { + VLOGF(1) << "Failed importing dummy handle"; + return nullptr; + } + + *id = pixmap->GetUniqueId(); + if (*id == 0) { + VLOGF(1) << "Failed acquiring unique id for handle"; + return nullptr; + } + + return pixmap; +} + +void ProtectedBufferManager::RemoveEntry(uint32_t id) { + VLOGF(2) << "id: " << id; + + base::AutoLock lock(buffer_map_lock_); + auto num_erased = buffer_map_.erase(id); + if (num_erased != 1) + VLOGF(1) << "No buffer id " << id << " to destroy"; +} + +} // namespace arc +} // namespace chromeos
diff --git a/chrome/gpu/protected_buffer_manager.h b/chrome/gpu/protected_buffer_manager.h new file mode 100644 index 0000000..40a5e01 --- /dev/null +++ b/chrome/gpu/protected_buffer_manager.h
@@ -0,0 +1,145 @@ +// 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 CHROME_GPU_PROTECTED_BUFFER_MANAGER_H_ +#define CHROME_GPU_PROTECTED_BUFFER_MANAGER_H_ + +#include <map> + +#include "base/memory/ref_counted.h" +#include "base/memory/shared_memory.h" +#include "base/memory/weak_ptr.h" +#include "base/synchronization/lock.h" +#include "ui/gfx/gpu_memory_buffer.h" +#include "ui/gfx/native_pixmap.h" + +namespace chromeos { +namespace arc { + +// A ProtectedBufferHandle is returned to the owning client that requested +// the underlying ProtectedBuffer to be allocated. +// +// A ProtectedBuffer is a buffer that can be referred to via a handle (a dummy +// handle), which does not provide access to the actual contents of the buffer. +// +// The client should release this handle once the buffer is no longer needed. +// Releasing triggers destruction of the ProtectedBuffer instance stored in +// the ProtectedBufferManager, via the destruction callback passed to the +// ProtectedBufferHandle's constructor. +class ProtectedBufferHandle { + public: + // ProtectedBufferHandle takes ownership of the passed |shm_handle|. + ProtectedBufferHandle(base::OnceClosure destruction_cb, + const base::SharedMemoryHandle& shm_handle); + + // ProtectedBufferHandle takes ownership of the passed |native_pixmap_handle|. + ProtectedBufferHandle(base::OnceClosure destruction_cb, + const gfx::NativePixmapHandle& native_pixmap_handle); + + // Closes the underlying handle. + ~ProtectedBufferHandle(); + + // Return a non-owned SharedMemoryHandle or NativePixmapHandle for this + // ProtectedBufferHandle, or an invalid/null handle if not applicable for the + // underlying type. + base::SharedMemoryHandle shm_handle() const; + gfx::NativePixmapHandle native_pixmap_handle() const; + + private: + // The underlying, owning handles to the protected buffer. + // Only one of the handles is valid for each instance of this class. + // Closed on destruction of this ProtectedBufferHandle. + base::SharedMemoryHandle shm_handle_; + gfx::NativePixmapHandle native_pixmap_handle_; + + base::OnceClosure destruction_cb_; +}; + +class ProtectedBufferManager { + public: + ProtectedBufferManager(); + ~ProtectedBufferManager(); + + // Allocate a ProtectedSharedMemory buffer of |size| bytes, to be referred to + // via |dummy_fd| as the dummy handle, returning a ProtectedBufferHandle to + // it. + // Destroying the ProtectedBufferHandle will result in permanently + // disassociating the |dummy_fd| with the underlying ProtectedBuffer, but may + // not free the underlying protected memory, which will remain valid as long + // as any SharedMemoryHandles to it are still in use. + // Return nullptr on failure. + std::unique_ptr<ProtectedBufferHandle> AllocateProtectedSharedMemory( + base::ScopedFD dummy_fd, + size_t size); + + // Allocate a ProtectedNativePixmap of |format| and |size|, to be referred to + // via |dummy_fd| as the dummy handle, returning a ProtectedBufferHandle to + // it. + // Destroying the ProtectedBufferHandle will result in permanently + // disassociating the |dummy_fd| with the underlying ProtectedBuffer, but may + // not free the underlying protected memory, which will remain valid as long + // as any NativePixmapHandles to it are still in use. + // Return nullptr on failure. + std::unique_ptr<ProtectedBufferHandle> AllocateProtectedNativePixmap( + base::ScopedFD dummy_fd, + gfx::BufferFormat format, + const gfx::Size& size); + + // Return a duplicated SharedMemoryHandle associated with the |dummy_fd|, + // if one exists, or an invalid handle otherwise. + // The client is responsible for closing the handle after use. + base::SharedMemoryHandle GetProtectedSharedMemoryHandleFor( + base::ScopedFD dummy_fd); + + // Return a duplicated NativePixmapHandle associated with the |dummy_fd|, + // if one exists, or an empty handle otherwise. + // The client is responsible for closing the handle after use. + gfx::NativePixmapHandle GetProtectedNativePixmapHandleFor( + base::ScopedFD dummy_fd); + + // Return a protected NativePixmap for a dummy |handle|, if one exists, or + // nullptr otherwise. On success, the |handle| is closed. + scoped_refptr<gfx::NativePixmap> GetProtectedNativePixmapFor( + const gfx::NativePixmapHandle& handle); + + private: + // Used internally to maintain the association between the dummy handle and + // the underlying buffer. + class ProtectedBuffer; + class ProtectedSharedMemory; + class ProtectedNativePixmap; + + // Imports the |dummy_fd| as a NativePixmap. This returns a unique |id|, + // which is guaranteed to be the same for all future imports of any fd + // referring to the buffer to which |dummy_fd| refers to, regardless of + // whether it is the same fd as the original one, or not, for the lifetime + // of the buffer. + // + // This allows us to have an unambiguous mapping from any fd referring to + // the same memory buffer to the same unique id. + // + // Returns nullptr on failure, in which case the returned id is not valid. + scoped_refptr<gfx::NativePixmap> ImportDummyFd(base::ScopedFD dummy_fd, + uint32_t* id) const; + + // Removes an entry for given |id| from buffer_map_, to be called when the + // last reference to the buffer is dropped. + void RemoveEntry(uint32_t id); + + // A map of unique ids to the ProtectedBuffers associated with them. + using ProtectedBufferMap = + std::map<uint32_t, std::unique_ptr<ProtectedBuffer>>; + ProtectedBufferMap buffer_map_; + base::Lock buffer_map_lock_; + + base::WeakPtr<ProtectedBufferManager> weak_this_; + base::WeakPtrFactory<ProtectedBufferManager> weak_factory_; + + DISALLOW_COPY_AND_ASSIGN(ProtectedBufferManager); +}; + +} // namespace arc +} // namespace chromeos + +#endif // CHROME_GPU_PROTECTED_BUFFER_MANAGER_H_
diff --git a/chrome/gpu/protected_buffer_manager_proxy.cc b/chrome/gpu/protected_buffer_manager_proxy.cc new file mode 100644 index 0000000..8080ed1 --- /dev/null +++ b/chrome/gpu/protected_buffer_manager_proxy.cc
@@ -0,0 +1,53 @@ +// 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 "chrome/gpu/protected_buffer_manager_proxy.h" + +#include "chrome/gpu/protected_buffer_manager.h" +#include "mojo/public/cpp/system/platform_handle.h" + +#define VLOGF(level) VLOG(level) << __func__ << "(): " + +namespace chromeos { +namespace arc { + +GpuArcProtectedBufferManagerProxy::GpuArcProtectedBufferManagerProxy( + chromeos::arc::ProtectedBufferManager* protected_buffer_manager) + : protected_buffer_manager_(protected_buffer_manager) { + DCHECK(protected_buffer_manager_); +} + +base::ScopedFD GpuArcProtectedBufferManagerProxy::UnwrapFdFromMojoHandle( + mojo::ScopedHandle handle) { + base::PlatformFile platform_file; + MojoResult mojo_result = + mojo::UnwrapPlatformFile(std::move(handle), &platform_file); + if (mojo_result != MOJO_RESULT_OK) { + VLOGF(1) << "UnwrapPlatformFile failed: " << mojo_result; + return base::ScopedFD(); + } + + return base::ScopedFD(platform_file); +} + +mojo::ScopedHandle GpuArcProtectedBufferManagerProxy::WrapFdInMojoHandle( + base::ScopedFD fd) { + return mojo::WrapPlatformFile(fd.release()); +} + +void GpuArcProtectedBufferManagerProxy::GetProtectedSharedMemoryFromHandle( + mojo::ScopedHandle dummy_handle, + GetProtectedSharedMemoryFromHandleCallback callback) { + base::ScopedFD unwrapped_fd = UnwrapFdFromMojoHandle(std::move(dummy_handle)); + + base::ScopedFD shmem_fd( + protected_buffer_manager_ + ->GetProtectedSharedMemoryHandleFor(std::move(unwrapped_fd)) + .Release()); + + std::move(callback).Run(WrapFdInMojoHandle(std::move(shmem_fd))); +} + +} // namespace arc +} // namespace chromeos
diff --git a/chrome/gpu/protected_buffer_manager_proxy.h b/chrome/gpu/protected_buffer_manager_proxy.h new file mode 100644 index 0000000..324065c --- /dev/null +++ b/chrome/gpu/protected_buffer_manager_proxy.h
@@ -0,0 +1,39 @@ +// 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 CHROME_GPU_PROTECTED_BUFFER_MANAGER_PROXY_H_ +#define CHROME_GPU_PROTECTED_BUFFER_MANAGER_PROXY_H_ + +#include "components/arc/common/protected_buffer_manager.mojom.h" + +namespace chromeos { +namespace arc { + +class ProtectedBufferManager; + +// Manages mojo IPC translation for chromeos::arc::ProtectedBufferManager. +class GpuArcProtectedBufferManagerProxy + : public ::arc::mojom::ProtectedBufferManager { + public: + explicit GpuArcProtectedBufferManagerProxy( + chromeos::arc::ProtectedBufferManager* protected_buffer_manager); + + // arc::mojom::ProtectedBufferManager implementation. + void GetProtectedSharedMemoryFromHandle( + mojo::ScopedHandle dummy_handle, + GetProtectedSharedMemoryFromHandleCallback callback) override; + + private: + base::ScopedFD UnwrapFdFromMojoHandle(mojo::ScopedHandle handle); + mojo::ScopedHandle WrapFdInMojoHandle(base::ScopedFD fd); + + chromeos::arc::ProtectedBufferManager* protected_buffer_manager_; + + DISALLOW_COPY_AND_ASSIGN(GpuArcProtectedBufferManagerProxy); +}; + +} // namespace arc +} // namespace chromeos + +#endif // CHROME_GPU_PROTECTED_BUFFER_MANAGER_PROXY_H_
diff --git a/components/arc/common/oemcrypto_daemon.mojom b/components/arc/common/oemcrypto_daemon.mojom index 5b2b782..b75db2d 100644 --- a/components/arc/common/oemcrypto_daemon.mojom +++ b/components/arc/common/oemcrypto_daemon.mojom
@@ -21,5 +21,5 @@ // Next Method ID: 1 interface OemCryptoHostDaemon { Connect@0(arc.mojom.OemCryptoService& oemcryptor, - media.mojom.ProtectedBufferManager protected_buffer_manager); + arc.mojom.ProtectedBufferManager protected_buffer_manager); };
diff --git a/components/arc/common/protected_buffer_manager.mojom b/components/arc/common/protected_buffer_manager.mojom index b226ea3d..298f6d3 100644 --- a/components/arc/common/protected_buffer_manager.mojom +++ b/components/arc/common/protected_buffer_manager.mojom
@@ -5,7 +5,7 @@ // The original version of this file lives in the Chromium repository at: // src/components/arc/common/protected_buffer_manager.mojom -module media.mojom; +module arc.mojom; // This interface is exposed by the GPU process for translating dummy handles // for secure buffers into a usable shared memory handle. The output of a
diff --git a/components/arc/common/video_decode_accelerator.mojom b/components/arc/common/video_decode_accelerator.mojom index bb732f9..5307594 100644 --- a/components/arc/common/video_decode_accelerator.mojom +++ b/components/arc/common/video_decode_accelerator.mojom
@@ -40,6 +40,7 @@ uint32 crop_height; }; +// Next MinVersion: 2 struct VideoDecodeAcceleratorConfig { // Deprecated. This config struct is used for decoder only. enum DeviceTypeDeprecated { @@ -51,11 +52,12 @@ DeviceTypeDeprecated device_type_deprecated; uint32 num_input_buffers; uint32 input_pixel_format; + [MinVersion=1] bool secure_mode; }; -// Next MinVersion: 4 +// Next MinVersion: 5 // Deprecated method IDs: 2, 7 -// Next method ID: 10 +// Next method ID: 11 interface VideoDecodeAccelerator { enum Result { SUCCESS = 0, @@ -70,6 +72,10 @@ Initialize@8(VideoDecodeAcceleratorConfig config, VideoDecodeClient client) => (Result result); + [MinVersion=4] + AllocateProtectedBuffer@10(PortType port, uint32 index, handle handle_fd, + uint64 size) => (bool result); + BindSharedMemory@1(PortType port, uint32 index, handle ashmem_fd, uint32 offset, uint32 length);
diff --git a/content/browser/frame_host/render_frame_host_impl.cc b/content/browser/frame_host/render_frame_host_impl.cc index f6b57a31..a955108 100644 --- a/content/browser/frame_host/render_frame_host_impl.cc +++ b/content/browser/frame_host/render_frame_host_impl.cc
@@ -139,8 +139,8 @@ #include "services/device/public/interfaces/vibration_manager.mojom.h" #include "services/device/public/interfaces/wake_lock.mojom.h" #include "services/device/public/interfaces/wake_lock_context.mojom.h" +#include "services/resource_coordinator/public/cpp/frame_resource_coordinator.h" #include "services/resource_coordinator/public/cpp/resource_coordinator_features.h" -#include "services/resource_coordinator/public/cpp/resource_coordinator_interface.h" #include "services/service_manager/public/cpp/connector.h" #include "services/service_manager/public/cpp/interface_provider.h" #include "services/shape_detection/public/interfaces/barcodedetection.mojom.h" @@ -290,9 +290,9 @@ }; #endif // BUILDFLAG(ENABLE_MEDIA_REMOTING) -void CreateResourceCoordinatorFrameInterface( +void CreateFrameResourceCoordinator( RenderFrameHostImpl* render_frame_host, - resource_coordinator::mojom::CoordinationUnitRequest request) { + resource_coordinator::mojom::FrameCoordinationUnitRequest request) { render_frame_host->GetFrameResourceCoordinator()->AddBinding( std::move(request)); } @@ -1286,8 +1286,7 @@ GetProcess()->OnMediaStreamRemoved(); is_audible_ = is_audible; - GetFrameResourceCoordinator()->SetProperty( - resource_coordinator::mojom::PropertyType::kAudible, is_audible_); + GetFrameResourceCoordinator()->SetAudibility(is_audible_); } void RenderFrameHostImpl::OnDidAddMessageToConsole( @@ -2004,10 +2003,8 @@ const GURL& frame_url, JavaScriptDialogType dialog_type, IPC::Message* reply_msg) { - if (dialog_type == JavaScriptDialogType::JAVASCRIPT_DIALOG_TYPE_ALERT) { - GetFrameResourceCoordinator()->SendEvent( - resource_coordinator::mojom::Event::kAlertFired); - } + if (dialog_type == JavaScriptDialogType::JAVASCRIPT_DIALOG_TYPE_ALERT) + GetFrameResourceCoordinator()->OnAlertFired(); if (IsWaitingForUnloadACK()) { SendJavaScriptDialogReply(reply_msg, true, base::string16()); @@ -3035,8 +3032,8 @@ } if (resource_coordinator::IsResourceCoordinatorEnabled()) { - registry_->AddInterface(base::Bind(&CreateResourceCoordinatorFrameInterface, - base::Unretained(this))); + registry_->AddInterface( + base::Bind(&CreateFrameResourceCoordinator, base::Unretained(this))); } #if BUILDFLAG(ENABLE_WEBRTC) @@ -3640,24 +3637,23 @@ return mojo_image_downloader_; } -resource_coordinator::ResourceCoordinatorInterface* +resource_coordinator::FrameResourceCoordinator* RenderFrameHostImpl::GetFrameResourceCoordinator() { if (frame_resource_coordinator_) return frame_resource_coordinator_.get(); if (!resource_coordinator::IsResourceCoordinatorEnabled()) { frame_resource_coordinator_ = - std::make_unique<resource_coordinator::ResourceCoordinatorInterface>( - nullptr, resource_coordinator::CoordinationUnitType::kFrame); + std::make_unique<resource_coordinator::FrameResourceCoordinator>( + nullptr); } else { auto* connection = ServiceManagerConnection::GetForProcess(); frame_resource_coordinator_ = - std::make_unique<resource_coordinator::ResourceCoordinatorInterface>( - connection ? connection->GetConnector() : nullptr, - resource_coordinator::CoordinationUnitType::kFrame); + std::make_unique<resource_coordinator::FrameResourceCoordinator>( + connection ? connection->GetConnector() : nullptr); } if (parent_) { - parent_->GetFrameResourceCoordinator()->AddChild( + parent_->GetFrameResourceCoordinator()->AddChildFrame( *frame_resource_coordinator_); } return frame_resource_coordinator_.get();
diff --git a/content/browser/frame_host/render_frame_host_impl.h b/content/browser/frame_host/render_frame_host_impl.h index b323c3c..3154180 100644 --- a/content/browser/frame_host/render_frame_host_impl.h +++ b/content/browser/frame_host/render_frame_host_impl.h
@@ -604,8 +604,8 @@ // Returns the Mojo ImageDownloader service. const content::mojom::ImageDownloaderPtr& GetMojoImageDownloader(); - resource_coordinator::ResourceCoordinatorInterface* - GetFrameResourceCoordinator() override; + resource_coordinator::FrameResourceCoordinator* GetFrameResourceCoordinator() + override; // Resets the loading state. Following this call, the RenderFrameHost will be // in a non-loading state. @@ -1221,7 +1221,7 @@ content::mojom::ImageDownloaderPtr mojo_image_downloader_; // Holds the interface wrapper to the Global Resource Coordinator service. - std::unique_ptr<resource_coordinator::ResourceCoordinatorInterface> + std::unique_ptr<resource_coordinator::FrameResourceCoordinator> frame_resource_coordinator_; // Tracks a navigation happening in this frame. Note that while there can be
diff --git a/content/browser/renderer_host/render_process_host_impl.cc b/content/browser/renderer_host/render_process_host_impl.cc index 3aa645c6..948e76b8 100644 --- a/content/browser/renderer_host/render_process_host_impl.cc +++ b/content/browser/renderer_host/render_process_host_impl.cc
@@ -196,8 +196,8 @@ #include "ppapi/features/features.h" #include "services/device/public/interfaces/battery_monitor.mojom.h" #include "services/device/public/interfaces/constants.mojom.h" +#include "services/resource_coordinator/public/cpp/process_resource_coordinator.h" #include "services/resource_coordinator/public/cpp/resource_coordinator_features.h" -#include "services/resource_coordinator/public/cpp/resource_coordinator_interface.h" #include "services/service_manager/embedder/switches.h" #include "services/service_manager/public/cpp/binder_registry.h" #include "services/service_manager/public/cpp/connector.h" @@ -687,9 +687,9 @@ std::move(request)); } -void CreateResourceCoordinatorProcessInterface( +void CreateProcessResourceCoordinator( RenderProcessHostImpl* render_process_host, - resource_coordinator::mojom::CoordinationUnitRequest request) { + resource_coordinator::mojom::ProcessCoordinationUnitRequest request) { render_process_host->GetProcessResourceCoordinator()->AddBinding( std::move(request)); } @@ -1836,9 +1836,9 @@ registry.get(), base::Bind(&CreateMemoryCoordinatorHandle, GetID())); } if (resource_coordinator::IsResourceCoordinatorEnabled()) { - AddUIThreadInterface(registry.get(), - base::Bind(&CreateResourceCoordinatorProcessInterface, - base::Unretained(this))); + AddUIThreadInterface( + registry.get(), + base::Bind(&CreateProcessResourceCoordinator, base::Unretained(this))); } media::VideoDecodePerfHistory* video_perf_history = @@ -2112,21 +2112,20 @@ return renderer_interface_.get(); } -resource_coordinator::ResourceCoordinatorInterface* +resource_coordinator::ProcessResourceCoordinator* RenderProcessHostImpl::GetProcessResourceCoordinator() { if (process_resource_coordinator_) return process_resource_coordinator_.get(); if (!resource_coordinator::IsResourceCoordinatorEnabled()) { process_resource_coordinator_ = - std::make_unique<resource_coordinator::ResourceCoordinatorInterface>( - nullptr, resource_coordinator::CoordinationUnitType::kProcess); + std::make_unique<resource_coordinator::ProcessResourceCoordinator>( + nullptr); } else { auto* connection = ServiceManagerConnection::GetForProcess(); process_resource_coordinator_ = - std::make_unique<resource_coordinator::ResourceCoordinatorInterface>( - connection ? connection->GetConnector() : nullptr, - resource_coordinator::CoordinationUnitType::kProcess); + std::make_unique<resource_coordinator::ProcessResourceCoordinator>( + connection ? connection->GetConnector() : nullptr); } return process_resource_coordinator_.get(); } @@ -3883,13 +3882,8 @@ observer.RenderProcessReady(this); } - GetProcessResourceCoordinator()->SetProperty( - resource_coordinator::mojom::PropertyType::kPID, - base::GetProcId(GetHandle())); - - GetProcessResourceCoordinator()->SetProperty( - resource_coordinator::mojom::PropertyType::kLaunchTime, - base::Time::Now().ToTimeT()); + GetProcessResourceCoordinator()->SetLaunchTime(base::Time::Now()); + GetProcessResourceCoordinator()->SetPID(base::GetProcId(GetHandle())); #if BUILDFLAG(ENABLE_WEBRTC) if (WebRTCInternals::GetInstance()->IsAudioDebugRecordingsEnabled()) {
diff --git a/content/browser/renderer_host/render_process_host_impl.h b/content/browser/renderer_host/render_process_host_impl.h index 11203ee..71c07f45 100644 --- a/content/browser/renderer_host/render_process_host_impl.h +++ b/content/browser/renderer_host/render_process_host_impl.h
@@ -215,7 +215,7 @@ void PurgeAndSuspend() override; void Resume() override; mojom::Renderer* GetRendererInterface() override; - resource_coordinator::ResourceCoordinatorInterface* + resource_coordinator::ProcessResourceCoordinator* GetProcessResourceCoordinator() override; void SetIsNeverSuitableForReuse() override; @@ -763,7 +763,7 @@ // determine if if a process should be backgrounded. int media_stream_count_ = 0; - std::unique_ptr<resource_coordinator::ResourceCoordinatorInterface> + std::unique_ptr<resource_coordinator::ProcessResourceCoordinator> process_resource_coordinator_; // A WeakPtrFactory which is reset every time Cleanup() runs. Used to vend
diff --git a/content/public/app/mojo/content_browser_manifest.json b/content/public/app/mojo/content_browser_manifest.json index 648236ba..737ced9 100644 --- a/content/public/app/mojo/content_browser_manifest.json +++ b/content/public/app/mojo/content_browser_manifest.json
@@ -50,7 +50,10 @@ "memory_coordinator::mojom::MemoryCoordinatorHandle", "metrics::mojom::SingleSampleMetricsProvider", "payments::mojom::PaymentManager", - "resource_coordinator::mojom::CoordinationUnit", + "resource_coordinator::mojom::ProcessCoordinationUnit", + "shape_detection::mojom::BarcodeDetection", + "shape_detection::mojom::FaceDetectionProvider", + "shape_detection::mojom::TextDetection", "ui::mojom::Gpu", "viz::mojom::SharedBitmapAllocationNotifier" ], @@ -158,7 +161,7 @@ "network::mojom::RestrictedCookieManager", "payments::mojom::PaymentManager", "payments::mojom::PaymentRequest", - "resource_coordinator::mojom::CoordinationUnit", + "resource_coordinator::mojom::FrameCoordinationUnit", "shape_detection::mojom::BarcodeDetection", "shape_detection::mojom::FaceDetectionProvider", "shape_detection::mojom::TextDetection",
diff --git a/content/public/browser/render_frame_host.h b/content/public/browser/render_frame_host.h index 711c457..d3d22ca9 100644 --- a/content/public/browser/render_frame_host.h +++ b/content/public/browser/render_frame_host.h
@@ -32,7 +32,7 @@ } namespace resource_coordinator { -class ResourceCoordinatorInterface; +class FrameResourceCoordinator; } namespace service_manager { @@ -98,7 +98,7 @@ // Returns the interface for the Global Resource Coordinator // for this frame. - virtual resource_coordinator::ResourceCoordinatorInterface* + virtual resource_coordinator::FrameResourceCoordinator* GetFrameResourceCoordinator() = 0; // Returns the process for this frame.
diff --git a/content/public/browser/render_process_host.h b/content/public/browser/render_process_host.h index 9ab86bc..b11b8394 100644 --- a/content/public/browser/render_process_host.h +++ b/content/public/browser/render_process_host.h
@@ -35,7 +35,7 @@ } namespace resource_coordinator { -class ResourceCoordinatorInterface; +class ProcessResourceCoordinator; } namespace viz { @@ -380,7 +380,7 @@ virtual mojom::Renderer* GetRendererInterface() = 0; // Acquires the interface to the Global Resource Coordinator for this process. - virtual resource_coordinator::ResourceCoordinatorInterface* + virtual resource_coordinator::ProcessResourceCoordinator* GetProcessResourceCoordinator() = 0; // Whether this process is locked out from ever being reused for sites other
diff --git a/content/public/test/mock_render_process_host.cc b/content/public/test/mock_render_process_host.cc index 071cd230..becda99c 100644 --- a/content/public/test/mock_render_process_host.cc +++ b/content/public/test/mock_render_process_host.cc
@@ -356,14 +356,14 @@ return renderer_interface_->get(); } -resource_coordinator::ResourceCoordinatorInterface* +resource_coordinator::ProcessResourceCoordinator* MockRenderProcessHost::GetProcessResourceCoordinator() { if (!process_resource_coordinator_) { service_manager::Connector* connector = content::ServiceManagerConnection::GetForProcess()->GetConnector(); process_resource_coordinator_ = - std::make_unique<resource_coordinator::ResourceCoordinatorInterface>( - connector, resource_coordinator::CoordinationUnitType::kProcess); + std::make_unique<resource_coordinator::ProcessResourceCoordinator>( + connector); } return process_resource_coordinator_.get(); }
diff --git a/content/public/test/mock_render_process_host.h b/content/public/test/mock_render_process_host.h index 99c981f..a3a8772 100644 --- a/content/public/test/mock_render_process_host.h +++ b/content/public/test/mock_render_process_host.h
@@ -22,7 +22,7 @@ #include "ipc/ipc_test_sink.h" #include "media/media_features.h" #include "mojo/public/cpp/bindings/associated_interface_ptr.h" -#include "services/resource_coordinator/public/cpp/resource_coordinator_interface.h" +#include "services/resource_coordinator/public/cpp/process_resource_coordinator.h" #include "services/service_manager/public/cpp/identity.h" #include "services/service_manager/public/cpp/interface_provider.h" @@ -129,7 +129,7 @@ void PurgeAndSuspend() override; void Resume() override; mojom::Renderer* GetRendererInterface() override; - resource_coordinator::ResourceCoordinatorInterface* + resource_coordinator::ProcessResourceCoordinator* GetProcessResourceCoordinator() override; void SetIsNeverSuitableForReuse() override; @@ -202,7 +202,7 @@ std::unique_ptr<mojo::AssociatedInterfacePtr<mojom::Renderer>> renderer_interface_; std::map<std::string, InterfaceBinder> binder_overrides_; - std::unique_ptr<resource_coordinator::ResourceCoordinatorInterface> + std::unique_ptr<resource_coordinator::ProcessResourceCoordinator> process_resource_coordinator_; service_manager::Identity child_identity_; viz::SharedBitmapAllocationNotifierImpl
diff --git a/media/audio/win/audio_low_latency_output_win.cc b/media/audio/win/audio_low_latency_output_win.cc index f806aa4..d9ecbdd 100644 --- a/media/audio/win/audio_low_latency_output_win.cc +++ b/media/audio/win/audio_low_latency_output_win.cc
@@ -180,7 +180,7 @@ const int preferred_frames_per_buffer = static_cast<int>( format_.Format.nSamplesPerSec * - CoreAudioUtil::RefererenceTimeToTimeDelta(device_period) + CoreAudioUtil::ReferenceTimeToTimeDelta(device_period) .InSecondsF() + 0.5);
diff --git a/media/audio/win/core_audio_util_win.cc b/media/audio/win/core_audio_util_win.cc index e532ef0..a525b22c 100644 --- a/media/audio/win/core_audio_util_win.cc +++ b/media/audio/win/core_audio_util_win.cc
@@ -34,6 +34,8 @@ 0xbe39af4f, 0x87c, 0x423f, { 0x93, 0x3, 0x23, 0x4e, 0xc1, 0xe5, 0xb8, 0xee } }; +namespace { + enum { KSAUDIO_SPEAKER_UNSUPPORTED = 0 }; // Converts Microsoft's channel configuration to ChannelLayout. @@ -44,7 +46,7 @@ // As an example: KSAUDIO_SPEAKER_7POINT1_SURROUND is mapped to // CHANNEL_LAYOUT_7_1 but the positions of Back L, Back R and Side L, Side R // speakers are different in these two definitions. -static ChannelLayout ChannelConfigToChannelLayout(ChannelConfig config) { +ChannelLayout ChannelConfigToChannelLayout(ChannelConfig config) { switch (config) { case KSAUDIO_SPEAKER_MONO: DVLOG(2) << "KSAUDIO_SPEAKER_MONO=>CHANNEL_LAYOUT_MONO"; @@ -77,7 +79,7 @@ } // TODO(henrika): add mapping for all types in the ChannelLayout enumerator. -static ChannelConfig ChannelLayoutToChannelConfig(ChannelLayout layout) { +ChannelConfig ChannelLayoutToChannelConfig(ChannelLayout layout) { switch (layout) { case CHANNEL_LAYOUT_MONO: DVLOG(2) << "CHANNEL_LAYOUT_MONO=>KSAUDIO_SPEAKER_MONO"; @@ -109,8 +111,7 @@ } } -static std::ostream& operator<<(std::ostream& os, - const WAVEFORMATPCMEX& format) { +std::ostream& operator<<(std::ostream& os, const WAVEFORMATPCMEX& format) { os << "wFormatTag: 0x" << std::hex << format.Format.wFormatTag << ", nChannels: " << std::dec << format.Format.nChannels << ", nSamplesPerSec: " << format.Format.nSamplesPerSec @@ -123,7 +124,7 @@ return os; } -static bool LoadAudiosesDll() { +bool LoadAudiosesDll() { static const wchar_t* const kAudiosesDLL = L"%WINDIR%\\system32\\audioses.dll"; @@ -132,7 +133,7 @@ return (LoadLibraryExW(path, NULL, LOAD_WITH_ALTERED_SEARCH_PATH) != NULL); } -static std::string GetDeviceID(IMMDevice* device) { +std::string GetDeviceID(IMMDevice* device) { ScopedCoMem<WCHAR> device_id_com; std::string device_id; if (SUCCEEDED(device->GetId(&device_id_com))) @@ -140,18 +141,18 @@ return device_id; } -static bool IsDefaultDeviceId(const std::string& device_id) { +bool IsDefaultDeviceId(const std::string& device_id) { return device_id.empty() || device_id == AudioDeviceDescription::kDefaultDeviceId; } -static bool IsDeviceActive(IMMDevice* device) { +bool IsDeviceActive(IMMDevice* device) { DWORD state = DEVICE_STATE_DISABLED; return SUCCEEDED(device->GetState(&state)) && (state & DEVICE_STATE_ACTIVE); } -static HRESULT GetDeviceFriendlyNameInternal(IMMDevice* device, - std::string* friendly_name) { +HRESULT GetDeviceFriendlyNameInternal(IMMDevice* device, + std::string* friendly_name) { // Retrieve user-friendly name of endpoint device. // Example: "Microphone (Realtek High Definition Audio)". ComPtr<IPropertyStore> properties; @@ -174,7 +175,7 @@ return hr; } -static ComPtr<IMMDeviceEnumerator> CreateDeviceEnumeratorInternal( +ComPtr<IMMDeviceEnumerator> CreateDeviceEnumeratorInternal( bool allow_reinitialize) { ComPtr<IMMDeviceEnumerator> device_enumerator; HRESULT hr = ::CoCreateInstance(__uuidof(MMDeviceEnumerator), NULL, @@ -196,27 +197,70 @@ return device_enumerator; } -static bool IsSupportedInternal() { - // It is possible to force usage of WaveXxx APIs by using a command line flag. +ChannelLayout GetChannelLayout(const WAVEFORMATPCMEX& mix_format) { + // Get the integer mask which corresponds to the channel layout the + // audio engine uses for its internal processing/mixing of shared-mode + // streams. This mask indicates which channels are present in the multi- + // channel stream. The least significant bit corresponds with the Front + // Left speaker, the next least significant bit corresponds to the Front + // Right speaker, and so on, continuing in the order defined in KsMedia.h. + // See + // http://msdn.microsoft.com/en-us/library/windows/hardware/ff537083.aspx + // for more details. + ChannelConfig channel_config = mix_format.dwChannelMask; + + // Convert Microsoft's channel configuration to genric ChannelLayout. + ChannelLayout channel_layout = ChannelConfigToChannelLayout(channel_config); + + // Some devices don't appear to set a valid channel layout, so guess based + // on the number of channels. See http://crbug.com/311906. + if (channel_layout == CHANNEL_LAYOUT_UNSUPPORTED) { + DVLOG(1) << "Unsupported channel config: " << std::hex << channel_config + << ". Guessing layout by channel count: " << std::dec + << mix_format.Format.nChannels; + channel_layout = GuessChannelLayout(mix_format.Format.nChannels); + } + + return channel_layout; +} + +ComPtr<IMMDevice> CreateDeviceInternal(const std::string& device_id, + bool is_output_device) { + EDataFlow data_flow = is_output_device ? eRender : eCapture; + if (device_id == AudioDeviceDescription::kDefaultDeviceId) { + return CoreAudioUtil::CreateDefaultDevice(data_flow, eConsole); + } else if (device_id == AudioDeviceDescription::kLoopbackInputDeviceId || + device_id == AudioDeviceDescription::kLoopbackWithMuteDeviceId) { + DCHECK(!is_output_device); + return CoreAudioUtil::CreateDefaultDevice(eRender, eConsole); + } else if (device_id == AudioDeviceDescription::kCommunicationsDeviceId) { + return CoreAudioUtil::CreateDefaultDevice(data_flow, eCommunications); + } else { + return CoreAudioUtil::CreateDevice(device_id); + } +} + +bool IsSupportedInternal() { + // It is possible to force usage of WaveXxx APIs by using a command line + // flag. const base::CommandLine* cmd_line = base::CommandLine::ForCurrentProcess(); if (cmd_line->HasSwitch(switches::kForceWaveAudio)) { DVLOG(1) << "Forcing usage of Windows WaveXxx APIs"; return false; } - // The audio core APIs are implemented in the Mmdevapi.dll and Audioses.dll - // system components. - // Dependency Walker shows that it is enough to verify possibility to load - // the Audioses DLL since it depends on Mmdevapi.dll. - // See http://crbug.com/166397 why this extra step is required to guarantee - // Core Audio support. + // The audio core APIs are implemented in the Mmdevapi.dll and + // Audioses.dll system components. Dependency Walker shows that it is + // enough to verify possibility to load the Audioses DLL since it depends + // on Mmdevapi.dll. See http://crbug.com/166397 why this extra step is + // required to guarantee Core Audio support. if (!LoadAudiosesDll()) return false; // Being able to load the Audioses.dll does not seem to be sufficient for // all devices to guarantee Core Audio support. To be 100%, we also verify - // that it is possible to a create the IMMDeviceEnumerator interface. If this - // works as well we should be home free. + // that it is possible to a create the IMMDeviceEnumerator interface. If + // this works as well we should be home free. ComPtr<IMMDeviceEnumerator> device_enumerator = CreateDeviceEnumeratorInternal(false); if (!device_enumerator) { @@ -228,13 +272,14 @@ return true; } +} // namespace bool CoreAudioUtil::IsSupported() { static bool g_is_supported = IsSupportedInternal(); return g_is_supported; } -base::TimeDelta CoreAudioUtil::RefererenceTimeToTimeDelta(REFERENCE_TIME time) { +base::TimeDelta CoreAudioUtil::ReferenceTimeToTimeDelta(REFERENCE_TIME time) { // Each unit of reference time is 100 nanoseconds <=> 0.1 microsecond. return base::TimeDelta::FromMicroseconds(0.1 * time + 0.5); } @@ -626,7 +671,7 @@ *device_period = (share_mode == AUDCLNT_SHAREMODE_SHARED) ? default_period : minimum_period; DVLOG(2) << "device_period: " - << RefererenceTimeToTimeDelta(*device_period).InMillisecondsF() + << ReferenceTimeToTimeDelta(*device_period).InMillisecondsF() << " [ms]"; return hr; } @@ -643,28 +688,7 @@ if (FAILED(hr)) return hr; - // Get the integer mask which corresponds to the channel layout the - // audio engine uses for its internal processing/mixing of shared-mode - // streams. This mask indicates which channels are present in the multi- - // channel stream. The least significant bit corresponds with the Front Left - // speaker, the next least significant bit corresponds to the Front Right - // speaker, and so on, continuing in the order defined in KsMedia.h. - // See http://msdn.microsoft.com/en-us/library/windows/hardware/ff537083.aspx - // for more details. - ChannelConfig channel_config = mix_format.dwChannelMask; - - // Convert Microsoft's channel configuration to genric ChannelLayout. - ChannelLayout channel_layout = ChannelConfigToChannelLayout(channel_config); - - // Some devices don't appear to set a valid channel layout, so guess based on - // the number of channels. See http://crbug.com/311906. - if (channel_layout == CHANNEL_LAYOUT_UNSUPPORTED) { - DVLOG(1) << "Unsupported channel config: " - << std::hex << channel_config - << ". Guessing layout by channel count: " - << std::dec << mix_format.Format.nChannels; - channel_layout = GuessChannelLayout(mix_format.Format.nChannels); - } + ChannelLayout channel_layout = GetChannelLayout(mix_format); // Preferred sample rate. int sample_rate = mix_format.Format.nSamplesPerSec; @@ -678,8 +702,9 @@ // buffer size in shared mode. Note that the actual endpoint buffer will be // larger than this size but it will be possible to fill it up in two calls. // TODO(henrika): ensure that this scheme works for capturing as well. - int frames_per_buffer = static_cast<int>(sample_rate * - RefererenceTimeToTimeDelta(default_period).InSecondsF() + 0.5); + int frames_per_buffer = static_cast<int>( + sample_rate * ReferenceTimeToTimeDelta(default_period).InSecondsF() + + 0.5); DVLOG(1) << "channel_layout : " << channel_layout; DVLOG(1) << "sample_rate : " << sample_rate; @@ -699,21 +724,7 @@ HRESULT CoreAudioUtil::GetPreferredAudioParameters(const std::string& device_id, bool is_output_device, AudioParameters* params) { - ComPtr<IMMDevice> device; - if (device_id == AudioDeviceDescription::kDefaultDeviceId) { - device = CoreAudioUtil::CreateDefaultDevice( - is_output_device ? eRender : eCapture, eConsole); - } else if (device_id == AudioDeviceDescription::kLoopbackInputDeviceId || - device_id == AudioDeviceDescription::kLoopbackWithMuteDeviceId) { - DCHECK(!is_output_device); - device = CoreAudioUtil::CreateDefaultDevice(eRender, eConsole); - } else if (device_id == AudioDeviceDescription::kCommunicationsDeviceId) { - device = CoreAudioUtil::CreateDefaultDevice( - is_output_device ? eRender : eCapture, eCommunications); - } else { - device = CreateDevice(device_id); - } - + ComPtr<IMMDevice> device = CreateDeviceInternal(device_id, is_output_device); if (!device.Get()) { // Map NULL-pointer to new error code which can be different from the // actual error code. The exact value is not important here. @@ -812,7 +823,7 @@ REFERENCE_TIME latency = 0; hr = client->GetStreamLatency(&latency); DVLOG(2) << "stream latency: " - << RefererenceTimeToTimeDelta(latency).InMillisecondsF() << " [ms]"; + << ReferenceTimeToTimeDelta(latency).InMillisecondsF() << " [ms]"; return hr; }
diff --git a/media/audio/win/core_audio_util_win.h b/media/audio/win/core_audio_util_win.h index 5ad1622..cdb5d19 100644 --- a/media/audio/win/core_audio_util_win.h +++ b/media/audio/win/core_audio_util_win.h
@@ -43,7 +43,7 @@ // Converts between reference time to base::TimeDelta. // One reference-time unit is 100 nanoseconds. // Example: double s = RefererenceTimeToTimeDelta(t).InMillisecondsF(); - static base::TimeDelta RefererenceTimeToTimeDelta(REFERENCE_TIME time); + static base::TimeDelta ReferenceTimeToTimeDelta(REFERENCE_TIME time); // Returns AUDCLNT_SHAREMODE_EXCLUSIVE if --enable-exclusive-mode is used // as command-line flag and AUDCLNT_SHAREMODE_SHARED otherwise (default).
diff --git a/media/gpu/BUILD.gn b/media/gpu/BUILD.gn index 3aa95b92..c6cf6304 100644 --- a/media/gpu/BUILD.gn +++ b/media/gpu/BUILD.gn
@@ -132,6 +132,8 @@ "fake_jpeg_decode_accelerator.h", "fake_video_decode_accelerator.cc", "fake_video_decode_accelerator.h", + "format_utils.cc", + "format_utils.h", "gles2_decoder_helper.cc", "gles2_decoder_helper.h", "gpu_video_accelerator_util.cc",
diff --git a/media/gpu/format_utils.cc b/media/gpu/format_utils.cc new file mode 100644 index 0000000..70745366b --- /dev/null +++ b/media/gpu/format_utils.cc
@@ -0,0 +1,51 @@ +// 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 "media/gpu/format_utils.h" +#include "base/logging.h" + +namespace media { + +VideoPixelFormat GfxBufferFormatToVideoPixelFormat(gfx::BufferFormat format) { + switch (format) { + case gfx::BufferFormat::BGRX_8888: + return PIXEL_FORMAT_XRGB; + + case gfx::BufferFormat::BGRA_8888: + return PIXEL_FORMAT_ARGB; + + case gfx::BufferFormat::YVU_420: + return PIXEL_FORMAT_YV12; + + case gfx::BufferFormat::YUV_420_BIPLANAR: + return PIXEL_FORMAT_NV12; + + default: + LOG(FATAL) << "Add more cases as needed"; + return PIXEL_FORMAT_UNKNOWN; + } +} + +gfx::BufferFormat VideoPixelFormatToGfxBufferFormat( + VideoPixelFormat pixel_format) { + switch (pixel_format) { + case PIXEL_FORMAT_ARGB: + return gfx::BufferFormat::BGRA_8888; + + case PIXEL_FORMAT_XRGB: + return gfx::BufferFormat::BGRX_8888; + + case PIXEL_FORMAT_YV12: + return gfx::BufferFormat::YVU_420; + + case PIXEL_FORMAT_NV12: + return gfx::BufferFormat::YUV_420_BIPLANAR; + + default: + LOG(FATAL) << "Add more cases as needed"; + return gfx::BufferFormat::BGRX_8888; + } +} + +} // namespace media
diff --git a/media/gpu/format_utils.h b/media/gpu/format_utils.h new file mode 100644 index 0000000..69457614 --- /dev/null +++ b/media/gpu/format_utils.h
@@ -0,0 +1,20 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef MEDIA_GPU_FORMAT_UTILS_H_ +#define MEDIA_GPU_FORMAT_UTILS_H_ + +#include "media/base/video_types.h" +#include "ui/gfx/buffer_types.h" + +namespace media { + +VideoPixelFormat GfxBufferFormatToVideoPixelFormat(gfx::BufferFormat format); + +gfx::BufferFormat VideoPixelFormatToGfxBufferFormat( + VideoPixelFormat pixel_format); + +} // namespace media + +#endif // MEDIA_GPU_FORMAT_UTILS_H_
diff --git a/media/gpu/vaapi_video_decode_accelerator.cc b/media/gpu/vaapi_video_decode_accelerator.cc index 10bcbde..608959e 100644 --- a/media/gpu/vaapi_video_decode_accelerator.cc +++ b/media/gpu/vaapi_video_decode_accelerator.cc
@@ -23,6 +23,7 @@ #include "gpu/ipc/service/gpu_channel.h" #include "media/base/bind_to_current_loop.h" #include "media/gpu/accelerated_video_decoder.h" +#include "media/gpu/format_utils.h" #include "media/gpu/h264_decoder.h" #include "media/gpu/vaapi_picture.h" #include "media/gpu/vp8_decoder.h" @@ -706,24 +707,6 @@ TryFinishSurfaceSetChange(); } -static VideoPixelFormat BufferFormatToVideoPixelFormat( - gfx::BufferFormat format) { - switch (format) { - case gfx::BufferFormat::BGRX_8888: - return PIXEL_FORMAT_XRGB; - - case gfx::BufferFormat::BGRA_8888: - return PIXEL_FORMAT_ARGB; - - case gfx::BufferFormat::YVU_420: - return PIXEL_FORMAT_YV12; - - default: - LOG(FATAL) << "Add more cases as needed"; - return PIXEL_FORMAT_UNKNOWN; - } -} - void VaapiVideoDecodeAccelerator::TryFinishSurfaceSetChange() { DCHECK(task_runner_->BelongsToCurrentThread()); @@ -763,7 +746,7 @@ VLOGF(2) << "Requesting " << requested_num_pics_ << " pictures of size: " << requested_pic_size_.ToString(); - VideoPixelFormat format = BufferFormatToVideoPixelFormat(output_format_); + VideoPixelFormat format = GfxBufferFormatToVideoPixelFormat(output_format_); task_runner_->PostTask( FROM_HERE, base::Bind(&Client::ProvidePictureBuffers, client_, requested_num_pics_, format, 1, requested_pic_size_,
diff --git a/media/gpu/video_decode_accelerator_unittest.cc b/media/gpu/video_decode_accelerator_unittest.cc index 4fd451b..8cf966aa 100644 --- a/media/gpu/video_decode_accelerator_unittest.cc +++ b/media/gpu/video_decode_accelerator_unittest.cc
@@ -61,6 +61,7 @@ #include "media/base/test_data_util.h" #include "media/gpu/fake_video_decode_accelerator.h" #include "media/gpu/features.h" +#include "media/gpu/format_utils.h" #include "media/gpu/gpu_video_decode_accelerator_factory.h" #include "media/gpu/rendering_helper.h" #include "media/gpu/video_accelerator_unittest_helpers.h" @@ -340,23 +341,6 @@ return base::WrapRefCounted(new TextureRef(texture_id, no_longer_needed_cb)); } -#if defined(OS_CHROMEOS) -gfx::BufferFormat VideoPixelFormatToGfxBufferFormat( - VideoPixelFormat pixel_format) { - switch (pixel_format) { - case VideoPixelFormat::PIXEL_FORMAT_ARGB: - return gfx::BufferFormat::BGRA_8888; - case VideoPixelFormat::PIXEL_FORMAT_XRGB: - return gfx::BufferFormat::BGRX_8888; - case VideoPixelFormat::PIXEL_FORMAT_NV12: - return gfx::BufferFormat::YUV_420_BIPLANAR; - default: - LOG_ASSERT(false) << "Unknown VideoPixelFormat"; - return gfx::BufferFormat::BGRX_8888; - } -} -#endif - // static scoped_refptr<TextureRef> TextureRef::CreatePreallocated( uint32_t texture_id,
diff --git a/services/resource_coordinator/BUILD.gn b/services/resource_coordinator/BUILD.gn index f561849f..2446064 100644 --- a/services/resource_coordinator/BUILD.gn +++ b/services/resource_coordinator/BUILD.gn
@@ -85,9 +85,11 @@ "coordination_unit/coordination_unit_base_unittest.cc", "coordination_unit/coordination_unit_test_harness.cc", "coordination_unit/coordination_unit_test_harness.h", + "coordination_unit/frame_coordination_unit_impl_unittest.cc", "coordination_unit/mock_coordination_unit_graphs.cc", "coordination_unit/mock_coordination_unit_graphs.h", "coordination_unit/page_coordination_unit_impl_unittest.cc", + "coordination_unit/process_coordination_unit_impl_unittest.cc", "memory_instrumentation/coordinator_impl_unittest.cc", "memory_instrumentation/graph_processor_unittest.cc", "memory_instrumentation/graph_unittest.cc",
diff --git a/services/resource_coordinator/coordination_unit/coordination_unit_base.cc b/services/resource_coordinator/coordination_unit/coordination_unit_base.cc index 8fa740bc..e0b5c799 100644 --- a/services/resource_coordinator/coordination_unit/coordination_unit_base.cc +++ b/services/resource_coordinator/coordination_unit/coordination_unit_base.cc
@@ -6,8 +6,6 @@ #include <unordered_map> -#include "base/strings/string_number_conversions.h" -#include "mojo/public/cpp/bindings/strong_binding.h" #include "services/resource_coordinator/coordination_unit/frame_coordination_unit_impl.h" #include "services/resource_coordinator/coordination_unit/page_coordination_unit_impl.h" #include "services/resource_coordinator/coordination_unit/process_coordination_unit_impl.h" @@ -29,24 +27,11 @@ } // namespace // static -const FrameCoordinationUnitImpl* CoordinationUnitBase::ToFrameCoordinationUnit( - const CoordinationUnitBase* coordination_unit) { - DCHECK(coordination_unit->id().type == CoordinationUnitType::kFrame); - return static_cast<const FrameCoordinationUnitImpl*>(coordination_unit); -} - -// static -PageCoordinationUnitImpl* CoordinationUnitBase::ToPageCoordinationUnit( - CoordinationUnitBase* coordination_unit) { - DCHECK(coordination_unit->id().type == CoordinationUnitType::kPage); - return static_cast<PageCoordinationUnitImpl*>(coordination_unit); -} - -// static -const PageCoordinationUnitImpl* CoordinationUnitBase::ToPageCoordinationUnit( - const CoordinationUnitBase* cu) { - DCHECK(cu->id().type == CoordinationUnitType::kPage); - return static_cast<const PageCoordinationUnitImpl*>(cu); +CoordinationUnitBase* CoordinationUnitBase::AddNewCoordinationUnit( + std::unique_ptr<CoordinationUnitBase> new_cu) { + auto it = g_cu_map().emplace(new_cu->id(), std::move(new_cu)); + DCHECK(it.second); // Inserted successfully + return it.first->second.get(); } // static @@ -54,231 +39,40 @@ CoordinationUnitBase::GetCoordinationUnitsOfType(CoordinationUnitType type) { std::vector<CoordinationUnitBase*> results; for (auto& el : g_cu_map()) { - if (el.second->id().type == type) + if (el.first.type == type) results.push_back(el.second.get()); } return results; } +// static void CoordinationUnitBase::AssertNoActiveCoordinationUnits() { CHECK(g_cu_map().empty()); } +// static void CoordinationUnitBase::ClearAllCoordinationUnits() { g_cu_map().clear(); } -CoordinationUnitBase* CoordinationUnitBase::CreateCoordinationUnit( - const CoordinationUnitID& id, - std::unique_ptr<service_manager::ServiceContextRef> service_ref) { - std::unique_ptr<CoordinationUnitBase> new_cu; - - switch (id.type) { - case CoordinationUnitType::kFrame: - new_cu = std::make_unique<FrameCoordinationUnitImpl>( - id, std::move(service_ref)); - break; - case CoordinationUnitType::kProcess: - new_cu = std::make_unique<ProcessCoordinationUnitImpl>( - id, std::move(service_ref)); - break; - case CoordinationUnitType::kPage: - new_cu = std::make_unique<PageCoordinationUnitImpl>( - id, std::move(service_ref)); - break; - default: - NOTREACHED(); - } - - auto it = g_cu_map().insert(std::make_pair(new_cu->id(), std::move(new_cu))); - DCHECK(it.second); // Inserted successfully - return it.first->second.get(); +// static +CoordinationUnitBase* CoordinationUnitBase::GetCoordinationUnitByID( + const CoordinationUnitID cu_id) { + auto cu_iter = g_cu_map().find(cu_id); + return cu_iter != g_cu_map().end() ? cu_iter->second.get() : nullptr; } +CoordinationUnitBase::CoordinationUnitBase(const CoordinationUnitID& id) + : id_(id.type, id.id) {} + +CoordinationUnitBase::~CoordinationUnitBase() = default; + void CoordinationUnitBase::Destruct() { size_t erased_count = g_cu_map().erase(id_); // After this point |this| is destructed and should not be accessed anymore. DCHECK_EQ(erased_count, 1u); } -CoordinationUnitBase::CoordinationUnitBase( - const CoordinationUnitID& id, - std::unique_ptr<service_manager::ServiceContextRef> service_ref) - : id_(id.type, id.id), binding_(this) { - service_ref_ = std::move(service_ref); -} - -CoordinationUnitBase::~CoordinationUnitBase() { - for (CoordinationUnitBase* child : children_) { - child->RemoveParent(this); - } - - for (CoordinationUnitBase* parent : parents_) { - parent->RemoveChild(this); - } -} - -void CoordinationUnitBase::Bind(mojom::CoordinationUnitRequest request) { - binding_.Bind(std::move(request)); -} - -void CoordinationUnitBase::SendEvent(mojom::Event event) { - OnEventReceived(event); -} - -void CoordinationUnitBase::GetID(const GetIDCallback& callback) { - callback.Run(id_); -} - -void CoordinationUnitBase::AddBinding(mojom::CoordinationUnitRequest request) { - bindings_.AddBinding(this, std::move(request)); -} - -void CoordinationUnitBase::AddChild(const CoordinationUnitID& child_id) { - // TODO(ojan): Make this a DCHECK. When does it make sense to add a - // CoordinationUnit as its own child? - if (child_id == id_) { - return; - } - - auto child_iter = g_cu_map().find(child_id); - if (child_iter != g_cu_map().end()) { - CoordinationUnitBase* child = child_iter->second.get(); - // In order to avoid cyclic reference inside the coordination unit graph. If - // |child| is one of the ancestors of |this| coordination unit, then |child| - // should not be added, abort this operation. If |this| coordination unit is - // one of the descendants of child coordination unit, then abort this - // operation. - if (HasAncestor(child) || child->HasDescendant(this)) { - DCHECK(false) << "Cyclic reference in coordination unit graph detected!"; - return; - } - - DCHECK(child->id_ == child_id); - DCHECK(child != this); - - if (AddChild(child)) { - child->AddParent(this); - } - } -} - -bool CoordinationUnitBase::AddChild(CoordinationUnitBase* child) { - bool success = - children_.count(child) ? false : children_.insert(child).second; - return success; -} - -void CoordinationUnitBase::RemoveChild(const CoordinationUnitID& child_id) { - auto child_iter = g_cu_map().find(child_id); - if (child_iter == g_cu_map().end()) { - return; - } - - CoordinationUnitBase* child = child_iter->second.get(); - - DCHECK(child->id_ == child_id); - DCHECK(child != this); - - if (RemoveChild(child)) { - child->RemoveParent(this); - } -} - -bool CoordinationUnitBase::RemoveChild(CoordinationUnitBase* child) { - size_t children_removed = children_.erase(child); - return children_removed > 0; -} - -void CoordinationUnitBase::AddParent(CoordinationUnitBase* parent) { - DCHECK_EQ(0u, parents_.count(parent)); - parents_.insert(parent); -} - -void CoordinationUnitBase::RemoveParent(CoordinationUnitBase* parent) { - size_t parents_removed = parents_.erase(parent); - DCHECK_EQ(1u, parents_removed); -} - -bool CoordinationUnitBase::HasAncestor(CoordinationUnitBase* ancestor) { - for (CoordinationUnitBase* parent : parents_) { - if (parent == ancestor || parent->HasAncestor(ancestor)) { - return true; - } - } - - return false; -} - -bool CoordinationUnitBase::HasDescendant(CoordinationUnitBase* descendant) { - for (CoordinationUnitBase* child : children_) { - if (child == descendant || child->HasDescendant(descendant)) { - return true; - } - } - - return false; -} - -std::set<CoordinationUnitBase*> -CoordinationUnitBase::GetChildCoordinationUnitsOfType( - CoordinationUnitType type) const { - std::set<CoordinationUnitBase*> coordination_units; - - for (auto* child : children()) { - if (child->id().type != type) - continue; - coordination_units.insert(child); - for (auto* coordination_unit : - child->GetChildCoordinationUnitsOfType(type)) { - coordination_units.insert(coordination_unit); - } - } - - return coordination_units; -} - -std::set<CoordinationUnitBase*> -CoordinationUnitBase::GetParentCoordinationUnitsOfType( - CoordinationUnitType type) const { - std::set<CoordinationUnitBase*> coordination_units; - - for (auto* parent : parents()) { - if (parent->id().type != type) - continue; - coordination_units.insert(parent); - for (auto* coordination_unit : - parent->GetParentCoordinationUnitsOfType(type)) { - coordination_units.insert(coordination_unit); - } - } - - return coordination_units; -} - -bool CoordinationUnitBase::GetProperty(const mojom::PropertyType property_type, - int64_t* result) const { - auto value_it = properties_.find(property_type); - - if (value_it != properties_.end()) { - *result = value_it->second; - return true; - } - - return false; -} - -void CoordinationUnitBase::SetProperty(mojom::PropertyType property_type, - int64_t value) { - // The |CoordinationUnitGraphObserver| API specification dictates that - // the property is guarranteed to be set on the |CoordinationUnitBase| - // and propagated to the appropriate associated |CoordianationUnitBase| - // before |OnPropertyChanged| is invoked on all of the registered observers. - properties_[property_type] = value; - PropagateProperty(property_type, value); - OnPropertyChanged(property_type, value); -} - void CoordinationUnitBase::BeforeDestroyed() { for (auto& observer : observers_) observer.OnBeforeCoordinationUnitDestroyed(this); @@ -294,16 +88,40 @@ observers_.RemoveObserver(observer); } -void CoordinationUnitBase::OnEventReceived(const mojom::Event event) { - for (auto& observer : observers_) +bool CoordinationUnitBase::GetProperty(const mojom::PropertyType property_type, + int64_t* result) const { + auto value_it = properties_.find(property_type); + if (value_it != properties_.end()) { + *result = value_it->second; + return true; + } + return false; +} + +void CoordinationUnitBase::OnEventReceived(mojom::Event event) { + for (auto& observer : observers()) observer.OnEventReceived(this, event); } -void CoordinationUnitBase::OnPropertyChanged( - const mojom::PropertyType property_type, - int64_t value) { - for (auto& observer : observers_) +void CoordinationUnitBase::OnPropertyChanged(mojom::PropertyType property_type, + int64_t value) { + for (auto& observer : observers()) observer.OnPropertyChanged(this, property_type, value); } +void CoordinationUnitBase::SendEvent(mojom::Event event) { + OnEventReceived(event); +} + +void CoordinationUnitBase::SetProperty(mojom::PropertyType property_type, + int64_t value) { + // The |CoordinationUnitGraphObserver| API specification dictates that + // the property is guarranteed to be set on the |CoordinationUnitBase| + // and propagated to the appropriate associated |CoordianationUnitBase| + // before |OnPropertyChanged| is invoked on all of the registered observers. + properties_[property_type] = value; + PropagateProperty(property_type, value); + OnPropertyChanged(property_type, value); +} + } // namespace resource_coordinator
diff --git a/services/resource_coordinator/coordination_unit/coordination_unit_base.h b/services/resource_coordinator/coordination_unit/coordination_unit_base.h index b0cb65d8..513c7207 100644 --- a/services/resource_coordinator/coordination_unit/coordination_unit_base.h +++ b/services/resource_coordinator/coordination_unit/coordination_unit_base.h
@@ -6,15 +6,13 @@ #define SERVICES_RESOURCE_COORDINATOR_COORDINATION_UNIT_COORDINATION_UNIT_BASE_H_ #include <memory> -#include <set> #include "base/callback.h" #include "base/observer_list.h" -#include "base/optional.h" -#include "base/values.h" #include "mojo/public/cpp/bindings/binding_set.h" #include "mojo/public/cpp/bindings/interface_request.h" #include "mojo/public/cpp/bindings/strong_binding.h" +#include "services/resource_coordinator/observers/coordination_unit_graph_observer.h" #include "services/resource_coordinator/public/cpp/coordination_unit_types.h" #include "services/resource_coordinator/public/interfaces/coordination_unit.mojom.h" #include "services/resource_coordinator/public/interfaces/coordination_unit_provider.mojom.h" @@ -22,116 +20,134 @@ namespace resource_coordinator { -class CoordinationUnitGraphObserver; -class FrameCoordinationUnitImpl; -class PageCoordinationUnitImpl; - // CoordinationUnitBase implements shared functionality among different types of // coordination units. A specific type of coordination unit will derive from // this class and can override shared funtionality when needed. -class CoordinationUnitBase : public mojom::CoordinationUnit { +class CoordinationUnitBase { public: - CoordinationUnitBase( - const CoordinationUnitID& id, - std::unique_ptr<service_manager::ServiceContextRef> service_ref); - - ~CoordinationUnitBase() override; - - static const FrameCoordinationUnitImpl* ToFrameCoordinationUnit( - const CoordinationUnitBase* coordination_unit); - static PageCoordinationUnitImpl* ToPageCoordinationUnit( - CoordinationUnitBase* coordination_unit); - static const PageCoordinationUnitImpl* ToPageCoordinationUnit( - const CoordinationUnitBase* coordination_unit); - - static std::vector<CoordinationUnitBase*> GetCoordinationUnitsOfType( - CoordinationUnitType type); - - static CoordinationUnitBase* CreateCoordinationUnit( - const CoordinationUnitID& id, - std::unique_ptr<service_manager::ServiceContextRef> service_ref); - + // Add the newly created coordination unit to the global coordination unit + // storage. + static CoordinationUnitBase* AddNewCoordinationUnit( + std::unique_ptr<CoordinationUnitBase> new_cu); static void AssertNoActiveCoordinationUnits(); static void ClearAllCoordinationUnits(); - void Destruct(); - void Bind(mojom::CoordinationUnitRequest request); - - // Overridden from mojom::CoordinationUnit: - void GetID(const GetIDCallback& callback) override; - void AddBinding(mojom::CoordinationUnitRequest request) override; - void AddChild(const CoordinationUnitID& child_id) override; - void RemoveChild(const CoordinationUnitID& child_id) override; - void SendEvent(mojom::Event event) override; - void SetProperty(mojom::PropertyType property_type, int64_t value) override; - - // Return all of the reachable |CoordinationUnitBase| instances - // of type |CoordinationUnitType|. Note that a callee should - // never be associated with itself. - virtual std::set<CoordinationUnitBase*> GetAssociatedCoordinationUnitsOfType( - CoordinationUnitType type) const = 0; // Recalculate property internally. virtual void RecalculateProperty(const mojom::PropertyType property_type) {} - // Operations performed on the internal key-value store. - bool GetProperty(const mojom::PropertyType property_type, - int64_t* result) const; + CoordinationUnitBase(const CoordinationUnitID& id); + virtual ~CoordinationUnitBase(); - // Methods utilized by the |CoordinationUnitGraphObserver| framework. + void Destruct(); void BeforeDestroyed(); void AddObserver(CoordinationUnitGraphObserver* observer); void RemoveObserver(CoordinationUnitGraphObserver* observer); + bool GetProperty(const mojom::PropertyType property_type, + int64_t* result) const; - // Coordination unit graph traversal helper functions. - std::set<CoordinationUnitBase*> GetChildCoordinationUnitsOfType( - CoordinationUnitType type) const; - std::set<CoordinationUnitBase*> GetParentCoordinationUnitsOfType( - CoordinationUnitType type) const; - - // Getters and setters. const CoordinationUnitID& id() const { return id_; } - const std::set<CoordinationUnitBase*>& children() const { return children_; } - const std::set<CoordinationUnitBase*>& parents() const { return parents_; } - const std::map<mojom::PropertyType, int64_t>& properties_for_testing() const { - return properties_; - } - mojo::Binding<mojom::CoordinationUnit>& binding() { return binding_; } - - protected: - virtual void OnEventReceived(const mojom::Event event); - virtual void OnPropertyChanged(const mojom::PropertyType property_type, - int64_t value); - // Propagate property change to relevant |CoordinationUnitBase| instances. - virtual void PropagateProperty(mojom::PropertyType property_type, - int64_t value) {} - const base::ObserverList<CoordinationUnitGraphObserver>& observers() const { return observers_; } + void SetPropertyForTesting(int64_t value) { + SetProperty(mojom::PropertyType::kTest, value); + } + + const std::map<mojom::PropertyType, int64_t>& properties_for_testing() const { + return properties_; + } + + protected: + static CoordinationUnitBase* GetCoordinationUnitByID( + const CoordinationUnitID cu_id); + static std::vector<CoordinationUnitBase*> GetCoordinationUnitsOfType( + CoordinationUnitType cu_type); + + // Propagate property change to relevant |CoordinationUnitBase| instances. + virtual void PropagateProperty(mojom::PropertyType property_type, + int64_t value) {} + virtual void OnEventReceived(mojom::Event event); + virtual void OnPropertyChanged(mojom::PropertyType property_type, + int64_t value); + + void SendEvent(mojom::Event event); + void SetProperty(mojom::PropertyType property_type, int64_t value); + const CoordinationUnitID id_; - std::set<CoordinationUnitBase*> children_; - std::set<CoordinationUnitBase*> parents_; private: - bool AddChild(CoordinationUnitBase* child); - bool RemoveChild(CoordinationUnitBase* child); - void AddParent(CoordinationUnitBase* parent); - void RemoveParent(CoordinationUnitBase* parent); - bool HasAncestor(CoordinationUnitBase* unit); - bool HasDescendant(CoordinationUnitBase* unit); - + base::ObserverList<CoordinationUnitGraphObserver> observers_; std::map<mojom::PropertyType, int64_t> properties_; - std::unique_ptr<service_manager::ServiceContextRef> service_ref_; - mojo::BindingSet<mojom::CoordinationUnit> bindings_; - - base::ObserverList<CoordinationUnitGraphObserver> observers_; - mojo::Binding<mojom::CoordinationUnit> binding_; - DISALLOW_COPY_AND_ASSIGN(CoordinationUnitBase); }; +template <class CoordinationUnitClass, + class MojoInterfaceClass, + class MojoRequestClass> +class CoordinationUnitInterface : public CoordinationUnitBase, + public MojoInterfaceClass { + public: + static CoordinationUnitClass* Create( + const CoordinationUnitID& id, + std::unique_ptr<service_manager::ServiceContextRef> service_ref) { + std::unique_ptr<CoordinationUnitClass> new_cu = + base::MakeUnique<CoordinationUnitClass>(id, std::move(service_ref)); + return static_cast<CoordinationUnitClass*>( + CoordinationUnitBase::AddNewCoordinationUnit(std::move(new_cu))); + } + + static const CoordinationUnitClass* FromCoordinationUnitBase( + const CoordinationUnitBase* cu) { + DCHECK(cu->id().type == CoordinationUnitClass::Type()); + return static_cast<const CoordinationUnitClass*>(cu); + } + + static CoordinationUnitClass* FromCoordinationUnitBase( + CoordinationUnitBase* cu) { + DCHECK(cu->id().type == CoordinationUnitClass::Type()); + return static_cast<CoordinationUnitClass*>(cu); + } + + CoordinationUnitInterface( + const CoordinationUnitID& id, + std::unique_ptr<service_manager::ServiceContextRef> service_ref) + : CoordinationUnitBase(id), binding_(this) { + service_ref_ = std::move(service_ref); + } + + ~CoordinationUnitInterface() override = default; + + void Bind(MojoRequestClass request) { binding_.Bind(std::move(request)); } + + void GetID( + const typename MojoInterfaceClass::GetIDCallback& callback) override { + callback.Run(id_); + } + void AddBinding(MojoRequestClass request) override { + bindings_.AddBinding(this, std::move(request)); + } + + mojo::Binding<MojoInterfaceClass>& binding() { return binding_; } + + protected: + static CoordinationUnitClass* GetCoordinationUnitByID( + const CoordinationUnitID cu_id) { + DCHECK(cu_id.type == CoordinationUnitClass::Type()); + auto* cu = CoordinationUnitBase::GetCoordinationUnitByID(cu_id); + DCHECK(cu->id().type == CoordinationUnitClass::Type()); + return static_cast<CoordinationUnitClass*>(cu); + } + + private: + mojo::BindingSet<MojoInterfaceClass> bindings_; + mojo::Binding<MojoInterfaceClass> binding_; + std::unique_ptr<service_manager::ServiceContextRef> service_ref_; + + DISALLOW_COPY_AND_ASSIGN(CoordinationUnitInterface); +}; + } // namespace resource_coordinator #endif // SERVICES_RESOURCE_COORDINATOR_COORDINATION_UNIT_COORDINATION_UNIT_BASE_H_
diff --git a/services/resource_coordinator/coordination_unit/coordination_unit_base_unittest.cc b/services/resource_coordinator/coordination_unit/coordination_unit_base_unittest.cc index c0e65185..b5d923db 100644 --- a/services/resource_coordinator/coordination_unit/coordination_unit_base_unittest.cc +++ b/services/resource_coordinator/coordination_unit/coordination_unit_base_unittest.cc
@@ -2,16 +2,12 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include <string> - -#include "base/bind.h" -#include "base/message_loop/message_loop.h" -#include "base/run_loop.h" -#include "base/values.h" #include "services/resource_coordinator/coordination_unit/coordination_unit_base.h" #include "services/resource_coordinator/coordination_unit/coordination_unit_provider_impl.h" #include "services/resource_coordinator/coordination_unit/coordination_unit_test_harness.h" #include "services/resource_coordinator/coordination_unit/mock_coordination_unit_graphs.h" +#include "services/resource_coordinator/coordination_unit/page_coordination_unit_impl.h" +#include "services/resource_coordinator/coordination_unit/process_coordination_unit_impl.h" #include "services/resource_coordinator/public/interfaces/coordination_unit.mojom.h" #include "services/service_manager/public/cpp/service_context_ref.h" #include "testing/gtest/include/gtest/gtest.h" @@ -26,90 +22,8 @@ } // namespace -TEST_F(CoordinationUnitBaseTest, AddChildBasic) { - auto page_cu = CreateCoordinationUnit(CoordinationUnitType::kPage); - auto frame1_cu = CreateCoordinationUnit(CoordinationUnitType::kFrame); - auto frame2_cu = CreateCoordinationUnit(CoordinationUnitType::kFrame); - auto frame3_cu = CreateCoordinationUnit(CoordinationUnitType::kFrame); - - page_cu->AddChild(frame1_cu->id()); - page_cu->AddChild(frame2_cu->id()); - page_cu->AddChild(frame3_cu->id()); - EXPECT_EQ(3u, page_cu->children().size()); -} - -#if (!defined(NDEBUG) || defined(DCHECK_ALWAYS_ON)) && GTEST_HAS_DEATH_TEST -TEST_F(CoordinationUnitBaseDeathTest, AddChildOnCyclicReference) { - ::testing::FLAGS_gtest_death_test_style = "threadsafe"; - - CoordinationUnitID frame1_cu_id(CoordinationUnitType::kFrame, std::string()); - CoordinationUnitID frame2_cu_id(CoordinationUnitType::kFrame, std::string()); - CoordinationUnitID frame3_cu_id(CoordinationUnitType::kFrame, std::string()); - - auto frame1_cu = CreateCoordinationUnit(frame1_cu_id); - auto frame2_cu = CreateCoordinationUnit(frame2_cu_id); - auto frame3_cu = CreateCoordinationUnit(frame3_cu_id); - - frame1_cu->AddChild(frame2_cu->id()); - frame2_cu->AddChild(frame3_cu->id()); - // |frame3_cu| can't add |frame1_cu| because |frame1_cu| is an ancestor of - // |frame3_cu|, and this will hit a DCHECK because of cyclic reference. - EXPECT_DEATH(frame3_cu->AddChild(frame1_cu->id()), ""); -} -#else -TEST_F(CoordinationUnitBaseTest, AddChildOnCyclicReference) { - CoordinationUnitID frame1_cu_id(CoordinationUnitType::kFrame, std::string()); - CoordinationUnitID frame2_cu_id(CoordinationUnitType::kFrame, std::string()); - CoordinationUnitID frame3_cu_id(CoordinationUnitType::kFrame, std::string()); - - auto frame1_cu = CreateCoordinationUnit(frame1_cu_id); - auto frame2_cu = CreateCoordinationUnit(frame2_cu_id); - auto frame3_cu = CreateCoordinationUnit(frame3_cu_id); - - frame1_cu->AddChild(frame2_cu->id()); - frame2_cu->AddChild(frame3_cu->id()); - frame3_cu->AddChild(frame1_cu->id()); - - EXPECT_EQ(1u, frame1_cu->children().count(frame2_cu.get())); - EXPECT_EQ(1u, frame2_cu->children().count(frame3_cu.get())); - // |frame1_cu| was not added successfully because |frame1_cu| is one of the - // ancestors of |frame3_cu|. - EXPECT_EQ(0u, frame3_cu->children().count(frame1_cu.get())); -} -#endif // (!defined(NDEBUG) || defined(DCHECK_ALWAYS_ON)) && - // GTEST_HAS_DEATH_TEST - -TEST_F(CoordinationUnitBaseTest, RemoveChild) { - auto parent_coordination_unit = - CreateCoordinationUnit(CoordinationUnitType::kFrame); - auto child_coordination_unit = - CreateCoordinationUnit(CoordinationUnitType::kFrame); - - // Parent-child relationships have not been established yet. - EXPECT_EQ(0u, parent_coordination_unit->children().size()); - EXPECT_EQ(0u, parent_coordination_unit->parents().size()); - EXPECT_EQ(0u, child_coordination_unit->children().size()); - EXPECT_EQ(0u, child_coordination_unit->parents().size()); - - parent_coordination_unit->AddChild(child_coordination_unit->id()); - - // Ensure correct Parent-child relationships have been established. - EXPECT_EQ(1u, parent_coordination_unit->children().size()); - EXPECT_EQ(0u, parent_coordination_unit->parents().size()); - EXPECT_EQ(0u, child_coordination_unit->children().size()); - EXPECT_EQ(1u, child_coordination_unit->parents().size()); - - parent_coordination_unit->RemoveChild(child_coordination_unit->id()); - - // Parent-child relationships should no longer exist. - EXPECT_EQ(0u, parent_coordination_unit->children().size()); - EXPECT_EQ(0u, parent_coordination_unit->parents().size()); - EXPECT_EQ(0u, child_coordination_unit->children().size()); - EXPECT_EQ(0u, child_coordination_unit->parents().size()); -} - TEST_F(CoordinationUnitBaseTest, GetSetProperty) { - auto coordination_unit = CreateCoordinationUnit(CoordinationUnitType::kPage); + auto coordination_unit = CreateCoordinationUnit<PageCoordinationUnitImpl>(); // An empty value should be returned if property is not found int64_t test_value; @@ -117,7 +31,7 @@ coordination_unit->GetProperty(mojom::PropertyType::kTest, &test_value)); // Perform a valid storage property set - coordination_unit->SetProperty(mojom::PropertyType::kTest, 41); + coordination_unit->SetPropertyForTesting(41); EXPECT_EQ(1u, coordination_unit->properties_for_testing().size()); EXPECT_TRUE( coordination_unit->GetProperty(mojom::PropertyType::kTest, &test_value)); @@ -129,14 +43,12 @@ MockSinglePageInSingleProcessCoordinationUnitGraph cu_graph; auto pages_associated_with_process = - cu_graph.process->GetAssociatedCoordinationUnitsOfType( - CoordinationUnitType::kPage); + cu_graph.process->GetAssociatedPageCoordinationUnits(); EXPECT_EQ(1u, pages_associated_with_process.size()); EXPECT_EQ(1u, pages_associated_with_process.count(cu_graph.page.get())); auto processes_associated_with_page = - cu_graph.page->GetAssociatedCoordinationUnitsOfType( - CoordinationUnitType::kProcess); + cu_graph.page->GetAssociatedProcessCoordinationUnits(); EXPECT_EQ(1u, processes_associated_with_page.size()); EXPECT_EQ(1u, processes_associated_with_page.count(cu_graph.process.get())); } @@ -146,21 +58,18 @@ MockMultiplePagesInSingleProcessCoordinationUnitGraph cu_graph; auto pages_associated_with_process = - cu_graph.process->GetAssociatedCoordinationUnitsOfType( - CoordinationUnitType::kPage); + cu_graph.process->GetAssociatedPageCoordinationUnits(); EXPECT_EQ(2u, pages_associated_with_process.size()); EXPECT_EQ(1u, pages_associated_with_process.count(cu_graph.page.get())); EXPECT_EQ(1u, pages_associated_with_process.count(cu_graph.other_page.get())); auto processes_associated_with_page = - cu_graph.page->GetAssociatedCoordinationUnitsOfType( - CoordinationUnitType::kProcess); + cu_graph.page->GetAssociatedProcessCoordinationUnits(); EXPECT_EQ(1u, processes_associated_with_page.size()); EXPECT_EQ(1u, processes_associated_with_page.count(cu_graph.process.get())); auto processes_associated_with_other_page = - cu_graph.other_page->GetAssociatedCoordinationUnitsOfType( - CoordinationUnitType::kProcess); + cu_graph.other_page->GetAssociatedProcessCoordinationUnits(); EXPECT_EQ(1u, processes_associated_with_other_page.size()); EXPECT_EQ(1u, processes_associated_with_page.count(cu_graph.process.get())); } @@ -170,20 +79,17 @@ MockSinglePageWithMultipleProcessesCoordinationUnitGraph cu_graph; auto pages_associated_with_process = - cu_graph.process->GetAssociatedCoordinationUnitsOfType( - CoordinationUnitType::kPage); + cu_graph.process->GetAssociatedPageCoordinationUnits(); EXPECT_EQ(1u, pages_associated_with_process.size()); EXPECT_EQ(1u, pages_associated_with_process.count(cu_graph.page.get())); auto pages_associated_with_other_process = - cu_graph.other_process->GetAssociatedCoordinationUnitsOfType( - CoordinationUnitType::kPage); + cu_graph.other_process->GetAssociatedPageCoordinationUnits(); EXPECT_EQ(1u, pages_associated_with_other_process.size()); EXPECT_EQ(1u, pages_associated_with_other_process.count(cu_graph.page.get())); auto processes_associated_with_page = - cu_graph.page->GetAssociatedCoordinationUnitsOfType( - CoordinationUnitType::kProcess); + cu_graph.page->GetAssociatedProcessCoordinationUnits(); EXPECT_EQ(2u, processes_associated_with_page.size()); EXPECT_EQ(1u, processes_associated_with_page.count(cu_graph.process.get())); EXPECT_EQ(1u, @@ -195,28 +101,24 @@ MockMultiplePagesWithMultipleProcessesCoordinationUnitGraph cu_graph; auto pages_associated_with_process = - cu_graph.process->GetAssociatedCoordinationUnitsOfType( - CoordinationUnitType::kPage); + cu_graph.process->GetAssociatedPageCoordinationUnits(); EXPECT_EQ(2u, pages_associated_with_process.size()); EXPECT_EQ(1u, pages_associated_with_process.count(cu_graph.page.get())); EXPECT_EQ(1u, pages_associated_with_process.count(cu_graph.other_page.get())); auto pages_associated_with_other_process = - cu_graph.other_process->GetAssociatedCoordinationUnitsOfType( - CoordinationUnitType::kPage); + cu_graph.other_process->GetAssociatedPageCoordinationUnits(); EXPECT_EQ(1u, pages_associated_with_other_process.size()); EXPECT_EQ( 1u, pages_associated_with_other_process.count(cu_graph.other_page.get())); auto processes_associated_with_page = - cu_graph.page->GetAssociatedCoordinationUnitsOfType( - CoordinationUnitType::kProcess); + cu_graph.page->GetAssociatedProcessCoordinationUnits(); EXPECT_EQ(1u, processes_associated_with_page.size()); EXPECT_EQ(1u, processes_associated_with_page.count(cu_graph.process.get())); auto processes_associated_with_other_page = - cu_graph.other_page->GetAssociatedCoordinationUnitsOfType( - CoordinationUnitType::kProcess); + cu_graph.other_page->GetAssociatedProcessCoordinationUnits(); EXPECT_EQ(2u, processes_associated_with_other_page.size()); EXPECT_EQ(1u, processes_associated_with_other_page.count(cu_graph.process.get()));
diff --git a/services/resource_coordinator/coordination_unit/coordination_unit_introspector_impl.cc b/services/resource_coordinator/coordination_unit/coordination_unit_introspector_impl.cc index 5890753f..f927985b 100644 --- a/services/resource_coordinator/coordination_unit/coordination_unit_introspector_impl.cc +++ b/services/resource_coordinator/coordination_unit/coordination_unit_introspector_impl.cc
@@ -8,9 +8,9 @@ #include "base/process/process_handle.h" #include "base/time/time.h" -#include "services/resource_coordinator/coordination_unit/coordination_unit_base.h" #include "services/resource_coordinator/coordination_unit/frame_coordination_unit_impl.h" #include "services/resource_coordinator/coordination_unit/page_coordination_unit_impl.h" +#include "services/resource_coordinator/coordination_unit/process_coordination_unit_impl.h" #include "services/service_manager/public/cpp/bind_source_info.h" namespace resource_coordinator { @@ -22,10 +22,9 @@ void CoordinationUnitIntrospectorImpl::GetProcessToURLMap( const GetProcessToURLMapCallback& callback) { std::vector<resource_coordinator::mojom::ProcessInfoPtr> process_infos; - std::vector<CoordinationUnitBase*> process_cus = - CoordinationUnitBase::GetCoordinationUnitsOfType( - CoordinationUnitType::kProcess); - for (CoordinationUnitBase* process_cu : process_cus) { + std::vector<ProcessCoordinationUnitImpl*> process_cus = + ProcessCoordinationUnitImpl::GetAllProcessCoordinationUnits(); + for (auto* process_cu : process_cus) { int64_t pid; if (!process_cu->GetProperty(mojom::PropertyType::kPID, &pid)) continue; @@ -40,12 +39,10 @@ process_info->launch_time = base::Time::FromTimeT(launch_time); } - std::set<CoordinationUnitBase*> page_cus = - process_cu->GetAssociatedCoordinationUnitsOfType( - CoordinationUnitType::kPage); + std::set<PageCoordinationUnitImpl*> page_cus = + process_cu->GetAssociatedPageCoordinationUnits(); std::vector<resource_coordinator::mojom::PageInfoPtr> page_infos; - for (CoordinationUnitBase* cu : page_cus) { - auto* page_cu = CoordinationUnitBase::ToPageCoordinationUnit(cu); + for (PageCoordinationUnitImpl* page_cu : page_cus) { int64_t ukm_source_id; if (page_cu->GetProperty( resource_coordinator::mojom::PropertyType::kUKMSourceId,
diff --git a/services/resource_coordinator/coordination_unit/coordination_unit_provider_impl.cc b/services/resource_coordinator/coordination_unit/coordination_unit_provider_impl.cc index d30d390..f4db74b 100644 --- a/services/resource_coordinator/coordination_unit/coordination_unit_provider_impl.cc +++ b/services/resource_coordinator/coordination_unit/coordination_unit_provider_impl.cc
@@ -8,7 +8,9 @@ #include <utility> #include "base/macros.h" -#include "services/resource_coordinator/coordination_unit/coordination_unit_base.h" +#include "services/resource_coordinator/coordination_unit/frame_coordination_unit_impl.h" +#include "services/resource_coordinator/coordination_unit/page_coordination_unit_impl.h" +#include "services/resource_coordinator/coordination_unit/process_coordination_unit_impl.h" #include "services/service_manager/public/cpp/bind_source_info.h" #include "services/service_manager/public/cpp/service_context_ref.h" @@ -32,20 +34,49 @@ coordination_unit->Destruct(); } -void CoordinationUnitProviderImpl::CreateCoordinationUnit( - mojom::CoordinationUnitRequest request, +void CoordinationUnitProviderImpl::CreateFrameCoordinationUnit( + mojom::FrameCoordinationUnitRequest request, const CoordinationUnitID& id) { - CoordinationUnitBase* coordination_unit = - CoordinationUnitBase::CreateCoordinationUnit( - id, service_ref_factory_->CreateRef()); + FrameCoordinationUnitImpl* frame_cu = + FrameCoordinationUnitImpl::Create(id, service_ref_factory_->CreateRef()); - coordination_unit->Bind(std::move(request)); - coordination_unit_manager_->OnCoordinationUnitCreated(coordination_unit); - auto& coordination_unit_binding = coordination_unit->binding(); + frame_cu->Bind(std::move(request)); + coordination_unit_manager_->OnCoordinationUnitCreated(frame_cu); + auto& frame_cu_binding = frame_cu->binding(); - coordination_unit_binding.set_connection_error_handler( + frame_cu_binding.set_connection_error_handler( base::BindOnce(&CoordinationUnitProviderImpl::OnConnectionError, - base::Unretained(this), coordination_unit)); + base::Unretained(this), frame_cu)); +} + +void CoordinationUnitProviderImpl::CreatePageCoordinationUnit( + mojom::PageCoordinationUnitRequest request, + const CoordinationUnitID& id) { + PageCoordinationUnitImpl* page_cu = + PageCoordinationUnitImpl::Create(id, service_ref_factory_->CreateRef()); + + page_cu->Bind(std::move(request)); + coordination_unit_manager_->OnCoordinationUnitCreated(page_cu); + auto& page_cu_binding = page_cu->binding(); + + page_cu_binding.set_connection_error_handler( + base::BindOnce(&CoordinationUnitProviderImpl::OnConnectionError, + base::Unretained(this), page_cu)); +} + +void CoordinationUnitProviderImpl::CreateProcessCoordinationUnit( + mojom::ProcessCoordinationUnitRequest request, + const CoordinationUnitID& id) { + ProcessCoordinationUnitImpl* process_cu = ProcessCoordinationUnitImpl::Create( + id, service_ref_factory_->CreateRef()); + + process_cu->Bind(std::move(request)); + coordination_unit_manager_->OnCoordinationUnitCreated(process_cu); + auto& process_cu_binding = process_cu->binding(); + + process_cu_binding.set_connection_error_handler( + base::BindOnce(&CoordinationUnitProviderImpl::OnConnectionError, + base::Unretained(this), process_cu)); } void CoordinationUnitProviderImpl::Bind(
diff --git a/services/resource_coordinator/coordination_unit/coordination_unit_provider_impl.h b/services/resource_coordinator/coordination_unit/coordination_unit_provider_impl.h index 6aa193d7..b82fce2 100644 --- a/services/resource_coordinator/coordination_unit/coordination_unit_provider_impl.h +++ b/services/resource_coordinator/coordination_unit/coordination_unit_provider_impl.h
@@ -35,8 +35,14 @@ void OnConnectionError(CoordinationUnitBase* coordination_unit); // Overridden from mojom::CoordinationUnitProvider: - void CreateCoordinationUnit( - resource_coordinator::mojom::CoordinationUnitRequest request, + void CreateFrameCoordinationUnit( + resource_coordinator::mojom::FrameCoordinationUnitRequest request, + const CoordinationUnitID& id) override; + void CreatePageCoordinationUnit( + resource_coordinator::mojom::PageCoordinationUnitRequest request, + const CoordinationUnitID& id) override; + void CreateProcessCoordinationUnit( + resource_coordinator::mojom::ProcessCoordinationUnitRequest request, const CoordinationUnitID& id) override; private:
diff --git a/services/resource_coordinator/coordination_unit/coordination_unit_test_harness.cc b/services/resource_coordinator/coordination_unit/coordination_unit_test_harness.cc index ec15b21..f81c547c 100644 --- a/services/resource_coordinator/coordination_unit/coordination_unit_test_harness.cc +++ b/services/resource_coordinator/coordination_unit/coordination_unit_test_harness.cc
@@ -4,12 +4,8 @@ #include "services/resource_coordinator/coordination_unit/coordination_unit_test_harness.h" -#include <string> - #include "base/bind.h" #include "base/run_loop.h" -#include "services/resource_coordinator/coordination_unit/coordination_unit_base.h" -#include "services/resource_coordinator/public/interfaces/coordination_unit.mojom.h" namespace resource_coordinator { @@ -32,17 +28,4 @@ base::RunLoop().RunUntilIdle(); } -TestCoordinationUnitWrapper CoordinationUnitTestHarness::CreateCoordinationUnit( - CoordinationUnitID cu_id) { - return TestCoordinationUnitWrapper( - CoordinationUnitBase::CreateCoordinationUnit( - cu_id, service_context_ref_factory()->CreateRef())); -} - -TestCoordinationUnitWrapper CoordinationUnitTestHarness::CreateCoordinationUnit( - CoordinationUnitType type) { - CoordinationUnitID cu_id(type, std::string()); - return CreateCoordinationUnit(cu_id); -} - } // namespace resource_coordinator
diff --git a/services/resource_coordinator/coordination_unit/coordination_unit_test_harness.h b/services/resource_coordinator/coordination_unit/coordination_unit_test_harness.h index f8054e93..49c1f51 100644 --- a/services/resource_coordinator/coordination_unit/coordination_unit_test_harness.h +++ b/services/resource_coordinator/coordination_unit/coordination_unit_test_harness.h
@@ -6,8 +6,7 @@ #define SERVICES_RESOURCE_COORDINATOR_COORDINATION_UNIT_COORDINATION_UNIT_TEST_HARNESS_H_ #include <stdint.h> - -#include <memory> +#include <string> #include "base/message_loop/message_loop.h" #include "services/resource_coordinator/coordination_unit/coordination_unit_base.h" @@ -20,22 +19,29 @@ struct CoordinationUnitID; +template <class CoordinationUnitClass> class TestCoordinationUnitWrapper { public: - TestCoordinationUnitWrapper(CoordinationUnitBase* impl) : impl_(impl) { + static TestCoordinationUnitWrapper<CoordinationUnitClass> Create() { + CoordinationUnitID cu_id(CoordinationUnitClass::Type(), std::string()); + return TestCoordinationUnitWrapper<CoordinationUnitClass>( + CoordinationUnitClass::Create(cu_id, nullptr)); + } + + TestCoordinationUnitWrapper(CoordinationUnitClass* impl) : impl_(impl) { DCHECK(impl); } ~TestCoordinationUnitWrapper() { impl_->Destruct(); } - CoordinationUnitBase* operator->() const { return impl_; } + CoordinationUnitClass* operator->() const { return impl_; } TestCoordinationUnitWrapper(TestCoordinationUnitWrapper&& other) : impl_(other.impl_) {} - CoordinationUnitBase* get() const { return impl_; } + CoordinationUnitClass* get() const { return impl_; } private: - CoordinationUnitBase* impl_; + CoordinationUnitClass* impl_; DISALLOW_COPY_AND_ASSIGN(TestCoordinationUnitWrapper); }; @@ -45,12 +51,22 @@ CoordinationUnitTestHarness(); ~CoordinationUnitTestHarness() override; + template <class CoordinationUnitClass> + TestCoordinationUnitWrapper<CoordinationUnitClass> CreateCoordinationUnit( + CoordinationUnitID cu_id) { + return TestCoordinationUnitWrapper<CoordinationUnitClass>( + CoordinationUnitClass::Create(cu_id, service_ref_factory_.CreateRef())); + } + + template <class CoordinationUnitClass> + TestCoordinationUnitWrapper<CoordinationUnitClass> CreateCoordinationUnit() { + CoordinationUnitID cu_id(CoordinationUnitClass::Type(), std::string()); + return CreateCoordinationUnit<CoordinationUnitClass>(cu_id); + } + // testing::Test: void TearDown() override; - TestCoordinationUnitWrapper CreateCoordinationUnit(CoordinationUnitID cu_id); - TestCoordinationUnitWrapper CreateCoordinationUnit(CoordinationUnitType type); - protected: service_manager::ServiceContextRefFactory* service_context_ref_factory() { return &service_ref_factory_;
diff --git a/services/resource_coordinator/coordination_unit/frame_coordination_unit_impl.cc b/services/resource_coordinator/coordination_unit/frame_coordination_unit_impl.cc index cf6cba2..7cbf9900 100644 --- a/services/resource_coordinator/coordination_unit/frame_coordination_unit_impl.cc +++ b/services/resource_coordinator/coordination_unit/frame_coordination_unit_impl.cc
@@ -5,6 +5,7 @@ #include "services/resource_coordinator/coordination_unit/frame_coordination_unit_impl.h" #include "services/resource_coordinator/coordination_unit/page_coordination_unit_impl.h" +#include "services/resource_coordinator/coordination_unit/process_coordination_unit_impl.h" #include "services/resource_coordinator/observers/coordination_unit_graph_observer.h" namespace resource_coordinator { @@ -12,51 +13,47 @@ FrameCoordinationUnitImpl::FrameCoordinationUnitImpl( const CoordinationUnitID& id, std::unique_ptr<service_manager::ServiceContextRef> service_ref) - : CoordinationUnitBase(id, std::move(service_ref)) {} + : CoordinationUnitInterface(id, std::move(service_ref)), + parent_frame_coordination_unit_(nullptr), + page_coordination_unit_(nullptr), + process_coordination_unit_(nullptr) {} -FrameCoordinationUnitImpl::~FrameCoordinationUnitImpl() = default; +FrameCoordinationUnitImpl::~FrameCoordinationUnitImpl() { + if (parent_frame_coordination_unit_) + parent_frame_coordination_unit_->RemoveChildFrame(this); + if (page_coordination_unit_) + page_coordination_unit_->RemoveFrame(this); + if (process_coordination_unit_) + process_coordination_unit_->RemoveFrame(this); + for (auto* child_frame : child_frame_coordination_units_) + child_frame->RemoveParentFrame(this); +} -std::set<CoordinationUnitBase*> -FrameCoordinationUnitImpl::GetAssociatedCoordinationUnitsOfType( - CoordinationUnitType type) const { - switch (type) { - case CoordinationUnitType::kProcess: - case CoordinationUnitType::kPage: - // Processes and Page are only associated with a frame if - // they are a direct parents of the frame. - return GetParentCoordinationUnitsOfType(type); - case CoordinationUnitType::kFrame: { - // Frame Coordination Units form a tree, thus associated frame - // coordination units are all frame coordination units in the tree. Loop - // back to the root frame, get all child frame coordination units from the - // root frame, add the root frame coordination unit and remove the current - // frame coordination unit. - const CoordinationUnitBase* root_frame_coordination_unit = this; - while (true) { - bool has_parent_frame_cu = false; - for (auto* parent : root_frame_coordination_unit->parents()) { - if (parent->id().type == CoordinationUnitType::kFrame) { - root_frame_coordination_unit = parent; - has_parent_frame_cu = true; - break; - } - } - if (has_parent_frame_cu) - continue; - break; - } - auto frame_coordination_units = - root_frame_coordination_unit->GetChildCoordinationUnitsOfType(type); - // Insert the root frame coordination unit. - frame_coordination_units.insert( - const_cast<CoordinationUnitBase*>(root_frame_coordination_unit)); - // Remove itself. - frame_coordination_units.erase( - const_cast<FrameCoordinationUnitImpl*>(this)); - return frame_coordination_units; - } - default: - return std::set<CoordinationUnitBase*>(); +void FrameCoordinationUnitImpl::AddChildFrame(const CoordinationUnitID& cu_id) { + DCHECK(cu_id != id()); + FrameCoordinationUnitImpl* frame_cu = + FrameCoordinationUnitImpl::GetCoordinationUnitByID(cu_id); + if (!frame_cu) + return; + if (HasFrameCoordinationUnitInAncestors(frame_cu) || + frame_cu->HasFrameCoordinationUnitInDescendants(this)) { + DCHECK(false) << "Cyclic reference in frame coordination units detected!"; + return; + } + if (AddChildFrame(frame_cu)) { + frame_cu->AddParentFrame(this); + } +} + +void FrameCoordinationUnitImpl::RemoveChildFrame( + const CoordinationUnitID& cu_id) { + DCHECK(cu_id != id()); + FrameCoordinationUnitImpl* frame_cu = + FrameCoordinationUnitImpl::GetCoordinationUnitByID(cu_id); + if (!frame_cu) + return; + if (RemoveChildFrame(frame_cu)) { + frame_cu->RemoveParentFrame(this); } } @@ -64,7 +61,7 @@ SetProperty(mojom::PropertyType::kAudible, audible); } -void FrameCoordinationUnitImpl::SetNetworkAlmostIdleness(bool idle) { +void FrameCoordinationUnitImpl::SetNetworkAlmostIdle(bool idle) { SetProperty(mojom::PropertyType::kNetworkAlmostIdle, idle); } @@ -76,34 +73,104 @@ SendEvent(mojom::Event::kNonPersistentNotificationCreated); } +FrameCoordinationUnitImpl* +FrameCoordinationUnitImpl::GetParentFrameCoordinationUnit() const { + return parent_frame_coordination_unit_; +} + PageCoordinationUnitImpl* FrameCoordinationUnitImpl::GetPageCoordinationUnit() const { - for (auto* parent : parents_) { - if (parent->id().type != CoordinationUnitType::kPage) - continue; - return CoordinationUnitBase::ToPageCoordinationUnit(parent); - } - return nullptr; + return page_coordination_unit_; +} + +ProcessCoordinationUnitImpl* +FrameCoordinationUnitImpl::GetProcessCoordinationUnit() const { + return process_coordination_unit_; } bool FrameCoordinationUnitImpl::IsMainFrame() const { - for (auto* parent : parents_) { - if (parent->id().type == CoordinationUnitType::kFrame) - return false; - } - return true; + return !parent_frame_coordination_unit_; } -void FrameCoordinationUnitImpl::OnEventReceived(const mojom::Event event) { +void FrameCoordinationUnitImpl::OnEventReceived(mojom::Event event) { for (auto& observer : observers()) observer.OnFrameEventReceived(this, event); } void FrameCoordinationUnitImpl::OnPropertyChanged( - const mojom::PropertyType property_type, + mojom::PropertyType property_type, int64_t value) { for (auto& observer : observers()) observer.OnFramePropertyChanged(this, property_type, value); } +bool FrameCoordinationUnitImpl::HasFrameCoordinationUnitInAncestors( + FrameCoordinationUnitImpl* frame_cu) const { + if (parent_frame_coordination_unit_ == frame_cu || + (parent_frame_coordination_unit_ && + parent_frame_coordination_unit_->HasFrameCoordinationUnitInAncestors( + frame_cu))) { + return true; + } + return false; +} + +bool FrameCoordinationUnitImpl::HasFrameCoordinationUnitInDescendants( + FrameCoordinationUnitImpl* frame_cu) const { + for (FrameCoordinationUnitImpl* child : child_frame_coordination_units_) { + if (child == frame_cu || + child->HasFrameCoordinationUnitInDescendants(frame_cu)) { + return true; + } + } + return false; +} + +void FrameCoordinationUnitImpl::AddParentFrame( + FrameCoordinationUnitImpl* parent_frame_cu) { + parent_frame_coordination_unit_ = parent_frame_cu; +} + +bool FrameCoordinationUnitImpl::AddChildFrame( + FrameCoordinationUnitImpl* child_frame_cu) { + return child_frame_coordination_units_.count(child_frame_cu) + ? false + : child_frame_coordination_units_.insert(child_frame_cu).second; +} + +void FrameCoordinationUnitImpl::RemoveParentFrame( + FrameCoordinationUnitImpl* parent_frame_cu) { + DCHECK(parent_frame_coordination_unit_ == parent_frame_cu); + parent_frame_coordination_unit_ = nullptr; +} + +bool FrameCoordinationUnitImpl::RemoveChildFrame( + FrameCoordinationUnitImpl* child_frame_cu) { + return child_frame_coordination_units_.erase(child_frame_cu) > 0; +} + +void FrameCoordinationUnitImpl::AddPageCoordinationUnit( + PageCoordinationUnitImpl* page_coordination_unit) { + DCHECK(!page_coordination_unit_); + page_coordination_unit_ = page_coordination_unit; +} + +void FrameCoordinationUnitImpl::AddProcessCoordinationUnit( + ProcessCoordinationUnitImpl* process_coordination_unit) { + DCHECK(!process_coordination_unit_); + process_coordination_unit_ = process_coordination_unit; +} + +void FrameCoordinationUnitImpl::RemovePageCoordinationUnit( + PageCoordinationUnitImpl* page_coordination_unit) { + DCHECK(page_coordination_unit == page_coordination_unit_); + page_coordination_unit_ = nullptr; +} + +void FrameCoordinationUnitImpl::RemoveProcessCoordinationUnit( + ProcessCoordinationUnitImpl* process_coordination_unit) { + DCHECK(process_coordination_unit == process_coordination_unit_); + process_coordination_unit_ = nullptr; +} + } // namespace resource_coordinator
diff --git a/services/resource_coordinator/coordination_unit/frame_coordination_unit_impl.h b/services/resource_coordinator/coordination_unit/frame_coordination_unit_impl.h index 7e4db36..7a32ce0 100644 --- a/services/resource_coordinator/coordination_unit/frame_coordination_unit_impl.h +++ b/services/resource_coordinator/coordination_unit/frame_coordination_unit_impl.h
@@ -5,43 +5,78 @@ #ifndef SERVICES_RESOURCE_COORDINATOR_COORDINATION_UNIT_FRAME_COORDINATION_UNIT_IMPL_H_ #define SERVICES_RESOURCE_COORDINATOR_COORDINATION_UNIT_FRAME_COORDINATION_UNIT_IMPL_H_ -#include <set> - #include "base/macros.h" #include "services/resource_coordinator/coordination_unit/coordination_unit_base.h" namespace resource_coordinator { +class PageCoordinationUnitImpl; +class ProcessCoordinationUnitImpl; + // Frame Coordination Units form a tree structure, each FrameCoordinationUnit at // most has one parent that is a FrameCoordinationUnit. // A Frame Coordination Unit will have parents only if navigation committed. -class FrameCoordinationUnitImpl : public CoordinationUnitBase, - public mojom::FrameCoordinationUnit { +class FrameCoordinationUnitImpl + : public CoordinationUnitInterface<FrameCoordinationUnitImpl, + mojom::FrameCoordinationUnit, + mojom::FrameCoordinationUnitRequest> { public: + static CoordinationUnitType Type() { return CoordinationUnitType::kFrame; } + FrameCoordinationUnitImpl( const CoordinationUnitID& id, std::unique_ptr<service_manager::ServiceContextRef> service_ref); ~FrameCoordinationUnitImpl() override; - // CoordinationUnitBase implementation. - std::set<CoordinationUnitBase*> GetAssociatedCoordinationUnitsOfType( - CoordinationUnitType type) const override; - // FrameCoordinationUnit implementation. + void AddChildFrame(const CoordinationUnitID& cu_id) override; + void RemoveChildFrame(const CoordinationUnitID& cu_id) override; void SetAudibility(bool audible) override; - void SetNetworkAlmostIdleness(bool idle) override; + void SetNetworkAlmostIdle(bool idle) override; void OnAlertFired() override; void OnNonPersistentNotificationCreated() override; + FrameCoordinationUnitImpl* GetParentFrameCoordinationUnit() const; PageCoordinationUnitImpl* GetPageCoordinationUnit() const; + ProcessCoordinationUnitImpl* GetProcessCoordinationUnit() const; bool IsMainFrame() const; + const std::set<FrameCoordinationUnitImpl*>& + child_frame_coordination_units_for_testing() const { + return child_frame_coordination_units_; + } + private: - // CoordinationUnitBase implementation. - void OnEventReceived(const mojom::Event event) override; - void OnPropertyChanged(const mojom::PropertyType property_type, + friend class PageCoordinationUnitImpl; + friend class ProcessCoordinationUnitImpl; + + // CoordinationUnitInterface implementation. + void OnEventReceived(mojom::Event event) override; + void OnPropertyChanged(mojom::PropertyType property_type, int64_t value) override; + bool HasFrameCoordinationUnitInAncestors( + FrameCoordinationUnitImpl* frame_cu) const; + bool HasFrameCoordinationUnitInDescendants( + FrameCoordinationUnitImpl* frame_cu) const; + + // The following methods will be called by other FrameCoordinationUnitImpl, + // PageCoordinationUnitImpl and ProcessCoordinationUnitImpl respectively to + // manipulate their relationship. + void AddParentFrame(FrameCoordinationUnitImpl* parent_frame_cu); + bool AddChildFrame(FrameCoordinationUnitImpl* child_frame_cu); + void RemoveParentFrame(FrameCoordinationUnitImpl* parent_frame_cu); + bool RemoveChildFrame(FrameCoordinationUnitImpl* child_frame_cu); + void AddPageCoordinationUnit(PageCoordinationUnitImpl* page_cu); + void AddProcessCoordinationUnit(ProcessCoordinationUnitImpl* process_cu); + void RemovePageCoordinationUnit(PageCoordinationUnitImpl* page_cu); + void RemoveProcessCoordinationUnit(ProcessCoordinationUnitImpl* process_cu); + + FrameCoordinationUnitImpl* parent_frame_coordination_unit_; + PageCoordinationUnitImpl* page_coordination_unit_; + ProcessCoordinationUnitImpl* process_coordination_unit_; + std::set<FrameCoordinationUnitImpl*> child_frame_coordination_units_; + DISALLOW_COPY_AND_ASSIGN(FrameCoordinationUnitImpl); };
diff --git a/services/resource_coordinator/coordination_unit/frame_coordination_unit_impl_unittest.cc b/services/resource_coordinator/coordination_unit/frame_coordination_unit_impl_unittest.cc new file mode 100644 index 0000000..3e3ae63 --- /dev/null +++ b/services/resource_coordinator/coordination_unit/frame_coordination_unit_impl_unittest.cc
@@ -0,0 +1,93 @@ +// 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 "services/resource_coordinator/coordination_unit/frame_coordination_unit_impl.h" +#include "services/resource_coordinator/coordination_unit/coordination_unit_test_harness.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace resource_coordinator { + +namespace { + +class FrameCoordinationUnitImplTest : public CoordinationUnitTestHarness {}; + +using FrameCoordinationUnitImplDeathTest = FrameCoordinationUnitImplTest; + +} // namespace + +TEST_F(FrameCoordinationUnitImplTest, AddChildFrameBasic) { + auto frame1_cu = CreateCoordinationUnit<FrameCoordinationUnitImpl>(); + auto frame2_cu = CreateCoordinationUnit<FrameCoordinationUnitImpl>(); + auto frame3_cu = CreateCoordinationUnit<FrameCoordinationUnitImpl>(); + + frame1_cu->AddChildFrame(frame2_cu->id()); + frame1_cu->AddChildFrame(frame3_cu->id()); + EXPECT_EQ(nullptr, frame1_cu->GetParentFrameCoordinationUnit()); + EXPECT_EQ(2u, frame1_cu->child_frame_coordination_units_for_testing().size()); + EXPECT_EQ(frame1_cu.get(), frame2_cu->GetParentFrameCoordinationUnit()); + EXPECT_EQ(frame1_cu.get(), frame3_cu->GetParentFrameCoordinationUnit()); +} + +TEST_F(FrameCoordinationUnitImplDeathTest, AddChildFrameOnCyclicReference) { + ::testing::FLAGS_gtest_death_test_style = "threadsafe"; + + auto frame1_cu = CreateCoordinationUnit<FrameCoordinationUnitImpl>(); + auto frame2_cu = CreateCoordinationUnit<FrameCoordinationUnitImpl>(); + auto frame3_cu = CreateCoordinationUnit<FrameCoordinationUnitImpl>(); + + frame1_cu->AddChildFrame(frame2_cu->id()); + frame2_cu->AddChildFrame(frame3_cu->id()); +// |frame3_cu| can't add |frame1_cu| because |frame1_cu| is an ancestor of +// |frame3_cu|, and this will hit a DCHECK because of cyclic reference. +#if !defined(NDEBUG) || defined(DCHECK_ALWAYS_ON) + EXPECT_DEATH_IF_SUPPORTED(frame3_cu->AddChildFrame(frame1_cu->id()), ""); +#else + frame3_cu->AddChildFrame(frame1_cu->id()); +#endif // !defined(NDEBUG) || defined(DCHECK_ALWAYS_ON) + + EXPECT_EQ(1u, frame1_cu->child_frame_coordination_units_for_testing().count( + frame2_cu.get())); + EXPECT_EQ(1u, frame2_cu->child_frame_coordination_units_for_testing().count( + frame3_cu.get())); + // |frame1_cu| was not added successfully because |frame1_cu| is one of the + // ancestors of |frame3_cu|. + EXPECT_EQ(0u, frame3_cu->child_frame_coordination_units_for_testing().count( + frame1_cu.get())); +} + +TEST_F(FrameCoordinationUnitImplTest, RemoveChildFrame) { + auto parent_frame_cu = CreateCoordinationUnit<FrameCoordinationUnitImpl>(); + auto child_frame_cu = CreateCoordinationUnit<FrameCoordinationUnitImpl>(); + + // Parent-child relationships have not been established yet. + EXPECT_EQ( + 0u, parent_frame_cu->child_frame_coordination_units_for_testing().size()); + EXPECT_TRUE(!parent_frame_cu->GetParentFrameCoordinationUnit()); + EXPECT_EQ( + 0u, child_frame_cu->child_frame_coordination_units_for_testing().size()); + EXPECT_TRUE(!child_frame_cu->GetParentFrameCoordinationUnit()); + + parent_frame_cu->AddChildFrame(child_frame_cu->id()); + + // Ensure correct Parent-child relationships have been established. + EXPECT_EQ( + 1u, parent_frame_cu->child_frame_coordination_units_for_testing().size()); + EXPECT_TRUE(!parent_frame_cu->GetParentFrameCoordinationUnit()); + EXPECT_EQ( + 0u, child_frame_cu->child_frame_coordination_units_for_testing().size()); + EXPECT_EQ(parent_frame_cu.get(), + child_frame_cu->GetParentFrameCoordinationUnit()); + + parent_frame_cu->RemoveChildFrame(child_frame_cu->id()); + + // Parent-child relationships should no longer exist. + EXPECT_EQ( + 0u, parent_frame_cu->child_frame_coordination_units_for_testing().size()); + EXPECT_TRUE(!parent_frame_cu->GetParentFrameCoordinationUnit()); + EXPECT_EQ( + 0u, child_frame_cu->child_frame_coordination_units_for_testing().size()); + EXPECT_TRUE(!child_frame_cu->GetParentFrameCoordinationUnit()); +} + +} // namespace resource_coordinator
diff --git a/services/resource_coordinator/coordination_unit/mock_coordination_unit_graphs.cc b/services/resource_coordinator/coordination_unit/mock_coordination_unit_graphs.cc index 4fad92b..24e061df 100644 --- a/services/resource_coordinator/coordination_unit/mock_coordination_unit_graphs.cc +++ b/services/resource_coordinator/coordination_unit/mock_coordination_unit_graphs.cc
@@ -7,6 +7,9 @@ #include <string> #include "services/resource_coordinator/coordination_unit/coordination_unit_base.h" +#include "services/resource_coordinator/coordination_unit/frame_coordination_unit_impl.h" +#include "services/resource_coordinator/coordination_unit/page_coordination_unit_impl.h" +#include "services/resource_coordinator/coordination_unit/process_coordination_unit_impl.h" #include "services/resource_coordinator/public/cpp/coordination_unit_id.h" #include "services/resource_coordinator/public/cpp/coordination_unit_types.h" @@ -16,23 +19,14 @@ namespace resource_coordinator { -namespace { - -TestCoordinationUnitWrapper CreateCoordinationUnit(CoordinationUnitType type) { - CoordinationUnitID cu_id(type, std::string()); - return TestCoordinationUnitWrapper( - CoordinationUnitBase::CreateCoordinationUnit(cu_id, nullptr)); -} - -} // namespace - MockSinglePageInSingleProcessCoordinationUnitGraph:: MockSinglePageInSingleProcessCoordinationUnitGraph() - : frame(CreateCoordinationUnit(CoordinationUnitType::kFrame)), - process(CreateCoordinationUnit(CoordinationUnitType::kProcess)), - page(CreateCoordinationUnit(CoordinationUnitType::kPage)) { - page->AddChild(frame->id()); - process->AddChild(frame->id()); + : frame(TestCoordinationUnitWrapper<FrameCoordinationUnitImpl>::Create()), + process( + TestCoordinationUnitWrapper<ProcessCoordinationUnitImpl>::Create()), + page(TestCoordinationUnitWrapper<PageCoordinationUnitImpl>::Create()) { + page->AddFrame(frame->id()); + process->AddFrame(frame->id()); } MockSinglePageInSingleProcessCoordinationUnitGraph:: @@ -40,10 +34,12 @@ MockMultiplePagesInSingleProcessCoordinationUnitGraph:: MockMultiplePagesInSingleProcessCoordinationUnitGraph() - : other_frame(CreateCoordinationUnit(CoordinationUnitType::kFrame)), - other_page(CreateCoordinationUnit(CoordinationUnitType::kPage)) { - other_page->AddChild(other_frame->id()); - process->AddChild(other_frame->id()); + : other_frame( + TestCoordinationUnitWrapper<FrameCoordinationUnitImpl>::Create()), + other_page( + TestCoordinationUnitWrapper<PageCoordinationUnitImpl>::Create()) { + other_page->AddFrame(other_frame->id()); + process->AddFrame(other_frame->id()); } MockMultiplePagesInSingleProcessCoordinationUnitGraph:: @@ -51,11 +47,13 @@ MockSinglePageWithMultipleProcessesCoordinationUnitGraph:: MockSinglePageWithMultipleProcessesCoordinationUnitGraph() - : child_frame(CreateCoordinationUnit(CoordinationUnitType::kFrame)), - other_process(CreateCoordinationUnit(CoordinationUnitType::kProcess)) { - frame->AddChild(child_frame->id()); - page->AddChild(child_frame->id()); - other_process->AddChild(child_frame->id()); + : child_frame( + TestCoordinationUnitWrapper<FrameCoordinationUnitImpl>::Create()), + other_process( + TestCoordinationUnitWrapper<ProcessCoordinationUnitImpl>::Create()) { + frame->AddChildFrame(child_frame->id()); + page->AddFrame(child_frame->id()); + other_process->AddFrame(child_frame->id()); } MockSinglePageWithMultipleProcessesCoordinationUnitGraph:: @@ -63,11 +61,13 @@ MockMultiplePagesWithMultipleProcessesCoordinationUnitGraph:: MockMultiplePagesWithMultipleProcessesCoordinationUnitGraph() - : child_frame(CreateCoordinationUnit(CoordinationUnitType::kFrame)), - other_process(CreateCoordinationUnit(CoordinationUnitType::kProcess)) { - other_frame->AddChild(child_frame->id()); - other_page->AddChild(child_frame->id()); - other_process->AddChild(child_frame->id()); + : child_frame( + TestCoordinationUnitWrapper<FrameCoordinationUnitImpl>::Create()), + other_process( + TestCoordinationUnitWrapper<ProcessCoordinationUnitImpl>::Create()) { + other_frame->AddChildFrame(child_frame->id()); + other_page->AddFrame(child_frame->id()); + other_process->AddFrame(child_frame->id()); } MockMultiplePagesWithMultipleProcessesCoordinationUnitGraph::
diff --git a/services/resource_coordinator/coordination_unit/mock_coordination_unit_graphs.h b/services/resource_coordinator/coordination_unit/mock_coordination_unit_graphs.h index e2b8ac06..338de5b 100644 --- a/services/resource_coordinator/coordination_unit/mock_coordination_unit_graphs.h +++ b/services/resource_coordinator/coordination_unit/mock_coordination_unit_graphs.h
@@ -11,6 +11,10 @@ namespace resource_coordinator { +class FrameCoordinationUnitImpl; +class PageCoordinationUnitImpl; +class ProcessCoordinationUnitImpl; + // The following coordination unit graph topology is created to emulate a // scenario when a single page are executes in a single process: // @@ -25,9 +29,9 @@ struct MockSinglePageInSingleProcessCoordinationUnitGraph { MockSinglePageInSingleProcessCoordinationUnitGraph(); ~MockSinglePageInSingleProcessCoordinationUnitGraph(); - TestCoordinationUnitWrapper frame; - TestCoordinationUnitWrapper process; - TestCoordinationUnitWrapper page; + TestCoordinationUnitWrapper<FrameCoordinationUnitImpl> frame; + TestCoordinationUnitWrapper<ProcessCoordinationUnitImpl> process; + TestCoordinationUnitWrapper<PageCoordinationUnitImpl> page; }; // The following coordination unit graph topology is created to emulate a @@ -47,8 +51,8 @@ : public MockSinglePageInSingleProcessCoordinationUnitGraph { MockMultiplePagesInSingleProcessCoordinationUnitGraph(); ~MockMultiplePagesInSingleProcessCoordinationUnitGraph(); - TestCoordinationUnitWrapper other_frame; - TestCoordinationUnitWrapper other_page; + TestCoordinationUnitWrapper<FrameCoordinationUnitImpl> other_frame; + TestCoordinationUnitWrapper<PageCoordinationUnitImpl> other_page; }; // The following coordination unit graph topology is created to emulate a @@ -71,8 +75,8 @@ : public MockSinglePageInSingleProcessCoordinationUnitGraph { MockSinglePageWithMultipleProcessesCoordinationUnitGraph(); ~MockSinglePageWithMultipleProcessesCoordinationUnitGraph(); - TestCoordinationUnitWrapper child_frame; - TestCoordinationUnitWrapper other_process; + TestCoordinationUnitWrapper<FrameCoordinationUnitImpl> child_frame; + TestCoordinationUnitWrapper<ProcessCoordinationUnitImpl> other_process; }; // The following coordination unit graph topology is created to emulate a @@ -97,8 +101,8 @@ : public MockMultiplePagesInSingleProcessCoordinationUnitGraph { MockMultiplePagesWithMultipleProcessesCoordinationUnitGraph(); ~MockMultiplePagesWithMultipleProcessesCoordinationUnitGraph(); - TestCoordinationUnitWrapper child_frame; - TestCoordinationUnitWrapper other_process; + TestCoordinationUnitWrapper<FrameCoordinationUnitImpl> child_frame; + TestCoordinationUnitWrapper<ProcessCoordinationUnitImpl> other_process; }; } // namespace resource_coordinator
diff --git a/services/resource_coordinator/coordination_unit/page_coordination_unit_impl.cc b/services/resource_coordinator/coordination_unit/page_coordination_unit_impl.cc index 159a79a..85124f37 100644 --- a/services/resource_coordinator/coordination_unit/page_coordination_unit_impl.cc +++ b/services/resource_coordinator/coordination_unit/page_coordination_unit_impl.cc
@@ -7,6 +7,7 @@ #include "base/logging.h" #include "base/time/default_tick_clock.h" #include "services/resource_coordinator/coordination_unit/frame_coordination_unit_impl.h" +#include "services/resource_coordinator/coordination_unit/process_coordination_unit_impl.h" #include "services/resource_coordinator/observers/coordination_unit_graph_observer.h" namespace resource_coordinator { @@ -14,10 +15,33 @@ PageCoordinationUnitImpl::PageCoordinationUnitImpl( const CoordinationUnitID& id, std::unique_ptr<service_manager::ServiceContextRef> service_ref) - : CoordinationUnitBase(id, std::move(service_ref)), + : CoordinationUnitInterface(id, std::move(service_ref)), clock_(new base::DefaultTickClock()) {} -PageCoordinationUnitImpl::~PageCoordinationUnitImpl() = default; +PageCoordinationUnitImpl::~PageCoordinationUnitImpl() { + for (auto* child_frame : frame_coordination_units_) + child_frame->RemovePageCoordinationUnit(this); +} + +void PageCoordinationUnitImpl::AddFrame(const CoordinationUnitID& cu_id) { + DCHECK(cu_id.type == CoordinationUnitType::kFrame); + FrameCoordinationUnitImpl* frame_cu = static_cast<FrameCoordinationUnitImpl*>( + CoordinationUnitBase::GetCoordinationUnitByID(cu_id)); + if (!frame_cu) + return; + if (AddFrame(frame_cu)) + frame_cu->AddPageCoordinationUnit(this); +} + +void PageCoordinationUnitImpl::RemoveFrame(const CoordinationUnitID& cu_id) { + DCHECK(cu_id != id()); + FrameCoordinationUnitImpl* frame_cu = + FrameCoordinationUnitImpl::GetCoordinationUnitByID(cu_id); + if (!frame_cu) + return; + if (RemoveFrame(frame_cu)) + frame_cu->RemovePageCoordinationUnit(this); +} void PageCoordinationUnitImpl::SetVisibility(bool visible) { SetProperty(mojom::PropertyType::kVisible, visible); @@ -39,35 +63,6 @@ SendEvent(mojom::Event::kNavigationCommitted); } -std::set<CoordinationUnitBase*> -PageCoordinationUnitImpl::GetAssociatedCoordinationUnitsOfType( - CoordinationUnitType type) const { - switch (type) { - case CoordinationUnitType::kProcess: { - std::set<CoordinationUnitBase*> process_coordination_units; - - // There is currently not a direct relationship between processes and - // pages. However, frames are children of both processes and frames, so we - // find all of the processes that are reachable from the pages's child - // frames. - for (auto* frame_coordination_unit : - GetChildCoordinationUnitsOfType(CoordinationUnitType::kFrame)) { - for (auto* process_coordination_unit : - frame_coordination_unit->GetAssociatedCoordinationUnitsOfType( - CoordinationUnitType::kProcess)) { - process_coordination_units.insert(process_coordination_unit); - } - } - - return process_coordination_units; - } - case CoordinationUnitType::kFrame: - return GetChildCoordinationUnitsOfType(type); - default: - return std::set<CoordinationUnitBase*>(); - } -} - void PageCoordinationUnitImpl::RecalculateProperty( const mojom::PropertyType property_type) { switch (property_type) { @@ -87,6 +82,18 @@ } } +std::set<ProcessCoordinationUnitImpl*> +PageCoordinationUnitImpl::GetAssociatedProcessCoordinationUnits() const { + std::set<ProcessCoordinationUnitImpl*> process_cus; + + for (auto* frame_cu : frame_coordination_units_) { + if (auto* process_cu = frame_cu->GetProcessCoordinationUnit()) { + process_cus.insert(process_cu); + } + } + return process_cus; +} + bool PageCoordinationUnitImpl::IsVisible() const { int64_t is_visible; bool has_property = GetProperty(mojom::PropertyType::kVisible, &is_visible); @@ -110,7 +117,7 @@ clock_ = std::move(test_clock); } -void PageCoordinationUnitImpl::OnEventReceived(const mojom::Event event) { +void PageCoordinationUnitImpl::OnEventReceived(mojom::Event event) { if (event == mojom::Event::kNavigationCommitted) navigation_committed_time_ = clock_->NowTicks(); for (auto& observer : observers()) @@ -122,62 +129,60 @@ int64_t value) { if (property_type == mojom::PropertyType::kVisible) visibility_change_time_ = clock_->NowTicks(); - for (auto& observer : observers()) { + for (auto& observer : observers()) observer.OnPagePropertyChanged(this, property_type, value); - } } -double PageCoordinationUnitImpl::CalculateCPUUsage() { - double cpu_usage = 0.0; +bool PageCoordinationUnitImpl::AddFrame(FrameCoordinationUnitImpl* frame_cu) { + return frame_coordination_units_.count(frame_cu) + ? false + : frame_coordination_units_.insert(frame_cu).second; +} - for (auto* process_coordination_unit : - GetAssociatedCoordinationUnitsOfType(CoordinationUnitType::kProcess)) { - size_t pages_in_process = - process_coordination_unit - ->GetAssociatedCoordinationUnitsOfType(CoordinationUnitType::kPage) - .size(); - DCHECK_LE(1u, pages_in_process); +bool PageCoordinationUnitImpl::RemoveFrame( + FrameCoordinationUnitImpl* frame_cu) { + return frame_coordination_units_.erase(frame_cu) > 0; +} - int64_t process_cpu_usage; - if (process_coordination_unit->GetProperty(mojom::PropertyType::kCPUUsage, - &process_cpu_usage)) - cpu_usage += (double)process_cpu_usage / pages_in_process; +FrameCoordinationUnitImpl* +PageCoordinationUnitImpl::GetMainFrameCoordinationUnit() { + for (auto* frame_cu : frame_coordination_units_) { + if (frame_cu->IsMainFrame()) + return frame_cu; } - - return cpu_usage; + return nullptr; } bool PageCoordinationUnitImpl::CalculateExpectedTaskQueueingDuration( int64_t* output) { // Calculate the EQT for the process of the main frame only because // the smoothness of the main frame may affect the users the most. - CoordinationUnitBase* main_frame_cu = GetMainFrameCoordinationUnit(); + FrameCoordinationUnitImpl* main_frame_cu = GetMainFrameCoordinationUnit(); if (!main_frame_cu) return false; - - auto associated_processes = - main_frame_cu->GetAssociatedCoordinationUnitsOfType( - CoordinationUnitType::kProcess); - - size_t num_processes_per_frame = associated_processes.size(); - // A frame should not belong to more than 1 process. - DCHECK_LE(num_processes_per_frame, 1u); - - if (num_processes_per_frame == 0) + auto* associated_process = main_frame_cu->GetProcessCoordinationUnit(); + if (!associated_process) return false; - - return (*associated_processes.begin()) - ->GetProperty(mojom::PropertyType::kExpectedTaskQueueingDuration, output); + return associated_process->GetProperty( + mojom::PropertyType::kExpectedTaskQueueingDuration, output); } -CoordinationUnitBase* PageCoordinationUnitImpl::GetMainFrameCoordinationUnit() { - for (auto* cu : - GetAssociatedCoordinationUnitsOfType(CoordinationUnitType::kFrame)) { - if (ToFrameCoordinationUnit(cu)->IsMainFrame()) - return cu; +double PageCoordinationUnitImpl::CalculateCPUUsage() { + double cpu_usage = 0.0; + + for (auto* process_cu : GetAssociatedProcessCoordinationUnits()) { + size_t pages_in_process = + process_cu->GetAssociatedPageCoordinationUnits().size(); + DCHECK_LE(1u, pages_in_process); + + int64_t process_cpu_usage; + if (process_cu->GetProperty(mojom::PropertyType::kCPUUsage, + &process_cpu_usage)) { + cpu_usage += static_cast<double>(process_cpu_usage) / pages_in_process; + } } - return nullptr; + return cpu_usage; } } // namespace resource_coordinator
diff --git a/services/resource_coordinator/coordination_unit/page_coordination_unit_impl.h b/services/resource_coordinator/coordination_unit/page_coordination_unit_impl.h index 416b9ce5..e88b2c5 100644 --- a/services/resource_coordinator/coordination_unit/page_coordination_unit_impl.h +++ b/services/resource_coordinator/coordination_unit/page_coordination_unit_impl.h
@@ -5,8 +5,6 @@ #ifndef SERVICES_RESOURCE_COORDINATOR_COORDINATION_UNIT_PAGE_COORDINATION_UNIT_IMPL_H_ #define SERVICES_RESOURCE_COORDINATOR_COORDINATION_UNIT_PAGE_COORDINATION_UNIT_IMPL_H_ -#include <set> - #include "base/macros.h" #include "base/time/tick_clock.h" #include "base/time/time.h" @@ -14,15 +12,24 @@ namespace resource_coordinator { -class PageCoordinationUnitImpl : public CoordinationUnitBase, - public mojom::PageCoordinationUnit { +class FrameCoordinationUnitImpl; +class ProcessCoordinationUnitImpl; + +class PageCoordinationUnitImpl + : public CoordinationUnitInterface<PageCoordinationUnitImpl, + mojom::PageCoordinationUnit, + mojom::PageCoordinationUnitRequest> { public: + static CoordinationUnitType Type() { return CoordinationUnitType::kPage; } + PageCoordinationUnitImpl( const CoordinationUnitID& id, std::unique_ptr<service_manager::ServiceContextRef> service_ref); ~PageCoordinationUnitImpl() override; // mojom::PageCoordinationUnit implementation. + void AddFrame(const CoordinationUnitID& cu_id) override; + void RemoveFrame(const CoordinationUnitID& cu_id) override; void SetVisibility(bool visible) override; void SetUKMSourceId(int64_t ukm_source_id) override; void OnFaviconUpdated() override; @@ -30,14 +37,19 @@ void OnMainFrameNavigationCommitted() override; // CoordinationUnitBase implementation. - std::set<CoordinationUnitBase*> GetAssociatedCoordinationUnitsOfType( - CoordinationUnitType type) const override; void RecalculateProperty(const mojom::PropertyType property_type) override; + // There is no direct relationship between processes and pages. However, + // frames are accessible by both processes and frames, so we find all of the + // processes that are reachable from the pages's accessible frames. + std::set<ProcessCoordinationUnitImpl*> GetAssociatedProcessCoordinationUnits() + const; bool IsVisible() const; + // Returns 0 if no navigation has happened, otherwise returns the time since // the last navigation commit. base::TimeDelta TimeSinceLastNavigation() const; + // Returns the time since the last visibility change, it should always have a // value since we set the visibility property when we create a // PageCoordinationUnit. @@ -45,18 +57,30 @@ void SetClockForTest(std::unique_ptr<base::TickClock> test_clock); + const std::set<FrameCoordinationUnitImpl*>& + frame_coordination_units_for_testing() const { + return frame_coordination_units_; + } + private: - // CoordinationUnitBase implementation. - void OnEventReceived(const mojom::Event event) override; - void OnPropertyChanged(const mojom::PropertyType property_type, + friend class FrameCoordinationUnitImpl; + + // CoordinationUnitInterface implementation. + void OnEventReceived(mojom::Event event) override; + void OnPropertyChanged(mojom::PropertyType property_type, int64_t value) override; - double CalculateCPUUsage(); + + bool AddFrame(FrameCoordinationUnitImpl* frame_cu); + bool RemoveFrame(FrameCoordinationUnitImpl* frame_cu); // Returns true for a valid value. Returns false otherwise. bool CalculateExpectedTaskQueueingDuration(int64_t* output); + double CalculateCPUUsage(); // Returns the main frame CU or nullptr if this page has no main frame. - CoordinationUnitBase* GetMainFrameCoordinationUnit(); + FrameCoordinationUnitImpl* GetMainFrameCoordinationUnit(); + + std::set<FrameCoordinationUnitImpl*> frame_coordination_units_; std::unique_ptr<base::TickClock> clock_; base::TimeTicks visibility_change_time_;
diff --git a/services/resource_coordinator/coordination_unit/page_coordination_unit_impl_unittest.cc b/services/resource_coordinator/coordination_unit/page_coordination_unit_impl_unittest.cc index 653b0351..f017af16 100644 --- a/services/resource_coordinator/coordination_unit/page_coordination_unit_impl_unittest.cc +++ b/services/resource_coordinator/coordination_unit/page_coordination_unit_impl_unittest.cc
@@ -5,9 +5,11 @@ #include "services/resource_coordinator/coordination_unit/page_coordination_unit_impl.h" #include "base/run_loop.h" #include "base/test/simple_test_tick_clock.h" -#include "services/resource_coordinator/coordination_unit/coordination_unit_base.h" #include "services/resource_coordinator/coordination_unit/coordination_unit_test_harness.h" +#include "services/resource_coordinator/coordination_unit/frame_coordination_unit_impl.h" #include "services/resource_coordinator/coordination_unit/mock_coordination_unit_graphs.h" +#include "services/resource_coordinator/coordination_unit/page_coordination_unit_impl.h" +#include "services/resource_coordinator/coordination_unit/process_coordination_unit_impl.h" #include "services/resource_coordinator/public/cpp/coordination_unit_types.h" #include "services/resource_coordinator/public/interfaces/coordination_unit.mojom.h" #include "testing/gtest/include/gtest/gtest.h" @@ -20,68 +22,114 @@ } // namespace +TEST_F(PageCoordinationUnitImplTest, AddFrameBasic) { + auto page_cu = CreateCoordinationUnit<PageCoordinationUnitImpl>(); + auto frame1_cu = CreateCoordinationUnit<FrameCoordinationUnitImpl>(); + auto frame2_cu = CreateCoordinationUnit<FrameCoordinationUnitImpl>(); + auto frame3_cu = CreateCoordinationUnit<FrameCoordinationUnitImpl>(); + + page_cu->AddFrame(frame1_cu->id()); + page_cu->AddFrame(frame2_cu->id()); + page_cu->AddFrame(frame3_cu->id()); + EXPECT_EQ(3u, page_cu->frame_coordination_units_for_testing().size()); +} + +TEST_F(PageCoordinationUnitImplTest, AddReduplicativeFrame) { + auto page_cu = CreateCoordinationUnit<PageCoordinationUnitImpl>(); + auto frame1_cu = CreateCoordinationUnit<FrameCoordinationUnitImpl>(); + auto frame2_cu = CreateCoordinationUnit<FrameCoordinationUnitImpl>(); + + page_cu->AddFrame(frame1_cu->id()); + page_cu->AddFrame(frame2_cu->id()); + page_cu->AddFrame(frame1_cu->id()); + EXPECT_EQ(2u, page_cu->frame_coordination_units_for_testing().size()); +} + +TEST_F(PageCoordinationUnitImplTest, RemoveFrame) { + auto page_cu = CreateCoordinationUnit<PageCoordinationUnitImpl>(); + auto frame_cu = CreateCoordinationUnit<FrameCoordinationUnitImpl>(); + + // Parent-child relationships have not been established yet. + EXPECT_EQ(0u, page_cu->frame_coordination_units_for_testing().size()); + EXPECT_FALSE(frame_cu->GetPageCoordinationUnit()); + + page_cu->AddFrame(frame_cu->id()); + + // Ensure correct Parent-child relationships have been established. + EXPECT_EQ(1u, page_cu->frame_coordination_units_for_testing().size()); + EXPECT_EQ(1u, page_cu->frame_coordination_units_for_testing().count( + frame_cu.get())); + EXPECT_EQ(page_cu.get(), frame_cu->GetPageCoordinationUnit()); + + page_cu->RemoveFrame(frame_cu->id()); + + // Parent-child relationships should no longer exist. + EXPECT_EQ(0u, page_cu->frame_coordination_units_for_testing().size()); + EXPECT_FALSE(frame_cu->GetPageCoordinationUnit()); +} + TEST_F(PageCoordinationUnitImplTest, CalculatePageCPUUsageForSinglePageInSingleProcess) { MockSinglePageInSingleProcessCoordinationUnitGraph cu_graph; - cu_graph.process->SetProperty(mojom::PropertyType::kCPUUsage, 40); + cu_graph.process->SetCPUUsage(40); int64_t cpu_usage; EXPECT_TRUE( cu_graph.page->GetProperty(mojom::PropertyType::kCPUUsage, &cpu_usage)); - EXPECT_EQ(40, cpu_usage); + EXPECT_EQ(40, cpu_usage / 1000); } TEST_F(PageCoordinationUnitImplTest, CalculatePageCPUUsageForMultiplePagesInSingleProcess) { MockMultiplePagesInSingleProcessCoordinationUnitGraph cu_graph; - cu_graph.process->SetProperty(mojom::PropertyType::kCPUUsage, 40); + cu_graph.process->SetCPUUsage(40); int64_t cpu_usage; EXPECT_TRUE( cu_graph.page->GetProperty(mojom::PropertyType::kCPUUsage, &cpu_usage)); - EXPECT_EQ(20, cpu_usage); + EXPECT_EQ(20, cpu_usage / 1000); EXPECT_TRUE(cu_graph.other_page->GetProperty(mojom::PropertyType::kCPUUsage, &cpu_usage)); - EXPECT_EQ(20, cpu_usage); + EXPECT_EQ(20, cpu_usage / 1000); } TEST_F(PageCoordinationUnitImplTest, CalculatePageCPUUsageForSinglePageWithMultipleProcesses) { MockSinglePageWithMultipleProcessesCoordinationUnitGraph cu_graph; - cu_graph.process->SetProperty(mojom::PropertyType::kCPUUsage, 40); - cu_graph.other_process->SetProperty(mojom::PropertyType::kCPUUsage, 30); + cu_graph.process->SetCPUUsage(40); + cu_graph.other_process->SetCPUUsage(30); int64_t cpu_usage; EXPECT_TRUE( cu_graph.page->GetProperty(mojom::PropertyType::kCPUUsage, &cpu_usage)); - EXPECT_EQ(70, cpu_usage); + EXPECT_EQ(70, cpu_usage / 1000); } TEST_F(PageCoordinationUnitImplTest, CalculatePageCPUUsageForMultiplePagesWithMultipleProcesses) { MockMultiplePagesWithMultipleProcessesCoordinationUnitGraph cu_graph; - cu_graph.process->SetProperty(mojom::PropertyType::kCPUUsage, 40); - cu_graph.other_process->SetProperty(mojom::PropertyType::kCPUUsage, 30); + cu_graph.process->SetCPUUsage(40); + cu_graph.other_process->SetCPUUsage(30); int64_t cpu_usage; EXPECT_TRUE( cu_graph.page->GetProperty(mojom::PropertyType::kCPUUsage, &cpu_usage)); - EXPECT_EQ(20, cpu_usage); + EXPECT_EQ(20, cpu_usage / 1000); EXPECT_TRUE(cu_graph.other_page->GetProperty(mojom::PropertyType::kCPUUsage, &cpu_usage)); - EXPECT_EQ(50, cpu_usage); + EXPECT_EQ(50, cpu_usage / 1000); } TEST_F(PageCoordinationUnitImplTest, CalculatePageEQTForSinglePageInSingleProcess) { MockSinglePageInSingleProcessCoordinationUnitGraph cu_graph; - cu_graph.process->SetProperty( - mojom::PropertyType::kExpectedTaskQueueingDuration, 1); + cu_graph.process->SetExpectedTaskQueueingDuration( + base::TimeDelta::FromMilliseconds(1)); int64_t eqt; ASSERT_TRUE(cu_graph.page->GetProperty( @@ -93,8 +141,8 @@ CalculatePageEQTForMultiplePagesInSingleProcess) { MockMultiplePagesInSingleProcessCoordinationUnitGraph cu_graph; - cu_graph.process->SetProperty( - mojom::PropertyType::kExpectedTaskQueueingDuration, 1); + cu_graph.process->SetExpectedTaskQueueingDuration( + base::TimeDelta::FromMilliseconds(1)); int64_t eqt; ASSERT_TRUE(cu_graph.page->GetProperty( @@ -108,21 +156,20 @@ TEST_F(PageCoordinationUnitImplTest, TimeSinceLastVisibilityChange) { MockSinglePageInSingleProcessCoordinationUnitGraph cu_graph; base::SimpleTestTickClock* clock = new base::SimpleTestTickClock(); - PageCoordinationUnitImpl* page_cu = - CoordinationUnitBase::ToPageCoordinationUnit(cu_graph.page.get()); - page_cu->SetClockForTest(std::unique_ptr<base::SimpleTestTickClock>(clock)); + cu_graph.page->SetClockForTest( + std::unique_ptr<base::SimpleTestTickClock>(clock)); - cu_graph.page->SetProperty(mojom::PropertyType::kVisible, true); - EXPECT_TRUE(page_cu->IsVisible()); + cu_graph.page->SetVisibility(true); + EXPECT_TRUE(cu_graph.page->IsVisible()); clock->Advance(base::TimeDelta::FromSeconds(42)); EXPECT_EQ(base::TimeDelta::FromSeconds(42), - page_cu->TimeSinceLastVisibilityChange()); + cu_graph.page->TimeSinceLastVisibilityChange()); - cu_graph.page->SetProperty(mojom::PropertyType::kVisible, false); + cu_graph.page->SetVisibility(false); clock->Advance(base::TimeDelta::FromSeconds(23)); EXPECT_EQ(base::TimeDelta::FromSeconds(23), - page_cu->TimeSinceLastVisibilityChange()); - EXPECT_FALSE(page_cu->IsVisible()); + cu_graph.page->TimeSinceLastVisibilityChange()); + EXPECT_FALSE(cu_graph.page->IsVisible()); // The clock is owned and destructed by the page cu. clock = nullptr; } @@ -132,23 +179,22 @@ base::SimpleTestTickClock* clock = new base::SimpleTestTickClock(); // Sets a valid starting time. clock->SetNowTicks(base::TimeTicks::Now()); - PageCoordinationUnitImpl* page_cu = - CoordinationUnitBase::ToPageCoordinationUnit(cu_graph.page.get()); - page_cu->SetClockForTest(std::unique_ptr<base::SimpleTestTickClock>(clock)); + cu_graph.page->SetClockForTest( + std::unique_ptr<base::SimpleTestTickClock>(clock)); // Before any commit events, timedelta should be 0. - EXPECT_TRUE(page_cu->TimeSinceLastNavigation().is_zero()); + EXPECT_TRUE(cu_graph.page->TimeSinceLastNavigation().is_zero()); // 1st navigation. - cu_graph.page->SendEvent(mojom::Event::kNavigationCommitted); + cu_graph.page->OnMainFrameNavigationCommitted(); clock->Advance(base::TimeDelta::FromSeconds(11)); EXPECT_EQ(base::TimeDelta::FromSeconds(11), - page_cu->TimeSinceLastNavigation()); + cu_graph.page->TimeSinceLastNavigation()); // 2nd navigation. - cu_graph.page->SendEvent(mojom::Event::kNavigationCommitted); + cu_graph.page->OnMainFrameNavigationCommitted(); clock->Advance(base::TimeDelta::FromSeconds(17)); EXPECT_EQ(base::TimeDelta::FromSeconds(17), - page_cu->TimeSinceLastNavigation()); + cu_graph.page->TimeSinceLastNavigation()); // The clock is owned and destructed by the page cu. clock = nullptr; }
diff --git a/services/resource_coordinator/coordination_unit/process_coordination_unit_impl.cc b/services/resource_coordinator/coordination_unit/process_coordination_unit_impl.cc index b1bed07..d350bd90 100644 --- a/services/resource_coordinator/coordination_unit/process_coordination_unit_impl.cc +++ b/services/resource_coordinator/coordination_unit/process_coordination_unit_impl.cc
@@ -5,18 +5,54 @@ #include "services/resource_coordinator/coordination_unit/process_coordination_unit_impl.h" #include "base/logging.h" -#include "base/values.h" #include "services/resource_coordinator/coordination_unit/frame_coordination_unit_impl.h" #include "services/resource_coordinator/coordination_unit/page_coordination_unit_impl.h" namespace resource_coordinator { +// static +std::vector<ProcessCoordinationUnitImpl*> +ProcessCoordinationUnitImpl::GetAllProcessCoordinationUnits() { + auto cus = CoordinationUnitBase::GetCoordinationUnitsOfType( + CoordinationUnitType::kProcess); + std::vector<ProcessCoordinationUnitImpl*> process_cus; + for (auto* process_cu : cus) { + process_cus.push_back( + static_cast<ProcessCoordinationUnitImpl*>(process_cu)); + } + return process_cus; +} + ProcessCoordinationUnitImpl::ProcessCoordinationUnitImpl( const CoordinationUnitID& id, std::unique_ptr<service_manager::ServiceContextRef> service_ref) - : CoordinationUnitBase(id, std::move(service_ref)) {} + : CoordinationUnitInterface(id, std::move(service_ref)) {} -ProcessCoordinationUnitImpl::~ProcessCoordinationUnitImpl() = default; +ProcessCoordinationUnitImpl::~ProcessCoordinationUnitImpl() { + for (auto* child_frame : frame_coordination_units_) + child_frame->RemoveProcessCoordinationUnit(this); +} + +void ProcessCoordinationUnitImpl::AddFrame(const CoordinationUnitID& cu_id) { + DCHECK(cu_id.type == CoordinationUnitType::kFrame); + auto* frame_cu = FrameCoordinationUnitImpl::GetCoordinationUnitByID(cu_id); + if (!frame_cu) + return; + if (AddFrame(frame_cu)) { + frame_cu->AddProcessCoordinationUnit(this); + } +} + +void ProcessCoordinationUnitImpl::RemoveFrame(const CoordinationUnitID& cu_id) { + DCHECK(cu_id != id()); + FrameCoordinationUnitImpl* frame_cu = + FrameCoordinationUnitImpl::GetCoordinationUnitByID(cu_id); + if (!frame_cu) + return; + if (RemoveFrame(frame_cu)) { + frame_cu->RemoveProcessCoordinationUnit(this); + } +} void ProcessCoordinationUnitImpl::SetCPUUsage(double cpu_usage) { SetProperty(mojom::PropertyType::kCPUUsage, cpu_usage * 1000); @@ -36,32 +72,18 @@ SetProperty(mojom::PropertyType::kPID, pid); } -std::set<CoordinationUnitBase*> -ProcessCoordinationUnitImpl::GetAssociatedCoordinationUnitsOfType( - CoordinationUnitType type) const { - switch (type) { - case CoordinationUnitType::kPage: { - // There is currently not a direct relationship between processes and - // pages. However, frames are children of both processes and frames, so we - // find all of the pages that are reachable from the process's child - // frames. - std::set<CoordinationUnitBase*> page_cus; - - for (auto* frame_cu : - GetChildCoordinationUnitsOfType(CoordinationUnitType::kFrame)) { - for (auto* page_cu : frame_cu->GetAssociatedCoordinationUnitsOfType( - CoordinationUnitType::kPage)) { - page_cus.insert(page_cu); - } - } - - return page_cus; - } - case CoordinationUnitType::kFrame: - return GetChildCoordinationUnitsOfType(type); - default: - return std::set<CoordinationUnitBase*>(); +// There is currently not a direct relationship between processes and +// pages. However, frames are children of both processes and frames, so we +// find all of the pages that are reachable from the process's child +// frames. +std::set<PageCoordinationUnitImpl*> +ProcessCoordinationUnitImpl::GetAssociatedPageCoordinationUnits() const { + std::set<PageCoordinationUnitImpl*> page_cus; + for (auto* frame_cu : frame_coordination_units_) { + if (auto* page_cu = frame_cu->GetPageCoordinationUnit()) + page_cus.insert(page_cu); } + return page_cus; } void ProcessCoordinationUnitImpl::PropagateProperty( @@ -70,22 +92,17 @@ switch (property_type) { // Trigger Page CU to recalculate their CPU usage. case mojom::PropertyType::kCPUUsage: { - for (auto* page_cu : - GetAssociatedCoordinationUnitsOfType(CoordinationUnitType::kPage)) { + for (auto* page_cu : GetAssociatedPageCoordinationUnits()) { page_cu->RecalculateProperty(mojom::PropertyType::kCPUUsage); } break; } case mojom::PropertyType::kExpectedTaskQueueingDuration: { // Do not propagate if the associated frame is not the main frame. - for (auto* cu : - GetAssociatedCoordinationUnitsOfType(CoordinationUnitType::kFrame)) { - auto* frame_cu = ToFrameCoordinationUnit(cu); + for (auto* frame_cu : frame_coordination_units_) { if (!frame_cu->IsMainFrame()) continue; - auto* page_cu = frame_cu->GetPageCoordinationUnit(); - if (page_cu) { page_cu->RecalculateProperty( mojom::PropertyType::kExpectedTaskQueueingDuration); @@ -98,4 +115,17 @@ } } +bool ProcessCoordinationUnitImpl::AddFrame( + FrameCoordinationUnitImpl* frame_cu) { + bool success = frame_coordination_units_.count(frame_cu) + ? false + : frame_coordination_units_.insert(frame_cu).second; + return success; +} + +bool ProcessCoordinationUnitImpl::RemoveFrame( + FrameCoordinationUnitImpl* frame_cu) { + return frame_coordination_units_.erase(frame_cu) > 0; +} + } // namespace resource_coordinator
diff --git a/services/resource_coordinator/coordination_unit/process_coordination_unit_impl.h b/services/resource_coordinator/coordination_unit/process_coordination_unit_impl.h index d9cced5d..2955073 100644 --- a/services/resource_coordinator/coordination_unit/process_coordination_unit_impl.h +++ b/services/resource_coordinator/coordination_unit/process_coordination_unit_impl.h
@@ -5,36 +5,51 @@ #ifndef SERVICES_RESOURCE_COORDINATOR_COORDINATION_UNIT_PROCESS_COORDINATION_UNIT_IMPL_H_ #define SERVICES_RESOURCE_COORDINATOR_COORDINATION_UNIT_PROCESS_COORDINATION_UNIT_IMPL_H_ -#include <set> - #include "base/macros.h" #include "services/resource_coordinator/coordination_unit/coordination_unit_base.h" namespace resource_coordinator { -class ProcessCoordinationUnitImpl : public CoordinationUnitBase, - public mojom::ProcessCoordinationUnit { +class FrameCoordinationUnitImpl; +class ProcessCoordinationUnitImpl; + +class ProcessCoordinationUnitImpl + : public CoordinationUnitInterface<ProcessCoordinationUnitImpl, + mojom::ProcessCoordinationUnit, + mojom::ProcessCoordinationUnitRequest> { public: + static std::vector<ProcessCoordinationUnitImpl*> + GetAllProcessCoordinationUnits(); + static CoordinationUnitType Type() { return CoordinationUnitType::kProcess; } + ProcessCoordinationUnitImpl( const CoordinationUnitID& id, std::unique_ptr<service_manager::ServiceContextRef> service_ref); ~ProcessCoordinationUnitImpl() override; - // CoordinationUnitBase implementation. // mojom::ProcessCoordinationUnit implementation. + void AddFrame(const CoordinationUnitID& cu_id) override; + void RemoveFrame(const CoordinationUnitID& cu_id) override; void SetCPUUsage(double cpu_usage) override; void SetExpectedTaskQueueingDuration(base::TimeDelta duration) override; void SetLaunchTime(base::Time launch_time) override; void SetPID(int64_t pid) override; - std::set<CoordinationUnitBase*> GetAssociatedCoordinationUnitsOfType( - CoordinationUnitType type) const override; + std::set<PageCoordinationUnitImpl*> GetAssociatedPageCoordinationUnits() + const; private: - // CoordinationUnitBase implementation. + friend class FrameCoordinationUnitImpl; + + // CoordinationUnitInterface implementation. void PropagateProperty(mojom::PropertyType property_type, int64_t value) override; + bool AddFrame(FrameCoordinationUnitImpl* frame_cu); + bool RemoveFrame(FrameCoordinationUnitImpl* frame_cu); + + std::set<FrameCoordinationUnitImpl*> frame_coordination_units_; + DISALLOW_COPY_AND_ASSIGN(ProcessCoordinationUnitImpl); };
diff --git a/services/resource_coordinator/coordination_unit/process_coordination_unit_impl_unittest.cc b/services/resource_coordinator/coordination_unit/process_coordination_unit_impl_unittest.cc index 3029fa7..6524cb5 100644 --- a/services/resource_coordinator/coordination_unit/process_coordination_unit_impl_unittest.cc +++ b/services/resource_coordinator/coordination_unit/process_coordination_unit_impl_unittest.cc
@@ -2,14 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "base/process/process_handle.h" -#include "base/run_loop.h" -#include "services/resource_coordinator/coordination_unit/coordination_unit_base.h" -#include "services/resource_coordinator/coordination_unit/coordination_unit_factory.h" +#include "services/resource_coordinator/coordination_unit/process_coordination_unit_impl.h" #include "services/resource_coordinator/coordination_unit/coordination_unit_test_harness.h" -#include "services/resource_coordinator/public/cpp/coordination_unit_id.h" -#include "services/resource_coordinator/public/cpp/coordination_unit_types.h" -#include "services/resource_coordinator/public/interfaces/coordination_unit.mojom.h" #include "testing/gtest/include/gtest/gtest.h" namespace resource_coordinator { @@ -21,18 +15,12 @@ } // namespace TEST_F(ProcessCoordinationUnitImplTest, MeasureCPUUsage) { - base::ProcessId current_pid = base::GetCurrentProcId(); - CoordinationUnitID cu_id(CoordinationUnitType::kProcess, current_pid); - - std::unique_ptr<CoordinationUnitBase> coordination_unit_ = - coordination_unit_factory::CreateCoordinationUnit( - cu_id, service_context_ref_factory()->CreateRef()); - - base::RunLoop().RunUntilIdle(); - - base::Value cpu_usage_value = - coordination_unit_->GetProperty(mojom::PropertyType::kCPUUsage); - EXPECT_LE(0.0, cpu_usage_value.GetDouble()); + auto process_cu = CreateCoordinationUnit<ProcessCoordinationUnitImpl>(); + process_cu->SetCPUUsage(1); + int64_t cpu_usage; + EXPECT_TRUE( + process_cu->GetProperty(mojom::PropertyType::kCPUUsage, &cpu_usage)); + EXPECT_EQ(1, cpu_usage / 1000.0); } } // namespace resource_coordinator
diff --git a/services/resource_coordinator/observers/coordination_unit_graph_observer_unittest.cc b/services/resource_coordinator/observers/coordination_unit_graph_observer_unittest.cc index 30b797c..fa25e37 100644 --- a/services/resource_coordinator/observers/coordination_unit_graph_observer_unittest.cc +++ b/services/resource_coordinator/observers/coordination_unit_graph_observer_unittest.cc
@@ -4,12 +4,11 @@ #include "services/resource_coordinator/observers/coordination_unit_graph_observer.h" -#include <string> - #include "base/process/process_handle.h" -#include "services/resource_coordinator/coordination_unit/coordination_unit_base.h" #include "services/resource_coordinator/coordination_unit/coordination_unit_manager.h" #include "services/resource_coordinator/coordination_unit/coordination_unit_test_harness.h" +#include "services/resource_coordinator/coordination_unit/frame_coordination_unit_impl.h" +#include "services/resource_coordinator/coordination_unit/process_coordination_unit_impl.h" #include "services/resource_coordinator/public/cpp/coordination_unit_id.h" #include "services/resource_coordinator/public/cpp/coordination_unit_types.h" #include "services/resource_coordinator/public/interfaces/coordination_unit.mojom.h" @@ -73,38 +72,28 @@ static_cast<TestCoordinationUnitGraphObserver*>( coordination_unit_manager().observers_for_testing()[0].get()); - CoordinationUnitID process_cu_id(CoordinationUnitType::kProcess, - std::string()); - CoordinationUnitID root_frame_cu_id(CoordinationUnitType::kFrame, - std::string()); - CoordinationUnitID frame_cu_id(CoordinationUnitType::kFrame, std::string()); + auto process_cu = CreateCoordinationUnit<ProcessCoordinationUnitImpl>(); + auto root_frame_cu = CreateCoordinationUnit<FrameCoordinationUnitImpl>(); + auto frame_cu = CreateCoordinationUnit<FrameCoordinationUnitImpl>(); - auto process_coordination_unit = CreateCoordinationUnit(process_cu_id); - auto root_frame_coordination_unit = CreateCoordinationUnit(root_frame_cu_id); - auto frame_coordination_unit = CreateCoordinationUnit(frame_cu_id); - - coordination_unit_manager().OnCoordinationUnitCreated( - process_coordination_unit.get()); - coordination_unit_manager().OnCoordinationUnitCreated( - root_frame_coordination_unit.get()); - coordination_unit_manager().OnCoordinationUnitCreated( - frame_coordination_unit.get()); + coordination_unit_manager().OnCoordinationUnitCreated(process_cu.get()); + coordination_unit_manager().OnCoordinationUnitCreated(root_frame_cu.get()); + coordination_unit_manager().OnCoordinationUnitCreated(frame_cu.get()); EXPECT_EQ(2u, observer->coordination_unit_created_count()); // The registered observer will only observe the events that happen to // |root_frame_coordination_unit| and |frame_coordination_unit| because // they are CoordinationUnitType::kFrame, so OnPropertyChanged // will only be called for |root_frame_coordination_unit|. - root_frame_coordination_unit->SetProperty(mojom::PropertyType::kTest, 42); - process_coordination_unit->SetProperty(mojom::PropertyType::kTest, 42); + root_frame_cu->SetPropertyForTesting(42); + process_cu->SetPropertyForTesting(42); EXPECT_EQ(1u, observer->property_changed_count()); coordination_unit_manager().OnBeforeCoordinationUnitDestroyed( - process_coordination_unit.get()); + process_cu.get()); coordination_unit_manager().OnBeforeCoordinationUnitDestroyed( - root_frame_coordination_unit.get()); - coordination_unit_manager().OnBeforeCoordinationUnitDestroyed( - frame_coordination_unit.get()); + root_frame_cu.get()); + coordination_unit_manager().OnBeforeCoordinationUnitDestroyed(frame_cu.get()); EXPECT_EQ(2u, observer->coordination_unit_destroyed_count()); }
diff --git a/services/resource_coordinator/observers/metrics_collector.cc b/services/resource_coordinator/observers/metrics_collector.cc index d0c58e1..1561c203 100644 --- a/services/resource_coordinator/observers/metrics_collector.cc +++ b/services/resource_coordinator/observers/metrics_collector.cc
@@ -5,10 +5,10 @@ #include "services/resource_coordinator/observers/metrics_collector.h" #include "base/metrics/histogram_macros.h" -#include "services/resource_coordinator/coordination_unit/coordination_unit_base.h" #include "services/resource_coordinator/coordination_unit/coordination_unit_manager.h" #include "services/resource_coordinator/coordination_unit/frame_coordination_unit_impl.h" #include "services/resource_coordinator/coordination_unit/page_coordination_unit_impl.h" +#include "services/resource_coordinator/coordination_unit/process_coordination_unit_impl.h" #include "services/resource_coordinator/public/cpp/coordination_unit_id.h" #include "services/resource_coordinator/public/cpp/resource_coordinator_features.h" @@ -40,13 +40,12 @@ // associated with a |CoordinationUnitType::kPage| coordination unit. size_t GetNumCoresidentTabs(const CoordinationUnitBase* coordination_unit) { DCHECK_EQ(CoordinationUnitType::kPage, coordination_unit->id().type); + auto* page_cu = + PageCoordinationUnitImpl::FromCoordinationUnitBase(coordination_unit); std::set<CoordinationUnitBase*> coresident_tabs; - for (auto* process_coordination_unit : - coordination_unit->GetAssociatedCoordinationUnitsOfType( - CoordinationUnitType::kProcess)) { + for (auto* process_cu : page_cu->GetAssociatedProcessCoordinationUnits()) { for (auto* tab_coordination_unit : - process_coordination_unit->GetAssociatedCoordinationUnitsOfType( - CoordinationUnitType::kPage)) { + process_cu->GetAssociatedPageCoordinationUnits()) { coresident_tabs.insert(tab_coordination_unit); } }
diff --git a/services/resource_coordinator/observers/metrics_collector_unittest.cc b/services/resource_coordinator/observers/metrics_collector_unittest.cc index b4c584d..7520fe7 100644 --- a/services/resource_coordinator/observers/metrics_collector_unittest.cc +++ b/services/resource_coordinator/observers/metrics_collector_unittest.cc
@@ -8,6 +8,7 @@ #include "base/test/simple_test_tick_clock.h" #include "build/build_config.h" #include "services/resource_coordinator/coordination_unit/coordination_unit_test_harness.h" +#include "services/resource_coordinator/coordination_unit/frame_coordination_unit_impl.h" #include "services/resource_coordinator/coordination_unit/page_coordination_unit_impl.h" namespace resource_coordinator { @@ -42,10 +43,11 @@ protected: void AdvanceClock(base::TimeDelta delta) { clock_->Advance(delta); } - TestCoordinationUnitWrapper CreatePageCoordinationUnitWithClock() { - auto page_cu = CreateCoordinationUnit(CoordinationUnitType::kPage); - CoordinationUnitBase::ToPageCoordinationUnit(page_cu.get()) - ->SetClockForTest(std::unique_ptr<base::SimpleTestTickClock>(clock_)); + TestCoordinationUnitWrapper<PageCoordinationUnitImpl> + CreatePageCoordinationUnitWithClock() { + auto page_cu = CreateCoordinationUnit<PageCoordinationUnitImpl>(); + page_cu->SetClockForTest( + std::unique_ptr<base::SimpleTestTickClock>(clock_)); return page_cu; } @@ -58,50 +60,50 @@ TEST_F(MAYBE_MetricsCollectorTest, FromBackgroundedToFirstAudioStartsUMA) { auto page_cu = CreatePageCoordinationUnitWithClock(); - auto frame_cu = CreateCoordinationUnit(CoordinationUnitType::kFrame); + auto frame_cu = CreateCoordinationUnit<FrameCoordinationUnitImpl>(); coordination_unit_manager().OnCoordinationUnitCreated(page_cu.get()); coordination_unit_manager().OnCoordinationUnitCreated(frame_cu.get()); - page_cu->AddChild(frame_cu->id()); + page_cu->AddFrame(frame_cu->id()); - page_cu->SendEvent(mojom::Event::kNavigationCommitted); + page_cu->OnMainFrameNavigationCommitted(); AdvanceClock(kTestMetricsReportDelayTimeout); - page_cu->SetProperty(mojom::PropertyType::kVisible, true); - frame_cu->SetProperty(mojom::PropertyType::kAudible, true); + page_cu->SetVisibility(true); + frame_cu->SetAudibility(true); // The page is not backgrounded, thus no metrics recorded. histogram_tester_.ExpectTotalCount(kTabFromBackgroundedToFirstAudioStartsUMA, 0); - frame_cu->SetProperty(mojom::PropertyType::kAudible, false); + frame_cu->SetAudibility(false); - page_cu->SetProperty(mojom::PropertyType::kVisible, false); - frame_cu->SetProperty(mojom::PropertyType::kAudible, true); + page_cu->SetVisibility(false); + frame_cu->SetAudibility(true); // The page was recently audible, thus no metrics recorded. histogram_tester_.ExpectTotalCount(kTabFromBackgroundedToFirstAudioStartsUMA, 0); - frame_cu->SetProperty(mojom::PropertyType::kAudible, false); + frame_cu->SetAudibility(false); AdvanceClock(kTestMaxAudioSlientTimeout); - page_cu->SetProperty(mojom::PropertyType::kVisible, true); - frame_cu->SetProperty(mojom::PropertyType::kAudible, true); + page_cu->SetVisibility(true); + frame_cu->SetAudibility(true); // The page was not recently audible but it is not backgrounded, thus no // metrics recorded. histogram_tester_.ExpectTotalCount(kTabFromBackgroundedToFirstAudioStartsUMA, 0); - frame_cu->SetProperty(mojom::PropertyType::kAudible, false); + frame_cu->SetAudibility(false); - page_cu->SetProperty(mojom::PropertyType::kVisible, false); + page_cu->SetVisibility(false); AdvanceClock(kTestMaxAudioSlientTimeout); - frame_cu->SetProperty(mojom::PropertyType::kAudible, true); + frame_cu->SetAudibility(true); // The page was not recently audible and it is backgrounded, thus metrics // recorded. histogram_tester_.ExpectTotalCount(kTabFromBackgroundedToFirstAudioStartsUMA, 1); - frame_cu->SetProperty(mojom::PropertyType::kAudible, false); + frame_cu->SetAudibility(false); - page_cu->SetProperty(mojom::PropertyType::kVisible, true); - page_cu->SetProperty(mojom::PropertyType::kVisible, false); + page_cu->SetVisibility(true); + page_cu->SetVisibility(false); AdvanceClock(kTestMaxAudioSlientTimeout); - frame_cu->SetProperty(mojom::PropertyType::kAudible, true); + frame_cu->SetAudibility(true); // The page becomes visible and then invisible again, thus metrics recorded. histogram_tester_.ExpectTotalCount(kTabFromBackgroundedToFirstAudioStartsUMA, 2); @@ -110,22 +112,22 @@ TEST_F(MAYBE_MetricsCollectorTest, FromBackgroundedToFirstAudioStartsUMA5MinutesTimeout) { auto page_cu = CreatePageCoordinationUnitWithClock(); - auto frame_cu = CreateCoordinationUnit(CoordinationUnitType::kFrame); + auto frame_cu = CreateCoordinationUnit<FrameCoordinationUnitImpl>(); coordination_unit_manager().OnCoordinationUnitCreated(page_cu.get()); coordination_unit_manager().OnCoordinationUnitCreated(frame_cu.get()); - page_cu->AddChild(frame_cu->id()); + page_cu->AddFrame(frame_cu->id()); - page_cu->SetProperty(mojom::PropertyType::kVisible, false); - page_cu->SendEvent(mojom::Event::kNavigationCommitted); - frame_cu->SetProperty(mojom::PropertyType::kAudible, true); + page_cu->SetVisibility(false); + page_cu->OnMainFrameNavigationCommitted(); + frame_cu->SetAudibility(true); // The page is within 5 minutes after main frame navigation was committed, // thus no metrics recorded. histogram_tester_.ExpectTotalCount(kTabFromBackgroundedToFirstAudioStartsUMA, 0); - frame_cu->SetProperty(mojom::PropertyType::kAudible, false); + frame_cu->SetAudibility(false); AdvanceClock(kTestMetricsReportDelayTimeout); - frame_cu->SetProperty(mojom::PropertyType::kAudible, true); + frame_cu->SetAudibility(true); histogram_tester_.ExpectTotalCount(kTabFromBackgroundedToFirstAudioStartsUMA, 1); } @@ -134,29 +136,29 @@ auto page_cu = CreatePageCoordinationUnitWithClock(); coordination_unit_manager().OnCoordinationUnitCreated(page_cu.get()); - page_cu->SendEvent(mojom::Event::kNavigationCommitted); + page_cu->OnMainFrameNavigationCommitted(); AdvanceClock(kTestMetricsReportDelayTimeout); - page_cu->SetProperty(mojom::PropertyType::kVisible, true); - page_cu->SendEvent(mojom::Event::kTitleUpdated); + page_cu->SetVisibility(true); + page_cu->OnTitleUpdated(); // The page is not backgrounded, thus no metrics recorded. histogram_tester_.ExpectTotalCount(kTabFromBackgroundedToFirstTitleUpdatedUMA, 0); - page_cu->SetProperty(mojom::PropertyType::kVisible, false); - page_cu->SendEvent(mojom::Event::kTitleUpdated); + page_cu->SetVisibility(false); + page_cu->OnTitleUpdated(); // The page is backgrounded, thus metrics recorded. histogram_tester_.ExpectTotalCount(kTabFromBackgroundedToFirstTitleUpdatedUMA, 1); - page_cu->SendEvent(mojom::Event::kTitleUpdated); + page_cu->OnTitleUpdated(); // Metrics should only be recorded once per background period, thus metrics // not recorded. histogram_tester_.ExpectTotalCount(kTabFromBackgroundedToFirstTitleUpdatedUMA, 1); - page_cu->SetProperty(mojom::PropertyType::kVisible, true); - page_cu->SetProperty(mojom::PropertyType::kVisible, false); - page_cu->SendEvent(mojom::Event::kTitleUpdated); + page_cu->SetVisibility(true); + page_cu->SetVisibility(false); + page_cu->OnTitleUpdated(); // The page is backgrounded from foregrounded, thus metrics recorded. histogram_tester_.ExpectTotalCount(kTabFromBackgroundedToFirstTitleUpdatedUMA, 2); @@ -167,49 +169,49 @@ auto page_cu = CreatePageCoordinationUnitWithClock(); coordination_unit_manager().OnCoordinationUnitCreated(page_cu.get()); - page_cu->SendEvent(mojom::Event::kNavigationCommitted); - page_cu->SetProperty(mojom::PropertyType::kVisible, false); - page_cu->SendEvent(mojom::Event::kTitleUpdated); + page_cu->OnMainFrameNavigationCommitted(); + page_cu->SetVisibility(false); + page_cu->OnTitleUpdated(); // The page is within 5 minutes after main frame navigation was committed, // thus no metrics recorded. histogram_tester_.ExpectTotalCount(kTabFromBackgroundedToFirstTitleUpdatedUMA, 0); AdvanceClock(kTestMetricsReportDelayTimeout); - page_cu->SendEvent(mojom::Event::kTitleUpdated); + page_cu->OnTitleUpdated(); histogram_tester_.ExpectTotalCount(kTabFromBackgroundedToFirstTitleUpdatedUMA, 1); } TEST_F(MAYBE_MetricsCollectorTest, FromBackgroundedToFirstAlertFiredUMA) { auto page_cu = CreatePageCoordinationUnitWithClock(); - auto frame_cu = CreateCoordinationUnit(CoordinationUnitType::kFrame); + auto frame_cu = CreateCoordinationUnit<FrameCoordinationUnitImpl>(); coordination_unit_manager().OnCoordinationUnitCreated(page_cu.get()); coordination_unit_manager().OnCoordinationUnitCreated(frame_cu.get()); - page_cu->AddChild(frame_cu->id()); + page_cu->AddFrame(frame_cu->id()); - page_cu->SendEvent(mojom::Event::kNavigationCommitted); + page_cu->OnMainFrameNavigationCommitted(); AdvanceClock(kTestMetricsReportDelayTimeout); - page_cu->SetProperty(mojom::PropertyType::kVisible, true); - frame_cu->SendEvent(mojom::Event::kAlertFired); + page_cu->SetVisibility(true); + frame_cu->OnAlertFired(); // The page is not backgrounded, thus no metrics recorded. histogram_tester_.ExpectTotalCount(kTabFromBackgroundedToFirstAlertFiredUMA, 0); - page_cu->SetProperty(mojom::PropertyType::kVisible, false); - frame_cu->SendEvent(mojom::Event::kAlertFired); + page_cu->SetVisibility(false); + frame_cu->OnAlertFired(); // The page is backgrounded, thus metrics recorded. histogram_tester_.ExpectTotalCount(kTabFromBackgroundedToFirstAlertFiredUMA, 1); - frame_cu->SendEvent(mojom::Event::kAlertFired); + frame_cu->OnAlertFired(); // Metrics should only be recorded once per background period, thus metrics // not recorded. histogram_tester_.ExpectTotalCount(kTabFromBackgroundedToFirstAlertFiredUMA, 1); - page_cu->SetProperty(mojom::PropertyType::kVisible, true); - page_cu->SetProperty(mojom::PropertyType::kVisible, false); - frame_cu->SendEvent(mojom::Event::kAlertFired); + page_cu->SetVisibility(true); + page_cu->SetVisibility(false); + frame_cu->OnAlertFired(); // The page is backgrounded from foregrounded, thus metrics recorded. histogram_tester_.ExpectTotalCount(kTabFromBackgroundedToFirstAlertFiredUMA, 2); @@ -218,20 +220,20 @@ TEST_F(MAYBE_MetricsCollectorTest, FromBackgroundedToFirstAlertFiredUMA5MinutesTimeout) { auto page_cu = CreatePageCoordinationUnitWithClock(); - auto frame_cu = CreateCoordinationUnit(CoordinationUnitType::kFrame); + auto frame_cu = CreateCoordinationUnit<FrameCoordinationUnitImpl>(); coordination_unit_manager().OnCoordinationUnitCreated(page_cu.get()); coordination_unit_manager().OnCoordinationUnitCreated(frame_cu.get()); - page_cu->AddChild(frame_cu->id()); + page_cu->AddFrame(frame_cu->id()); - page_cu->SendEvent(mojom::Event::kNavigationCommitted); - page_cu->SetProperty(mojom::PropertyType::kVisible, false); - frame_cu->SendEvent(mojom::Event::kAlertFired); + page_cu->OnMainFrameNavigationCommitted(); + page_cu->SetVisibility(false); + frame_cu->OnAlertFired(); // The page is within 5 minutes after main frame navigation was committed, // thus no metrics recorded. histogram_tester_.ExpectTotalCount(kTabFromBackgroundedToFirstAlertFiredUMA, 0); AdvanceClock(kTestMetricsReportDelayTimeout); - frame_cu->SendEvent(mojom::Event::kAlertFired); + frame_cu->OnAlertFired(); histogram_tester_.ExpectTotalCount(kTabFromBackgroundedToFirstAlertFiredUMA, 1); } @@ -239,34 +241,34 @@ TEST_F(MAYBE_MetricsCollectorTest, FromBackgroundedToFirstNonPersistentNotificationCreatedUMA) { auto page_cu = CreatePageCoordinationUnitWithClock(); - auto frame_cu = CreateCoordinationUnit(CoordinationUnitType::kFrame); + auto frame_cu = CreateCoordinationUnit<FrameCoordinationUnitImpl>(); coordination_unit_manager().OnCoordinationUnitCreated(page_cu.get()); coordination_unit_manager().OnCoordinationUnitCreated(frame_cu.get()); - page_cu->AddChild(frame_cu->id()); + page_cu->AddFrame(frame_cu->id()); - page_cu->SendEvent(mojom::Event::kNavigationCommitted); + page_cu->OnMainFrameNavigationCommitted(); AdvanceClock(kTestMetricsReportDelayTimeout); - page_cu->SetProperty(mojom::PropertyType::kVisible, true); - frame_cu->SendEvent(mojom::Event::kNonPersistentNotificationCreated); + page_cu->SetVisibility(true); + frame_cu->OnNonPersistentNotificationCreated(); // The page is not backgrounded, thus no metrics recorded. histogram_tester_.ExpectTotalCount( kTabFromBackgroundedToFirstNonPersistentNotificationCreatedUMA, 0); - page_cu->SetProperty(mojom::PropertyType::kVisible, false); - frame_cu->SendEvent(mojom::Event::kNonPersistentNotificationCreated); + page_cu->SetVisibility(false); + frame_cu->OnNonPersistentNotificationCreated(); // The page is backgrounded, thus metrics recorded. histogram_tester_.ExpectTotalCount( kTabFromBackgroundedToFirstNonPersistentNotificationCreatedUMA, 1); - frame_cu->SendEvent(mojom::Event::kNonPersistentNotificationCreated); + frame_cu->OnNonPersistentNotificationCreated(); // Metrics should only be recorded once per background period, thus metrics // not recorded. histogram_tester_.ExpectTotalCount( kTabFromBackgroundedToFirstNonPersistentNotificationCreatedUMA, 1); - page_cu->SetProperty(mojom::PropertyType::kVisible, true); - page_cu->SetProperty(mojom::PropertyType::kVisible, false); - frame_cu->SendEvent(mojom::Event::kNonPersistentNotificationCreated); + page_cu->SetVisibility(true); + page_cu->SetVisibility(false); + frame_cu->OnNonPersistentNotificationCreated(); // The page is backgrounded from foregrounded, thus metrics recorded. histogram_tester_.ExpectTotalCount( kTabFromBackgroundedToFirstNonPersistentNotificationCreatedUMA, 2); @@ -276,20 +278,20 @@ MAYBE_MetricsCollectorTest, FromBackgroundedToFirstNonPersistentNotificationCreatedUMA5MinutesTimeout) { auto page_cu = CreatePageCoordinationUnitWithClock(); - auto frame_cu = CreateCoordinationUnit(CoordinationUnitType::kFrame); + auto frame_cu = CreateCoordinationUnit<FrameCoordinationUnitImpl>(); coordination_unit_manager().OnCoordinationUnitCreated(page_cu.get()); coordination_unit_manager().OnCoordinationUnitCreated(frame_cu.get()); - page_cu->AddChild(frame_cu->id()); + page_cu->AddFrame(frame_cu->id()); - page_cu->SendEvent(mojom::Event::kNavigationCommitted); - page_cu->SetProperty(mojom::PropertyType::kVisible, false); - frame_cu->SendEvent(mojom::Event::kNonPersistentNotificationCreated); + page_cu->OnMainFrameNavigationCommitted(); + page_cu->SetVisibility(false); + frame_cu->OnNonPersistentNotificationCreated(); // The page is within 5 minutes after main frame navigation was committed, // thus no metrics recorded. histogram_tester_.ExpectTotalCount( kTabFromBackgroundedToFirstNonPersistentNotificationCreatedUMA, 0); AdvanceClock(kTestMetricsReportDelayTimeout); - frame_cu->SendEvent(mojom::Event::kNonPersistentNotificationCreated); + frame_cu->OnNonPersistentNotificationCreated(); histogram_tester_.ExpectTotalCount( kTabFromBackgroundedToFirstNonPersistentNotificationCreatedUMA, 1); } @@ -298,29 +300,29 @@ auto page_cu = CreatePageCoordinationUnitWithClock(); coordination_unit_manager().OnCoordinationUnitCreated(page_cu.get()); - page_cu->SendEvent(mojom::Event::kNavigationCommitted); + page_cu->OnMainFrameNavigationCommitted(); AdvanceClock(kTestMetricsReportDelayTimeout); - page_cu->SetProperty(mojom::PropertyType::kVisible, true); - page_cu->SendEvent(mojom::Event::kFaviconUpdated); + page_cu->SetVisibility(true); + page_cu->OnFaviconUpdated(); // The page is not backgrounded, thus no metrics recorded. histogram_tester_.ExpectTotalCount( kTabFromBackgroundedToFirstFaviconUpdatedUMA, 0); - page_cu->SetProperty(mojom::PropertyType::kVisible, false); - page_cu->SendEvent(mojom::Event::kFaviconUpdated); + page_cu->SetVisibility(false); + page_cu->OnFaviconUpdated(); // The page is backgrounded, thus metrics recorded. histogram_tester_.ExpectTotalCount( kTabFromBackgroundedToFirstFaviconUpdatedUMA, 1); - page_cu->SendEvent(mojom::Event::kFaviconUpdated); + page_cu->OnFaviconUpdated(); // Metrics should only be recorded once per background period, thus metrics // not recorded. histogram_tester_.ExpectTotalCount( kTabFromBackgroundedToFirstFaviconUpdatedUMA, 1); - page_cu->SetProperty(mojom::PropertyType::kVisible, true); - page_cu->SetProperty(mojom::PropertyType::kVisible, false); - page_cu->SendEvent(mojom::Event::kFaviconUpdated); + page_cu->SetVisibility(true); + page_cu->SetVisibility(false); + page_cu->OnFaviconUpdated(); // The page is backgrounded from foregrounded, thus metrics recorded. histogram_tester_.ExpectTotalCount( kTabFromBackgroundedToFirstFaviconUpdatedUMA, 2); @@ -331,15 +333,15 @@ auto page_cu = CreatePageCoordinationUnitWithClock(); coordination_unit_manager().OnCoordinationUnitCreated(page_cu.get()); - page_cu->SendEvent(mojom::Event::kNavigationCommitted); - page_cu->SetProperty(mojom::PropertyType::kVisible, false); - page_cu->SendEvent(mojom::Event::kFaviconUpdated); + page_cu->OnMainFrameNavigationCommitted(); + page_cu->SetVisibility(false); + page_cu->OnFaviconUpdated(); // The page is within 5 minutes after main frame navigation was committed, // thus no metrics recorded. histogram_tester_.ExpectTotalCount( kTabFromBackgroundedToFirstFaviconUpdatedUMA, 0); AdvanceClock(kTestMetricsReportDelayTimeout); - page_cu->SendEvent(mojom::Event::kFaviconUpdated); + page_cu->OnFaviconUpdated(); histogram_tester_.ExpectTotalCount( kTabFromBackgroundedToFirstFaviconUpdatedUMA, 1); }
diff --git a/services/resource_coordinator/observers/tab_signal_generator_impl.cc b/services/resource_coordinator/observers/tab_signal_generator_impl.cc index 7b6b4027..3c0f8bca 100644 --- a/services/resource_coordinator/observers/tab_signal_generator_impl.cc +++ b/services/resource_coordinator/observers/tab_signal_generator_impl.cc
@@ -7,7 +7,6 @@ #include <utility> #include "base/values.h" -#include "services/resource_coordinator/coordination_unit/coordination_unit_base.h" #include "services/resource_coordinator/coordination_unit/frame_coordination_unit_impl.h" #include "services/resource_coordinator/coordination_unit/page_coordination_unit_impl.h" #include "services/service_manager/public/cpp/bind_source_info.h" @@ -43,12 +42,9 @@ if (!frame_cu->IsMainFrame()) return; // TODO(lpy) Combine CPU usage or long task idleness signal. - for (auto* parent : frame_cu->parents()) { - if (parent->id().type != CoordinationUnitType::kPage) - continue; - DISPATCH_TAB_SIGNAL(observers_, OnEventReceived, parent, + if (auto* page_cu = frame_cu->GetPageCoordinationUnit()) { + DISPATCH_TAB_SIGNAL(observers_, OnEventReceived, page_cu, mojom::TabEvent::kDoneLoading); - break; } } }
diff --git a/services/resource_coordinator/observers/tab_signal_generator_impl_unittest.cc b/services/resource_coordinator/observers/tab_signal_generator_impl_unittest.cc index 6a47a223..344b4e9e 100644 --- a/services/resource_coordinator/observers/tab_signal_generator_impl_unittest.cc +++ b/services/resource_coordinator/observers/tab_signal_generator_impl_unittest.cc
@@ -4,11 +4,10 @@ #include "services/resource_coordinator/observers/tab_signal_generator_impl.h" -#include "services/resource_coordinator/coordination_unit/coordination_unit_base.h" #include "services/resource_coordinator/coordination_unit/coordination_unit_test_harness.h" #include "services/resource_coordinator/coordination_unit/mock_coordination_unit_graphs.h" -#include "services/resource_coordinator/public/cpp/coordination_unit_types.h" -#include "services/resource_coordinator/public/interfaces/coordination_unit.mojom.h" +#include "services/resource_coordinator/coordination_unit/page_coordination_unit_impl.h" +#include "services/resource_coordinator/coordination_unit/process_coordination_unit_impl.h" #include "testing/gtest/include/gtest/gtest.h" namespace resource_coordinator { @@ -44,10 +43,10 @@ MockSinglePageWithMultipleProcessesCoordinationUnitGraph cu_graph; cu_graph.page->AddObserver(tab_signal_generator()); - cu_graph.process->SetProperty( - mojom::PropertyType::kExpectedTaskQueueingDuration, 1); - cu_graph.other_process->SetProperty( - mojom::PropertyType::kExpectedTaskQueueingDuration, 10); + cu_graph.process->SetExpectedTaskQueueingDuration( + base::TimeDelta::FromMilliseconds(1)); + cu_graph.other_process->SetExpectedTaskQueueingDuration( + base::TimeDelta::FromMilliseconds(10)); // The |other_process| is not for the main frame so its EQT values does not // propagate to the page.
diff --git a/services/resource_coordinator/public/cpp/BUILD.gn b/services/resource_coordinator/public/cpp/BUILD.gn index 4e76e805..91183046 100644 --- a/services/resource_coordinator/public/cpp/BUILD.gn +++ b/services/resource_coordinator/public/cpp/BUILD.gn
@@ -7,6 +7,8 @@ "coordination_unit_id.cc", "coordination_unit_id.h", "coordination_unit_types.h", + "frame_resource_coordinator.cc", + "frame_resource_coordinator.h", "memory_instrumentation/client_process_impl.cc", "memory_instrumentation/client_process_impl.h", "memory_instrumentation/coordinator.h", @@ -19,9 +21,12 @@ "memory_instrumentation/os_metrics_win.cc", "memory_instrumentation/tracing_observer.cc", "memory_instrumentation/tracing_observer.h", + "page_resource_coordinator.cc", + "page_resource_coordinator.h", + "process_resource_coordinator.cc", + "process_resource_coordinator.h", "resource_coordinator_features.cc", "resource_coordinator_features.h", - "resource_coordinator_interface.cc", "resource_coordinator_interface.h", "tracing/chrome_trace_event_agent.cc", "tracing/chrome_trace_event_agent.h",
diff --git a/services/resource_coordinator/public/cpp/coordination_unit_id.h b/services/resource_coordinator/public/cpp/coordination_unit_id.h index 44d815b..f63ac7d 100644 --- a/services/resource_coordinator/public/cpp/coordination_unit_id.h +++ b/services/resource_coordinator/public/cpp/coordination_unit_id.h
@@ -30,6 +30,8 @@ return id == b.id && type == b.type; } + bool operator!=(const CoordinationUnitID& b) const { return !(*this == b); } + bool operator<(const CoordinationUnitID& b) const { return std::tie(id, type) < std::tie(b.id, b.type); }
diff --git a/services/resource_coordinator/public/cpp/frame_resource_coordinator.cc b/services/resource_coordinator/public/cpp/frame_resource_coordinator.cc new file mode 100644 index 0000000..c270a26 --- /dev/null +++ b/services/resource_coordinator/public/cpp/frame_resource_coordinator.cc
@@ -0,0 +1,70 @@ +// 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 "services/resource_coordinator/public/cpp/frame_resource_coordinator.h" + +namespace resource_coordinator { + +FrameResourceCoordinator::FrameResourceCoordinator( + service_manager::Connector* connector) + : ResourceCoordinatorInterface(), weak_ptr_factory_(this) { + CoordinationUnitID new_cu_id(CoordinationUnitType::kFrame, std::string()); + ResourceCoordinatorInterface::ConnectToService(connector, new_cu_id); +} + +FrameResourceCoordinator::~FrameResourceCoordinator() = default; + +void FrameResourceCoordinator::SetAudibility(bool audible) { + if (!service_) + return; + service_->SetAudibility(audible); +} + +void FrameResourceCoordinator::OnAlertFired() { + if (!service_) + return; + service_->OnAlertFired(); +} + +void FrameResourceCoordinator::AddChildFrame( + const FrameResourceCoordinator& child) { + DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); + if (!service_) + return; + // We could keep the ID around ourselves, but this hop ensures that the child + // has been created on the service-side. + child.service()->GetID( + base::Bind(&FrameResourceCoordinator::AddChildFrameByID, + weak_ptr_factory_.GetWeakPtr())); +} + +void FrameResourceCoordinator::RemoveChildFrame( + const FrameResourceCoordinator& child) { + DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); + if (!service_) + return; + child.service()->GetID( + base::Bind(&FrameResourceCoordinator::RemoveChildFrameByID, + weak_ptr_factory_.GetWeakPtr())); +} + +void FrameResourceCoordinator::ConnectToService( + mojom::CoordinationUnitProviderPtr& provider, + const CoordinationUnitID& cu_id) { + provider->CreateFrameCoordinationUnit(mojo::MakeRequest(&service_), cu_id); +} + +void FrameResourceCoordinator::AddChildFrameByID( + const CoordinationUnitID& child_id) { + DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); + service_->AddChildFrame(child_id); +} + +void FrameResourceCoordinator::RemoveChildFrameByID( + const CoordinationUnitID& child_id) { + DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); + service_->RemoveChildFrame(child_id); +} + +} // namespace resource_coordinator
diff --git a/services/resource_coordinator/public/cpp/frame_resource_coordinator.h b/services/resource_coordinator/public/cpp/frame_resource_coordinator.h new file mode 100644 index 0000000..0f5875f --- /dev/null +++ b/services/resource_coordinator/public/cpp/frame_resource_coordinator.h
@@ -0,0 +1,45 @@ +// 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 SERVICES_RESOURCE_COORDINATOR_PUBLIC_CPP_FRAME_RESOURCE_COORDINATOR_H_ +#define SERVICES_RESOURCE_COORDINATOR_PUBLIC_CPP_FRAME_RESOURCE_COORDINATOR_H_ + +#include "base/memory/weak_ptr.h" +#include "base/threading/thread_checker.h" +#include "services/resource_coordinator/public/cpp/resource_coordinator_interface.h" +#include "services/resource_coordinator/public/interfaces/coordination_unit.mojom.h" + +namespace resource_coordinator { + +class SERVICES_RESOURCE_COORDINATOR_PUBLIC_CPP_EXPORT FrameResourceCoordinator + : public ResourceCoordinatorInterface<mojom::FrameCoordinationUnitPtr, + mojom::FrameCoordinationUnitRequest> { + public: + FrameResourceCoordinator(service_manager::Connector* connector); + ~FrameResourceCoordinator() override; + + void SetAudibility(bool audible); + void OnAlertFired(); + void AddChildFrame(const FrameResourceCoordinator& child); + void RemoveChildFrame(const FrameResourceCoordinator& child); + + private: + void ConnectToService(mojom::CoordinationUnitProviderPtr& provider, + const CoordinationUnitID& cu_id) override; + + void AddChildFrameByID(const CoordinationUnitID& child_id); + void RemoveChildFrameByID(const CoordinationUnitID& child_id); + + THREAD_CHECKER(thread_checker_); + + // The WeakPtrFactory should come last so the weak ptrs are invalidated + // before the rest of the member variables. + base::WeakPtrFactory<FrameResourceCoordinator> weak_ptr_factory_; + + DISALLOW_COPY_AND_ASSIGN(FrameResourceCoordinator); +}; + +} // namespace resource_coordinator + +#endif // SERVICES_RESOURCE_COORDINATOR_PUBLIC_CPP_FRAME_RESOURCE_COORDINATOR_H_
diff --git a/services/resource_coordinator/public/cpp/page_resource_coordinator.cc b/services/resource_coordinator/public/cpp/page_resource_coordinator.cc new file mode 100644 index 0000000..c2cb58e --- /dev/null +++ b/services/resource_coordinator/public/cpp/page_resource_coordinator.cc
@@ -0,0 +1,73 @@ +// 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 "services/resource_coordinator/public/cpp/page_resource_coordinator.h" + +namespace resource_coordinator { + +PageResourceCoordinator::PageResourceCoordinator( + service_manager::Connector* connector) + : ResourceCoordinatorInterface(), weak_ptr_factory_(this) { + CoordinationUnitID new_cu_id(CoordinationUnitType::kPage, std::string()); + ResourceCoordinatorInterface::ConnectToService(connector, new_cu_id); +} + +PageResourceCoordinator::~PageResourceCoordinator() = default; + +void PageResourceCoordinator::SetVisibility(bool visible) { + service_->SetVisibility(visible); +} + +void PageResourceCoordinator::SetUKMSourceId(int64_t ukm_source_id) { + service_->SetUKMSourceId(ukm_source_id); +} + +void PageResourceCoordinator::OnFaviconUpdated() { + service_->OnFaviconUpdated(); +} + +void PageResourceCoordinator::OnTitleUpdated() { + service_->OnTitleUpdated(); +} + +void PageResourceCoordinator::OnMainFrameNavigationCommitted() { + service_->OnMainFrameNavigationCommitted(); +} + +void PageResourceCoordinator::AddFrame(const FrameResourceCoordinator& frame) { + DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); + if (!service_) + return; + // We could keep the ID around ourselves, but this hop ensures that the child + // has been created on the service-side. + frame.service()->GetID(base::Bind(&PageResourceCoordinator::AddFrameByID, + weak_ptr_factory_.GetWeakPtr())); +} + +void PageResourceCoordinator::RemoveFrame( + const FrameResourceCoordinator& frame) { + DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); + if (!service_) + return; + frame.service()->GetID(base::Bind(&PageResourceCoordinator::RemoveFrameByID, + weak_ptr_factory_.GetWeakPtr())); +} + +void PageResourceCoordinator::ConnectToService( + mojom::CoordinationUnitProviderPtr& provider, + const CoordinationUnitID& cu_id) { + provider->CreatePageCoordinationUnit(mojo::MakeRequest(&service_), cu_id); +} + +void PageResourceCoordinator::AddFrameByID(const CoordinationUnitID& cu_id) { + DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); + service_->AddFrame(cu_id); +} + +void PageResourceCoordinator::RemoveFrameByID(const CoordinationUnitID& cu_id) { + DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); + service_->RemoveFrame(cu_id); +} + +} // namespace resource_coordinator
diff --git a/services/resource_coordinator/public/cpp/page_resource_coordinator.h b/services/resource_coordinator/public/cpp/page_resource_coordinator.h new file mode 100644 index 0000000..bd49552 --- /dev/null +++ b/services/resource_coordinator/public/cpp/page_resource_coordinator.h
@@ -0,0 +1,50 @@ +// 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 SERVICES_RESOURCE_COORDINATOR_PUBLIC_CPP_PAGE_RESOURCE_COORDINATOR_H_ +#define SERVICES_RESOURCE_COORDINATOR_PUBLIC_CPP_PAGE_RESOURCE_COORDINATOR_H_ + +#include "base/memory/weak_ptr.h" +#include "base/threading/thread_checker.h" +#include "services/resource_coordinator/public/cpp/frame_resource_coordinator.h" +#include "services/resource_coordinator/public/cpp/resource_coordinator_interface.h" +#include "services/resource_coordinator/public/interfaces/coordination_unit.mojom.h" + +namespace resource_coordinator { + +class SERVICES_RESOURCE_COORDINATOR_PUBLIC_CPP_EXPORT PageResourceCoordinator + : public ResourceCoordinatorInterface<mojom::PageCoordinationUnitPtr, + mojom::PageCoordinationUnitRequest> { + public: + PageResourceCoordinator(service_manager::Connector* connector); + ~PageResourceCoordinator() override; + + void SetVisibility(bool visible); + void SetUKMSourceId(int64_t ukm_source_id); + void OnFaviconUpdated(); + void OnTitleUpdated(); + void OnMainFrameNavigationCommitted(); + + void AddFrame(const FrameResourceCoordinator& frame); + void RemoveFrame(const FrameResourceCoordinator& frame); + + private: + void ConnectToService(mojom::CoordinationUnitProviderPtr& provider, + const CoordinationUnitID& cu_id) override; + + void AddFrameByID(const CoordinationUnitID& cu_id); + void RemoveFrameByID(const CoordinationUnitID& cu_id); + + THREAD_CHECKER(thread_checker_); + + // The WeakPtrFactory should come last so the weak ptrs are invalidated + // before the rest of the member variables. + base::WeakPtrFactory<PageResourceCoordinator> weak_ptr_factory_; + + DISALLOW_COPY_AND_ASSIGN(PageResourceCoordinator); +}; + +} // namespace resource_coordinator + +#endif // SERVICES_RESOURCE_COORDINATOR_PUBLIC_CPP_PAGE_RESOURCE_COORDINATOR_H_
diff --git a/services/resource_coordinator/public/cpp/process_resource_coordinator.cc b/services/resource_coordinator/public/cpp/process_resource_coordinator.cc new file mode 100644 index 0000000..06d3c0c5 --- /dev/null +++ b/services/resource_coordinator/public/cpp/process_resource_coordinator.cc
@@ -0,0 +1,74 @@ +// 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 "services/resource_coordinator/public/cpp/process_resource_coordinator.h" + +namespace resource_coordinator { + +ProcessResourceCoordinator::ProcessResourceCoordinator( + service_manager::Connector* connector) + : ResourceCoordinatorInterface(), weak_ptr_factory_(this) { + CoordinationUnitID new_cu_id(CoordinationUnitType::kProcess, std::string()); + ResourceCoordinatorInterface::ConnectToService(connector, new_cu_id); +} + +ProcessResourceCoordinator::~ProcessResourceCoordinator() = default; + +void ProcessResourceCoordinator::SetCPUUsage(double cpu_usage) { + if (!service_) + return; + service_->SetCPUUsage(cpu_usage); +} + +void ProcessResourceCoordinator::SetLaunchTime(base::Time launch_time) { + if (!service_) + return; + service_->SetLaunchTime(launch_time); +} + +void ProcessResourceCoordinator::SetPID(int64_t pid) { + if (!service_) + return; + service_->SetPID(pid); +} + +void ProcessResourceCoordinator::AddFrame( + const FrameResourceCoordinator& frame) { + DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); + if (!service_) + return; + // We could keep the ID around ourselves, but this hop ensures that the child + // has been created on the service-side. + frame.service()->GetID(base::Bind(&ProcessResourceCoordinator::AddFrameByID, + weak_ptr_factory_.GetWeakPtr())); +} + +void ProcessResourceCoordinator::RemoveFrame( + const FrameResourceCoordinator& frame) { + DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); + if (!service_) + return; + frame.service()->GetID( + base::Bind(&ProcessResourceCoordinator::RemoveFrameByID, + weak_ptr_factory_.GetWeakPtr())); +} + +void ProcessResourceCoordinator::ConnectToService( + mojom::CoordinationUnitProviderPtr& provider, + const CoordinationUnitID& cu_id) { + provider->CreateProcessCoordinationUnit(mojo::MakeRequest(&service_), cu_id); +} + +void ProcessResourceCoordinator::AddFrameByID(const CoordinationUnitID& cu_id) { + DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); + service_->AddFrame(cu_id); +} + +void ProcessResourceCoordinator::RemoveFrameByID( + const CoordinationUnitID& cu_id) { + DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); + service_->RemoveFrame(cu_id); +} + +} // namespace resource_coordinator
diff --git a/services/resource_coordinator/public/cpp/process_resource_coordinator.h b/services/resource_coordinator/public/cpp/process_resource_coordinator.h new file mode 100644 index 0000000..75c68db --- /dev/null +++ b/services/resource_coordinator/public/cpp/process_resource_coordinator.h
@@ -0,0 +1,49 @@ +// 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 SERVICES_RESOURCE_COORDINATOR_PUBLIC_CPP_PROCESS_RESOURCE_COORDINATOR_H_ +#define SERVICES_RESOURCE_COORDINATOR_PUBLIC_CPP_PROCESS_RESOURCE_COORDINATOR_H_ + +#include "base/memory/weak_ptr.h" +#include "base/threading/thread_checker.h" +#include "services/resource_coordinator/public/cpp/frame_resource_coordinator.h" +#include "services/resource_coordinator/public/cpp/resource_coordinator_interface.h" +#include "services/resource_coordinator/public/interfaces/coordination_unit.mojom.h" + +namespace resource_coordinator { + +class SERVICES_RESOURCE_COORDINATOR_PUBLIC_CPP_EXPORT ProcessResourceCoordinator + : public ResourceCoordinatorInterface< + mojom::ProcessCoordinationUnitPtr, + mojom::ProcessCoordinationUnitRequest> { + public: + ProcessResourceCoordinator(service_manager::Connector* connector); + ~ProcessResourceCoordinator() override; + + void SetCPUUsage(double usage); + void SetLaunchTime(base::Time launch_time); + void SetPID(int64_t pid); + + void AddFrame(const FrameResourceCoordinator& frame); + void RemoveFrame(const FrameResourceCoordinator& frame); + + private: + void ConnectToService(mojom::CoordinationUnitProviderPtr& provider, + const CoordinationUnitID& cu_id) override; + + void AddFrameByID(const CoordinationUnitID& cu_id); + void RemoveFrameByID(const CoordinationUnitID& cu_id); + + THREAD_CHECKER(thread_checker_); + + // The WeakPtrFactory should come last so the weak ptrs are invalidated + // before the rest of the member variables. + base::WeakPtrFactory<ProcessResourceCoordinator> weak_ptr_factory_; + + DISALLOW_COPY_AND_ASSIGN(ProcessResourceCoordinator); +}; + +} // namespace resource_coordinator + +#endif // SERVICES_RESOURCE_COORDINATOR_PUBLIC_CPP_PROCESS_RESOURCE_COORDINATOR_H_
diff --git a/services/resource_coordinator/public/cpp/resource_coordinator_interface.cc b/services/resource_coordinator/public/cpp/resource_coordinator_interface.cc deleted file mode 100644 index 65c7104..0000000 --- a/services/resource_coordinator/public/cpp/resource_coordinator_interface.cc +++ /dev/null
@@ -1,123 +0,0 @@ -// Copyright 2017 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "services/resource_coordinator/public/cpp/resource_coordinator_interface.h" - -#include <utility> - -#include "mojo/public/cpp/bindings/binding.h" -#include "services/resource_coordinator/public/interfaces/coordination_unit_provider.mojom.h" -#include "services/resource_coordinator/public/interfaces/service_constants.mojom.h" -#include "services/service_manager/public/cpp/connector.h" - -namespace { - -void OnConnectionError() { - DCHECK(false); -} - -} // namespace - -namespace resource_coordinator { - -ResourceCoordinatorInterface::ResourceCoordinatorInterface( - service_manager::Connector* connector, - const CoordinationUnitType& type, - const std::string& id) - : weak_ptr_factory_(this) { - CoordinationUnitID new_cu_id(type, id); - ConnectToService(connector, new_cu_id); -} - -ResourceCoordinatorInterface::ResourceCoordinatorInterface( - service_manager::Connector* connector, - const CoordinationUnitType& type) - : ResourceCoordinatorInterface(connector, type, std::string()) {} - -ResourceCoordinatorInterface::ResourceCoordinatorInterface( - service_manager::Connector* connector, - const CoordinationUnitType& type, - uint64_t id) - : weak_ptr_factory_(this) { - CoordinationUnitID new_cu_id(type, id); - ConnectToService(connector, new_cu_id); -} - -ResourceCoordinatorInterface::~ResourceCoordinatorInterface() = default; - -void ResourceCoordinatorInterface::ConnectToService( - service_manager::Connector* connector, - const CoordinationUnitID& cu_id) { - DCHECK(thread_checker_.CalledOnValidThread()); - if (!connector) - return; - cu_id_ = cu_id; - mojom::CoordinationUnitProviderPtr provider; - connector->BindInterface(mojom::kServiceName, mojo::MakeRequest(&provider)); - - provider->CreateCoordinationUnit(mojo::MakeRequest(&service_), cu_id); - - service_.set_connection_error_handler(base::Bind(&OnConnectionError)); -} - -void ResourceCoordinatorInterface::SendEvent(const mojom::Event event) { - DCHECK(thread_checker_.CalledOnValidThread()); - if (!service_) - return; - - service_->SendEvent(event); -} - -void ResourceCoordinatorInterface::SetProperty( - mojom::PropertyType property_type, - int64_t value) { - DCHECK(thread_checker_.CalledOnValidThread()); - if (!service_) - return; - service_->SetProperty(property_type, value); -} - -void ResourceCoordinatorInterface::AddBinding( - mojom::CoordinationUnitRequest request) { - if (!service_) - return; - service_->AddBinding(std::move(request)); -} - -void ResourceCoordinatorInterface::AddChild( - const ResourceCoordinatorInterface& child) { - DCHECK(thread_checker_.CalledOnValidThread()); - if (!service_) - return; - // We could keep the ID around ourselves, but this hop ensures that the child - // has been created on the service-side. - child.service()->GetID(base::Bind(&ResourceCoordinatorInterface::AddChildByID, - weak_ptr_factory_.GetWeakPtr())); -} - -void ResourceCoordinatorInterface::RemoveChild( - const ResourceCoordinatorInterface& child) { - DCHECK(thread_checker_.CalledOnValidThread()); - if (!service_) - return; - // We could keep the ID around ourselves, but this hop ensures that the child - // has been created on the service-side. - child.service()->GetID( - base::Bind(&ResourceCoordinatorInterface::RemoveChildByID, - weak_ptr_factory_.GetWeakPtr())); -} - -void ResourceCoordinatorInterface::AddChildByID( - const CoordinationUnitID& child_id) { - DCHECK(thread_checker_.CalledOnValidThread()); - service_->AddChild(child_id); -} - -void ResourceCoordinatorInterface::RemoveChildByID( - const CoordinationUnitID& child_id) { - DCHECK(thread_checker_.CalledOnValidThread()); - service_->RemoveChild(child_id); -} - -} // namespace resource_coordinator
diff --git a/services/resource_coordinator/public/cpp/resource_coordinator_interface.h b/services/resource_coordinator/public/cpp/resource_coordinator_interface.h index c9d55ee9..3d6e08f 100644 --- a/services/resource_coordinator/public/cpp/resource_coordinator_interface.h +++ b/services/resource_coordinator/public/cpp/resource_coordinator_interface.h
@@ -7,58 +7,48 @@ #include <stdint.h> -#include <string> - #include "base/macros.h" -#include "base/memory/weak_ptr.h" -#include "base/threading/thread_checker.h" -#include "services/resource_coordinator/public/interfaces/coordination_unit.mojom.h" - -namespace service_manager { -class Connector; -} +#include "services/resource_coordinator/public/cpp/coordination_unit_id.h" +#include "services/resource_coordinator/public/cpp/resource_coordinator_export.h" +#include "services/resource_coordinator/public/interfaces/coordination_unit_provider.mojom.h" +#include "services/resource_coordinator/public/interfaces/service_constants.mojom.h" +#include "services/service_manager/public/cpp/connector.h" namespace resource_coordinator { -class SERVICES_RESOURCE_COORDINATOR_PUBLIC_CPP_EXPORT - ResourceCoordinatorInterface { +template <class CoordinationUnitMojoPtr, class CoordinationUnitMojoRequest> +class ResourceCoordinatorInterface { public: - ResourceCoordinatorInterface(service_manager::Connector* connector, - const CoordinationUnitType& type); - ResourceCoordinatorInterface(service_manager::Connector* connector, - const CoordinationUnitType& type, - const std::string& id); - ResourceCoordinatorInterface(service_manager::Connector* connector, - const CoordinationUnitType& type, - uint64_t id); + ResourceCoordinatorInterface() = default; + virtual ~ResourceCoordinatorInterface() = default; - ~ResourceCoordinatorInterface(); - - void SendEvent(const mojom::Event event); - void SetProperty(mojom::PropertyType property_type, int64_t value); - void AddBinding(mojom::CoordinationUnitRequest request); - void AddChild(const ResourceCoordinatorInterface& child); - void RemoveChild(const ResourceCoordinatorInterface& child); + void AddBinding(CoordinationUnitMojoRequest request) { + if (!service_) + return; + service_->AddBinding(std::move(request)); + } CoordinationUnitID id() const { return cu_id_; } + const CoordinationUnitMojoPtr& service() const { return service_; } - private: + protected: + virtual void ConnectToService(mojom::CoordinationUnitProviderPtr& provider, + const CoordinationUnitID& cu_id) = 0; + void ConnectToService(service_manager::Connector* connector, - const CoordinationUnitID& cu_id); - void AddChildByID(const CoordinationUnitID& child_id); - void RemoveChildByID(const CoordinationUnitID& child_id); + const CoordinationUnitID& cu_id) { + if (!connector) + return; + cu_id_ = cu_id; + mojom::CoordinationUnitProviderPtr provider; + connector->BindInterface(mojom::kServiceName, mojo::MakeRequest(&provider)); + ConnectToService(provider, cu_id); + } - const mojom::CoordinationUnitPtr& service() const { return service_; } - - mojom::CoordinationUnitPtr service_; + CoordinationUnitMojoPtr service_; CoordinationUnitID cu_id_; - base::ThreadChecker thread_checker_; - - // The WeakPtrFactory should come last so the weak ptrs are invalidated - // before the rest of the member variables. - base::WeakPtrFactory<ResourceCoordinatorInterface> weak_ptr_factory_; - + private: DISALLOW_COPY_AND_ASSIGN(ResourceCoordinatorInterface); };
diff --git a/services/resource_coordinator/public/interfaces/coordination_unit.mojom b/services/resource_coordinator/public/interfaces/coordination_unit.mojom index 905779b..b48ffb91 100644 --- a/services/resource_coordinator/public/interfaces/coordination_unit.mojom +++ b/services/resource_coordinator/public/interfaces/coordination_unit.mojom
@@ -21,38 +21,23 @@ int64 id; }; -// TODO(lpy) Remove this interface once the per-type interface is fully in use. -interface CoordinationUnit { - // Mainly used to force a round-trip to the service over the pipe for - // a specific unit, so we don't have to deal with possibly-not-yet-created - // children in AddChild() - GetID() => (CoordinationUnitID id); - - // Add a new binding to an existing CoordinationUnit. - AddBinding(CoordinationUnit& request); - - // child_id must represent a CU that already exists service-side, - // and can't be a direct ascendent or descendent of the current unit. - AddChild(CoordinationUnitID child_id); - - // child_id must represent a CU that already exists service-side - // and is a direct descendent of the current unit. - RemoveChild(CoordinationUnitID child_id); - - // Sends event signal to CoordinationUnit, an event signal are non-persistent. - SendEvent(Event event); - - // Sets a property on the CoordinationUnit's internal key-value store. - SetProperty(PropertyType property_type, int64 value); -}; - // A FrameCoordinationUnit has at most one ProcessCoordinationUnit as its // parent, at most one PageCoordinationUnit as its parent, at most one // FrameCoordinationUnit as parent frame, and can have many child frames. interface FrameCoordinationUnit { + // Mainly used to force a round-trip to the service over the pipe for + // a specific unit, so we don't have to deal with possibly-not-yet-created + // children. + GetID() => (CoordinationUnitID id); + + // Add a new binding to an existing FrameCoordinationUnit. + AddBinding(FrameCoordinationUnit& request); + AddChildFrame(CoordinationUnitID cu_id); + RemoveChildFrame(CoordinationUnitID cu_id); + // Property signals. SetAudibility(bool audible); - SetNetworkAlmostIdleness(bool idle); + SetNetworkAlmostIdle(bool idle); // Event signals. OnAlertFired(); @@ -60,6 +45,16 @@ }; interface PageCoordinationUnit { + // Mainly used to force a round-trip to the service over the pipe for + // a specific unit, so we don't have to deal with possibly-not-yet-created + // children. + GetID() => (CoordinationUnitID id); + + // Add a new binding to an existing PageCoordinationUnit. + AddBinding(PageCoordinationUnit& request); + AddFrame(CoordinationUnitID cu_id); + RemoveFrame(CoordinationUnitID cu_id); + // Property signals. SetVisibility(bool visible); SetUKMSourceId(int64 ukm_source_id); @@ -71,9 +66,19 @@ }; interface ProcessCoordinationUnit { + // Mainly used to force a round-trip to the service over the pipe for + // a specific unit, so we don't have to deal with possibly-not-yet-created + // children. + GetID() => (CoordinationUnitID id); + + // Add a new binding to an existing ProcessCoordinationUnit. + AddBinding(ProcessCoordinationUnit& request); + AddFrame(CoordinationUnitID cu_id); + RemoveFrame(CoordinationUnitID cu_id); + // Property signals. SetCPUUsage(double cpu_usage); SetExpectedTaskQueueingDuration(mojo.common.mojom.TimeDelta duration); SetLaunchTime(mojo.common.mojom.Time launch_time); SetPID(int64 pid); -}; \ No newline at end of file +};
diff --git a/services/resource_coordinator/public/interfaces/coordination_unit_provider.mojom b/services/resource_coordinator/public/interfaces/coordination_unit_provider.mojom index 59c4451..d96fc30 100644 --- a/services/resource_coordinator/public/interfaces/coordination_unit_provider.mojom +++ b/services/resource_coordinator/public/interfaces/coordination_unit_provider.mojom
@@ -7,5 +7,7 @@ import "coordination_unit.mojom"; interface CoordinationUnitProvider { - CreateCoordinationUnit(CoordinationUnit& request, CoordinationUnitID id); + CreateFrameCoordinationUnit(FrameCoordinationUnit& request, CoordinationUnitID id); + CreatePageCoordinationUnit(PageCoordinationUnit& request, CoordinationUnitID id); + CreateProcessCoordinationUnit(ProcessCoordinationUnit& request, CoordinationUnitID id); };
diff --git a/services/resource_coordinator/resource_coordinator_service_unittest.cc b/services/resource_coordinator/resource_coordinator_service_unittest.cc index e9ff43b..20147aaa 100644 --- a/services/resource_coordinator/resource_coordinator_service_unittest.cc +++ b/services/resource_coordinator/resource_coordinator_service_unittest.cc
@@ -43,9 +43,9 @@ connector()->BindInterface(mojom::kServiceName, mojo::MakeRequest(&provider)); CoordinationUnitID new_id(CoordinationUnitType::kPage, "test_id"); - mojom::CoordinationUnitPtr coordination_unit; - provider->CreateCoordinationUnit(mojo::MakeRequest(&coordination_unit), - new_id); + mojom::PageCoordinationUnitPtr coordination_unit; + provider->CreatePageCoordinationUnit(mojo::MakeRequest(&coordination_unit), + new_id); coordination_unit->GetID(base::Bind(&ResourceCoordinatorTest::GetIDCallback, base::Unretained(this)));
diff --git a/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter b/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter index a44dea6..62d1327 100644 --- a/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter +++ b/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter
@@ -116,18 +116,21 @@ -ResourcePrefetchPredictorPrefetchingBrowserTest.Simple -SSLUIWorkerFetchTest.TestUnsafeContentsInWorkerWithUserException/2 -SSLUIWorkerFetchTest.TestUnsafeContentsInWorkerWithUserException/3 +-SSLUITestCommittedInterstitials.ErrorPageType -SafeBrowsingBlockingPageBrowserTestWithThreatTypeAndIsolationSetting/SafeBrowsingBlockingPageBrowserTest.SecurityStateGoBackOnSubresourceInterstitial/0 -SafeBrowsingBlockingPageBrowserTestWithThreatTypeAndIsolationSetting/SafeBrowsingBlockingPageBrowserTest.SecurityStateGoBackOnSubresourceInterstitial/1 -SafeBrowsingBlockingPageBrowserTestWithThreatTypeAndIsolationSetting/SafeBrowsingBlockingPageBrowserTest.SecurityStateGoBackOnSubresourceInterstitial/2 -SafeBrowsingBlockingPageBrowserTestWithThreatTypeAndIsolationSetting/SafeBrowsingBlockingPageBrowserTest.SecurityStateGoBackOnSubresourceInterstitial/3 -SafeBrowsingBlockingPageBrowserTestWithThreatTypeAndIsolationSetting/SafeBrowsingBlockingPageBrowserTest.SecurityStateGoBackOnSubresourceInterstitial/4 -SafeBrowsingBlockingPageBrowserTestWithThreatTypeAndIsolationSetting/SafeBrowsingBlockingPageBrowserTest.SecurityStateGoBackOnSubresourceInterstitial/5 +-SafeBrowsingTriggeredInterceptingBrowserTest.AbusiveMetadata -SafeBrowsingTriggeredPopupBlockerBrowserTest.BlockCreatingNewWindows -SafeBrowsingTriggeredPopupBlockerBrowserTest.BlockOpenURLFromTab -SafeBrowsingTriggeredPopupBlockerBrowserTest.BlockOpenURLFromTabInIframe -SafeBrowsingTriggeredPopupBlockerBrowserTest.MultipleNavigations -SafeBrowsingTriggeredPopupBlockerBrowserTest.WarningAllowCreatingNewWindows_LogsToConsole -SafeBrowsingTriggeredPopupBlockerBrowserTest.WarningDoNotBlockCreatingNewWindows_LogsToConsole +-SafeBrowsingTriggeredInterceptingBrowserTest.AbusiveMetadata -SavePageBrowserTest.SaveUnauthorizedResource -ServiceWorkerPaymentAppFactoryBrowserTest.AllOringsSupported -ServiceWorkerPaymentAppFactoryBrowserTest.SupportedOrigin @@ -1565,6 +1568,7 @@ -SocketApiTest.SocketUDPExtension -SpeechRecognitionTest.SpeechFromBackgroundPage -SpeechRecognitionTest.SpeechFromBackgroundPageWithoutPermission +-SSLUITestCommittedInterstitials.ErrorPageType -StreamsPrivateApiTest.Abort -StreamsPrivateApiTest.FileURL -StreamsPrivateApiTest.Headers
diff --git a/third_party/WebKit/LayoutTests/FlagExpectations/enable-browser-side-navigation b/third_party/WebKit/LayoutTests/FlagExpectations/enable-browser-side-navigation index 29147e6..7dfd7af60 100644 --- a/third_party/WebKit/LayoutTests/FlagExpectations/enable-browser-side-navigation +++ b/third_party/WebKit/LayoutTests/FlagExpectations/enable-browser-side-navigation
@@ -8,6 +8,7 @@ # These tests are flaky. Bug(none) http/tests/security/popup-allowed-by-sandbox-can-navigate.html [ Failure ] Bug(none) virtual/mojo-loading/http/tests/security/popup-allowed-by-sandbox-can-navigate.html [ Failure ] +Bug(none) virtual/threaded/fast/scroll-behavior/first-scroll-runs-on-compositor.html [ Failure ] # Not disclosing |source_location| and/or |blocked url| between cross-origin # renderers when a CSP policy is violated regresses the quality of console
diff --git a/third_party/WebKit/Source/core/loader/IdlenessDetector.cpp b/third_party/WebKit/Source/core/loader/IdlenessDetector.cpp index 9557026..ddb65680 100644 --- a/third_party/WebKit/Source/core/loader/IdlenessDetector.cpp +++ b/third_party/WebKit/Source/core/loader/IdlenessDetector.cpp
@@ -40,8 +40,7 @@ if (auto* frame_resource_coordinator = local_frame_->GetFrameResourceCoordinator()) { - frame_resource_coordinator->SetProperty( - resource_coordinator::mojom::PropertyType::kNetworkAlmostIdle, false); + frame_resource_coordinator->SetNetworkAlmostIdle(false); } OnDidLoadResource(); } @@ -124,8 +123,7 @@ network_2_quiet_start_time_); if (auto* frame_resource_coordinator = local_frame_->GetFrameResourceCoordinator()) { - frame_resource_coordinator->SetProperty( - resource_coordinator::mojom::PropertyType::kNetworkAlmostIdle, true); + frame_resource_coordinator->SetNetworkAlmostIdle(true); } local_frame_->GetDocument()->Fetcher()->OnNetworkQuiet(); network_2_quiet_ = -1;
diff --git a/third_party/WebKit/Source/modules/notifications/Notification.cpp b/third_party/WebKit/Source/modules/notifications/Notification.cpp index 2178967..08d04e4e 100644 --- a/third_party/WebKit/Source/modules/notifications/Notification.cpp +++ b/third_party/WebKit/Source/modules/notifications/Notification.cpp
@@ -126,9 +126,7 @@ if (document && document->GetFrame()) { if (auto* frame_resource_coordinator = document->GetFrame()->GetFrameResourceCoordinator()) { - frame_resource_coordinator->SendEvent( - resource_coordinator::mojom::Event:: - kNonPersistentNotificationCreated); + frame_resource_coordinator->OnNonPersistentNotificationCreated(); } }
diff --git a/third_party/WebKit/Source/platform/instrumentation/BUILD.gn b/third_party/WebKit/Source/platform/instrumentation/BUILD.gn index 8fe0f5d..bc8fa60 100644 --- a/third_party/WebKit/Source/platform/instrumentation/BUILD.gn +++ b/third_party/WebKit/Source/platform/instrumentation/BUILD.gn
@@ -6,7 +6,6 @@ sources = [ "PlatformInstrumentation.cpp", "PlatformInstrumentation.h", - "resource_coordinator/BlinkResourceCoordinatorBase.cpp", "resource_coordinator/BlinkResourceCoordinatorBase.h", "resource_coordinator/FrameResourceCoordinator.cpp", "resource_coordinator/FrameResourceCoordinator.h",
diff --git a/third_party/WebKit/Source/platform/instrumentation/resource_coordinator/BlinkResourceCoordinatorBase.cpp b/third_party/WebKit/Source/platform/instrumentation/resource_coordinator/BlinkResourceCoordinatorBase.cpp deleted file mode 100644 index ab7f03f..0000000 --- a/third_party/WebKit/Source/platform/instrumentation/resource_coordinator/BlinkResourceCoordinatorBase.cpp +++ /dev/null
@@ -1,50 +0,0 @@ -// Copyright 2017 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "platform/instrumentation/resource_coordinator/BlinkResourceCoordinatorBase.h" - -#include "platform/wtf/Functional.h" -#include "services/resource_coordinator/public/cpp/resource_coordinator_features.h" -#include "services/resource_coordinator/public/interfaces/coordination_unit_provider.mojom-blink.h" -#include "services/service_manager/public/cpp/connector.h" -#include "services/service_manager/public/cpp/interface_provider.h" - -namespace blink { - -// static -bool BlinkResourceCoordinatorBase::IsEnabled() { - return resource_coordinator::IsResourceCoordinatorEnabled(); -} - -BlinkResourceCoordinatorBase::~BlinkResourceCoordinatorBase() = default; - -BlinkResourceCoordinatorBase::BlinkResourceCoordinatorBase( - service_manager::InterfaceProvider* interface_provider) { - interface_provider->GetInterface(mojo::MakeRequest(&service_)); -} - -BlinkResourceCoordinatorBase::BlinkResourceCoordinatorBase( - service_manager::Connector* connector, - const std::string& service_name) { - connector->BindInterface(service_name, &service_); -} - -BlinkResourceCoordinatorBase::BlinkResourceCoordinatorBase() {} - -void BlinkResourceCoordinatorBase::SendEvent( - const resource_coordinator::mojom::blink::Event event) { - if (!service_) - return; - service_->SendEvent(event); -} - -void BlinkResourceCoordinatorBase::SetProperty( - const resource_coordinator::mojom::blink::PropertyType property_type, - int64_t value) { - // service_ can be uninitialized in testing. - if (service_) - service_->SetProperty(property_type, value); -} - -} // namespace blink
diff --git a/third_party/WebKit/Source/platform/instrumentation/resource_coordinator/BlinkResourceCoordinatorBase.h b/third_party/WebKit/Source/platform/instrumentation/resource_coordinator/BlinkResourceCoordinatorBase.h index 8536ddd..28fefe3 100644 --- a/third_party/WebKit/Source/platform/instrumentation/resource_coordinator/BlinkResourceCoordinatorBase.h +++ b/third_party/WebKit/Source/platform/instrumentation/resource_coordinator/BlinkResourceCoordinatorBase.h
@@ -7,37 +7,20 @@ #include "platform/PlatformExport.h" #include "platform/wtf/Noncopyable.h" -#include "services/resource_coordinator/public/interfaces/coordination_unit.mojom-blink.h" - -namespace service_manager { -class InterfaceProvider; -class Connector; -} // namespace service_manager +#include "services/resource_coordinator/public/cpp/resource_coordinator_features.h" namespace blink { -class InterfaceProvider; - // Base class for Resource Coordinators in Blink. class PLATFORM_EXPORT BlinkResourceCoordinatorBase { WTF_MAKE_NONCOPYABLE(BlinkResourceCoordinatorBase); public: - static bool IsEnabled(); - virtual ~BlinkResourceCoordinatorBase(); + static bool IsEnabled() { + return resource_coordinator::IsResourceCoordinatorEnabled(); + } - void SendEvent(const resource_coordinator::mojom::blink::Event); - void SetProperty(const resource_coordinator::mojom::blink::PropertyType, - int64_t); - - protected: - explicit BlinkResourceCoordinatorBase(service_manager::InterfaceProvider*); - BlinkResourceCoordinatorBase(service_manager::Connector*, - const std::string& service_name); - BlinkResourceCoordinatorBase(); - - private: - resource_coordinator::mojom::blink::CoordinationUnitPtr service_; + BlinkResourceCoordinatorBase() = default; }; } // namespace blink
diff --git a/third_party/WebKit/Source/platform/instrumentation/resource_coordinator/FrameResourceCoordinator.cpp b/third_party/WebKit/Source/platform/instrumentation/resource_coordinator/FrameResourceCoordinator.cpp index 524337c..ea64ef6 100644 --- a/third_party/WebKit/Source/platform/instrumentation/resource_coordinator/FrameResourceCoordinator.cpp +++ b/third_party/WebKit/Source/platform/instrumentation/resource_coordinator/FrameResourceCoordinator.cpp
@@ -15,9 +15,22 @@ } FrameResourceCoordinator::FrameResourceCoordinator( - service_manager::InterfaceProvider* interface_provider) - : BlinkResourceCoordinatorBase(interface_provider) {} + service_manager::InterfaceProvider* interface_provider) { + interface_provider->GetInterface(mojo::MakeRequest(&service_)); +} FrameResourceCoordinator::~FrameResourceCoordinator() = default; +void FrameResourceCoordinator::SetNetworkAlmostIdle(bool idle) { + if (!service_) + return; + service_->SetNetworkAlmostIdle(idle); +} + +void FrameResourceCoordinator::OnNonPersistentNotificationCreated() { + if (!service_) + return; + service_->OnNonPersistentNotificationCreated(); +} + } // namespace blink
diff --git a/third_party/WebKit/Source/platform/instrumentation/resource_coordinator/FrameResourceCoordinator.h b/third_party/WebKit/Source/platform/instrumentation/resource_coordinator/FrameResourceCoordinator.h index 1983642..3023a6ef 100644 --- a/third_party/WebKit/Source/platform/instrumentation/resource_coordinator/FrameResourceCoordinator.h +++ b/third_party/WebKit/Source/platform/instrumentation/resource_coordinator/FrameResourceCoordinator.h
@@ -6,8 +6,7 @@ #define FrameResourceCoordinator_h #include "platform/instrumentation/resource_coordinator/BlinkResourceCoordinatorBase.h" - -#include "platform/wtf/Noncopyable.h" +#include "services/resource_coordinator/public/interfaces/coordination_unit.mojom-blink.h" namespace service_manager { class InterfaceProvider; @@ -21,10 +20,15 @@ public: static FrameResourceCoordinator* Create(service_manager::InterfaceProvider*); - ~FrameResourceCoordinator() override; + ~FrameResourceCoordinator(); + + void SetNetworkAlmostIdle(bool); + void OnNonPersistentNotificationCreated(); private: explicit FrameResourceCoordinator(service_manager::InterfaceProvider*); + + resource_coordinator::mojom::blink::FrameCoordinationUnitPtr service_; }; } // namespace blink
diff --git a/third_party/WebKit/Source/platform/instrumentation/resource_coordinator/RendererResourceCoordinator.cpp b/third_party/WebKit/Source/platform/instrumentation/resource_coordinator/RendererResourceCoordinator.cpp index 8707392..0e45d74 100644 --- a/third_party/WebKit/Source/platform/instrumentation/resource_coordinator/RendererResourceCoordinator.cpp +++ b/third_party/WebKit/Source/platform/instrumentation/resource_coordinator/RendererResourceCoordinator.cpp
@@ -6,6 +6,7 @@ #include "platform/heap/ThreadState.h" #include "public/platform/Platform.h" +#include "services/service_manager/public/cpp/connector.h" namespace blink { @@ -39,11 +40,19 @@ RendererResourceCoordinator::RendererResourceCoordinator( service_manager::Connector* connector, - const std::string& service_name) - : BlinkResourceCoordinatorBase(connector, service_name) {} + const std::string& service_name) { + connector->BindInterface(service_name, &service_); +} RendererResourceCoordinator::RendererResourceCoordinator() {} RendererResourceCoordinator::~RendererResourceCoordinator() = default; +void RendererResourceCoordinator::SetExpectedTaskQueueingDuration( + base::TimeDelta duration) { + if (!service_) + return; + service_->SetExpectedTaskQueueingDuration(duration); +} + } // namespace blink
diff --git a/third_party/WebKit/Source/platform/instrumentation/resource_coordinator/RendererResourceCoordinator.h b/third_party/WebKit/Source/platform/instrumentation/resource_coordinator/RendererResourceCoordinator.h index 7975649..3d23072 100644 --- a/third_party/WebKit/Source/platform/instrumentation/resource_coordinator/RendererResourceCoordinator.h +++ b/third_party/WebKit/Source/platform/instrumentation/resource_coordinator/RendererResourceCoordinator.h
@@ -6,8 +6,7 @@ #define RendererResourceCoordinator_h #include "platform/instrumentation/resource_coordinator/BlinkResourceCoordinatorBase.h" - -#include "platform/wtf/Noncopyable.h" +#include "services/resource_coordinator/public/interfaces/coordination_unit.mojom-blink.h" namespace service_manager { class Connector; @@ -27,7 +26,9 @@ static void SetCurrentRendererResourceCoordinatorForTesting( RendererResourceCoordinator*); - ~RendererResourceCoordinator() override; + ~RendererResourceCoordinator(); + + void SetExpectedTaskQueueingDuration(base::TimeDelta duration); protected: RendererResourceCoordinator(); @@ -35,6 +36,8 @@ private: RendererResourceCoordinator(service_manager::Connector*, const std::string& service_name); + + resource_coordinator::mojom::blink::ProcessCoordinationUnitPtr service_; }; } // namespace blink
diff --git a/third_party/WebKit/Source/platform/scheduler/base/cancelable_closure_holder.cc b/third_party/WebKit/Source/platform/scheduler/base/cancelable_closure_holder.cc index 808e351..c77b074 100644 --- a/third_party/WebKit/Source/platform/scheduler/base/cancelable_closure_holder.cc +++ b/third_party/WebKit/Source/platform/scheduler/base/cancelable_closure_holder.cc
@@ -21,7 +21,7 @@ cancelable_callback_.Reset(callback_); } -const base::Closure& CancelableClosureHolder::GetCallback() const { +base::Closure CancelableClosureHolder::GetCallback() const { DCHECK(!callback_.is_null()); return cancelable_callback_.callback(); }
diff --git a/third_party/WebKit/Source/platform/scheduler/base/cancelable_closure_holder.h b/third_party/WebKit/Source/platform/scheduler/base/cancelable_closure_holder.h index 05d62cb4..626eabd 100644 --- a/third_party/WebKit/Source/platform/scheduler/base/cancelable_closure_holder.h +++ b/third_party/WebKit/Source/platform/scheduler/base/cancelable_closure_holder.h
@@ -27,7 +27,7 @@ // Returns a callback that will be disabled by calling Cancel(). Callback // must have been set using Reset() before calling this function. - const base::Closure& GetCallback() const; + base::Closure GetCallback() const; private: base::Closure callback_;
diff --git a/third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc b/third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc index e7ffd7a..bb72371e 100644 --- a/third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc +++ b/third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc
@@ -2042,11 +2042,9 @@ "estimated_queueing_time_for_window", queueing_time.InMillisecondsF()); - if (BlinkResourceCoordinatorBase::IsEnabled()) { - RendererResourceCoordinator::Get().SetProperty( - resource_coordinator::mojom::PropertyType:: - kExpectedTaskQueueingDuration, - queueing_time.InMilliseconds()); + if (::resource_coordinator::IsResourceCoordinatorEnabled()) { + RendererResourceCoordinator::Get().SetExpectedTaskQueueingDuration( + queueing_time); } }
diff --git a/third_party/WebKit/Source/platform/wtf/Functional.h b/third_party/WebKit/Source/platform/wtf/Functional.h index 43a68b5..c33be44 100644 --- a/third_party/WebKit/Source/platform/wtf/Functional.h +++ b/third_party/WebKit/Source/platform/wtf/Functional.h
@@ -172,10 +172,10 @@ return UnretainedWrapper<T, kCrossThreadAffinity>(value); } -template <typename T> -struct ParamStorageTraits { - typedef T StorageType; +namespace internal { +template <size_t, typename T> +struct CheckGCedTypeRestriction { static_assert(!std::is_pointer<T>::value, "Raw pointers are not allowed to bind into WTF::Function. Wrap " "it with either WrapPersistent, WrapWeakPersistent, " @@ -191,28 +191,16 @@ "GCed type is forbidden as a bound parameters."); }; -template <typename T> -struct ParamStorageTraits<scoped_refptr<T>> { - typedef scoped_refptr<T> StorageType; +template <typename Index, typename... Args> +struct CheckGCedTypeRestrictions; + +template <size_t... Ns, typename... Args> +struct CheckGCedTypeRestrictions<std::index_sequence<Ns...>, Args...> + : CheckGCedTypeRestriction<Ns, Args>... { + static constexpr bool ok = true; }; -template <typename> -class RetainPtr; - -template <typename T> -struct ParamStorageTraits<RetainPtr<T>> { - typedef RetainPtr<T> StorageType; -}; - -template <typename T> -struct ParamStorageTraits<PassedWrapper<T>> { - typedef PassedWrapper<T> StorageType; -}; - -template <typename T, FunctionThreadAffinity threadAffinity> -struct ParamStorageTraits<UnretainedWrapper<T, threadAffinity>> { - typedef UnretainedWrapper<T, threadAffinity> StorageType; -}; +} // namespace internal template <typename Signature, FunctionThreadAffinity threadAffinity = kSameThreadAffinity> @@ -280,12 +268,14 @@ Function<base::MakeUnboundRunType<FunctionType, BoundParameters...>, threadAffinity> BindInternal(FunctionType function, BoundParameters&&... bound_parameters) { + static_assert(internal::CheckGCedTypeRestrictions< + std::index_sequence_for<BoundParameters...>, + std::decay_t<BoundParameters>...>::ok, + "A bound argument uses a bad pattern."); using UnboundRunType = base::MakeUnboundRunType<FunctionType, BoundParameters...>; - return Function<UnboundRunType, threadAffinity>(base::Bind( - function, - typename ParamStorageTraits<typename std::decay<BoundParameters>::type>:: - StorageType(std::forward<BoundParameters>(bound_parameters))...)); + return Function<UnboundRunType, threadAffinity>( + base::Bind(function, std::forward<BoundParameters>(bound_parameters)...)); } template <typename FunctionType, typename... BoundParameters>
diff --git a/third_party/WebKit/Source/platform/wtf/FunctionalTest.cpp b/third_party/WebKit/Source/platform/wtf/FunctionalTest.cpp index cc5c9cd..a4f29f0 100644 --- a/third_party/WebKit/Source/platform/wtf/FunctionalTest.cpp +++ b/third_party/WebKit/Source/platform/wtf/FunctionalTest.cpp
@@ -44,37 +44,6 @@ int value_; }; -// This class must be wrapped in bind() and unwrapped in closure execution. -class ClassToBeWrapped { - WTF_MAKE_NONCOPYABLE(ClassToBeWrapped); - - public: - explicit ClassToBeWrapped(int value) : value_(value) {} - - int Value() const { return value_; } - - private: - int value_; -}; - -class WrappedClass { - public: - WrappedClass(const ClassToBeWrapped& to_be_wrapped) - : value_(to_be_wrapped.Value()) {} - - explicit WrappedClass(int value) : value_(value) {} - - UnwrappedClass Unwrap() const { return UnwrappedClass(value_); } - - private: - int value_; -}; - -template <> -struct ParamStorageTraits<ClassToBeWrapped> { - using StorageType = WrappedClass; -}; - class HasWeakPtrSupport { public: HasWeakPtrSupport() : weak_ptr_factory_(this) {} @@ -93,17 +62,6 @@ } // namespace WTF -namespace base { - -template <> -struct BindUnwrapTraits<WTF::WrappedClass> { - static WTF::UnwrappedClass Unwrap(const WTF::WrappedClass& wrapped) { - return wrapped.Unwrap(); - } -}; - -} // namespace base - namespace WTF { namespace {
diff --git a/ui/gfx/native_pixmap.h b/ui/gfx/native_pixmap.h index 1677dc0..c168001 100644 --- a/ui/gfx/native_pixmap.h +++ b/ui/gfx/native_pixmap.h
@@ -34,6 +34,14 @@ virtual gfx::BufferFormat GetBufferFormat() const = 0; virtual gfx::Size GetBufferSize() const = 0; + // Return an id that is guaranteed to be unique and equal for all instances + // of this NativePixmap backed by the same buffer, for the duration of its + // lifetime. If such id cannot be generated, 0 (an invalid id) is returned. + // + // TODO(posciak): crbug.com/771863, remove this once a different mechanism + // for protected shared memory buffers is implemented. + virtual uint32_t GetUniqueId() const = 0; + // Sets the overlay plane to switch to at the next page flip. // |widget| specifies the screen to display this overlay plane on. // |plane_z_order| specifies the stacking order of the plane relative to the
diff --git a/ui/ozone/platform/cast/surface_factory_cast.cc b/ui/ozone/platform/cast/surface_factory_cast.cc index 6cc5b50a..234c3f9 100644 --- a/ui/ozone/platform/cast/surface_factory_cast.cc +++ b/ui/ozone/platform/cast/surface_factory_cast.cc
@@ -63,6 +63,7 @@ return gfx::BufferFormat::BGRA_8888; } gfx::Size GetBufferSize() const override { return gfx::Size(); } + uint32_t GetUniqueId() const override { return 0; } bool ScheduleOverlayPlane(gfx::AcceleratedWidget widget, int plane_z_order,
diff --git a/ui/ozone/platform/drm/gpu/gbm_buffer.cc b/ui/ozone/platform/drm/gpu/gbm_buffer.cc index e7b24aa..38ad6cb 100644 --- a/ui/ozone/platform/drm/gpu/gbm_buffer.cc +++ b/ui/ozone/platform/drm/gpu/gbm_buffer.cc
@@ -261,11 +261,11 @@ // Try to use scanout if supported. int gbm_flags = GBM_BO_USE_SCANOUT | GBM_BO_USE_TEXTURING; - bool try_scanout = - gbm_device_is_format_supported(gbm->device(), format, gbm_flags); + if (!gbm_device_is_format_supported(gbm->device(), format, gbm_flags)) + gbm_flags &= ~GBM_BO_USE_SCANOUT; gbm_bo* bo = nullptr; - if (try_scanout) { + if (gbm_device_is_format_supported(gbm->device(), format, gbm_flags)) { struct gbm_import_fd_planar_data fd_data; fd_data.width = size.width(); fd_data.height = size.height(); @@ -287,8 +287,6 @@ LOG(ERROR) << "nullptr returned from gbm_bo_import"; return nullptr; } - } else { - gbm_flags &= ~GBM_BO_USE_SCANOUT; } scoped_refptr<GbmBuffer> buffer(new GbmBuffer(gbm, bo, format, gbm_flags, 0, @@ -365,6 +363,10 @@ return buffer_->GetSize(); } +uint32_t GbmPixmap::GetUniqueId() const { + return buffer_->GetHandle(); +} + bool GbmPixmap::ScheduleOverlayPlane(gfx::AcceleratedWidget widget, int plane_z_order, gfx::OverlayTransform plane_transform,
diff --git a/ui/ozone/platform/drm/gpu/gbm_buffer.h b/ui/ozone/platform/drm/gpu/gbm_buffer.h index 6715274..4891b8f 100644 --- a/ui/ozone/platform/drm/gpu/gbm_buffer.h +++ b/ui/ozone/platform/drm/gpu/gbm_buffer.h
@@ -119,6 +119,7 @@ uint64_t GetDmaBufModifier(size_t plane) const override; gfx::BufferFormat GetBufferFormat() const override; gfx::Size GetBufferSize() const override; + uint32_t GetUniqueId() const override; bool ScheduleOverlayPlane(gfx::AcceleratedWidget widget, int plane_z_order, gfx::OverlayTransform plane_transform,
diff --git a/ui/ozone/platform/drm/gpu/gbm_surface_factory.cc b/ui/ozone/platform/drm/gpu/gbm_surface_factory.cc index cbb3e077..ca8a520 100644 --- a/ui/ozone/platform/drm/gpu/gbm_surface_factory.cc +++ b/ui/ozone/platform/drm/gpu/gbm_surface_factory.cc
@@ -160,7 +160,7 @@ } scoped_refptr<gfx::NativePixmap> -GbmSurfaceFactory::CreateNativePixmapFromHandle( +GbmSurfaceFactory::CreateNativePixmapFromHandleInternal( gfx::AcceleratedWidget widget, gfx::Size size, gfx::BufferFormat format, @@ -176,7 +176,6 @@ } std::vector<gfx::NativePixmapPlane> planes; - for (const auto& plane : handle.planes) { planes.push_back(plane); } @@ -185,7 +184,44 @@ widget, size, format, std::move(scoped_fds), planes); if (!buffer) return nullptr; + return base::MakeRefCounted<GbmPixmap>(this, buffer); } +scoped_refptr<gfx::NativePixmap> +GbmSurfaceFactory::CreateNativePixmapFromHandle( + gfx::AcceleratedWidget widget, + gfx::Size size, + gfx::BufferFormat format, + const gfx::NativePixmapHandle& handle) { + // Query the external service (if available), whether it recognizes this + // NativePixmapHandle, and whether it can provide a corresponding NativePixmap + // backing it. If so, the handle is consumed. Otherwise, the handle remains + // valid and can be further importer by standard means. + if (!get_protected_native_pixmap_callback_.is_null()) { + auto protected_pixmap = get_protected_native_pixmap_callback_.Run(handle); + if (protected_pixmap) + return protected_pixmap; + } + + return CreateNativePixmapFromHandleInternal(widget, size, format, handle); +} + +scoped_refptr<gfx::NativePixmap> +GbmSurfaceFactory::CreateNativePixmapForProtectedBufferHandle( + gfx::AcceleratedWidget widget, + gfx::Size size, + gfx::BufferFormat format, + const gfx::NativePixmapHandle& handle) { + // Create a new NativePixmap without querying the external service for any + // existing mappings. + return CreateNativePixmapFromHandleInternal(widget, size, format, handle); +} + +void GbmSurfaceFactory::SetGetProtectedNativePixmapDelegate( + const GetProtectedNativePixmapCallback& + get_protected_native_pixmap_callback) { + get_protected_native_pixmap_callback_ = get_protected_native_pixmap_callback; +} + } // namespace ui
diff --git a/ui/ozone/platform/drm/gpu/gbm_surface_factory.h b/ui/ozone/platform/drm/gpu/gbm_surface_factory.h index ce1abfdc..3955c1c1 100644 --- a/ui/ozone/platform/drm/gpu/gbm_surface_factory.h +++ b/ui/ozone/platform/drm/gpu/gbm_surface_factory.h
@@ -50,8 +50,22 @@ gfx::Size size, gfx::BufferFormat format, const gfx::NativePixmapHandle& handle) override; + void SetGetProtectedNativePixmapDelegate( + const GetProtectedNativePixmapCallback& + get_protected_native_pixmap_callback) override; + scoped_refptr<gfx::NativePixmap> CreateNativePixmapForProtectedBufferHandle( + gfx::AcceleratedWidget widget, + gfx::Size size, + gfx::BufferFormat format, + const gfx::NativePixmapHandle& handle) override; private: + scoped_refptr<gfx::NativePixmap> CreateNativePixmapFromHandleInternal( + gfx::AcceleratedWidget widget, + gfx::Size size, + gfx::BufferFormat format, + const gfx::NativePixmapHandle& handle); + std::unique_ptr<GLOzone> egl_implementation_; std::unique_ptr<GLOzone> osmesa_implementation_; @@ -61,6 +75,8 @@ std::map<gfx::AcceleratedWidget, GbmSurfaceless*> widget_to_surface_map_; + GetProtectedNativePixmapCallback get_protected_native_pixmap_callback_; + DISALLOW_COPY_AND_ASSIGN(GbmSurfaceFactory); };
diff --git a/ui/ozone/platform/headless/headless_surface_factory.cc b/ui/ozone/platform/headless/headless_surface_factory.cc index dcfcf54..4ba3929 100644 --- a/ui/ozone/platform/headless/headless_surface_factory.cc +++ b/ui/ozone/platform/headless/headless_surface_factory.cc
@@ -83,6 +83,7 @@ uint64_t GetDmaBufModifier(size_t plane) const override { return 0; } gfx::BufferFormat GetBufferFormat() const override { return format_; } gfx::Size GetBufferSize() const override { return gfx::Size(); } + uint32_t GetUniqueId() const override { return 0; } bool ScheduleOverlayPlane(gfx::AcceleratedWidget widget, int plane_z_order, gfx::OverlayTransform plane_transform,
diff --git a/ui/ozone/public/surface_factory_ozone.cc b/ui/ozone/public/surface_factory_ozone.cc index 527aa29..c7c2dc3 100644 --- a/ui/ozone/public/surface_factory_ozone.cc +++ b/ui/ozone/public/surface_factory_ozone.cc
@@ -52,4 +52,17 @@ return nullptr; } +scoped_refptr<gfx::NativePixmap> +SurfaceFactoryOzone::CreateNativePixmapForProtectedBufferHandle( + gfx::AcceleratedWidget widget, + gfx::Size size, + gfx::BufferFormat format, + const gfx::NativePixmapHandle& handle) { + return nullptr; +} + +void SurfaceFactoryOzone::SetGetProtectedNativePixmapDelegate( + const GetProtectedNativePixmapCallback& + get_protected_native_pixmap_callback) {} + } // namespace ui
diff --git a/ui/ozone/public/surface_factory_ozone.h b/ui/ozone/public/surface_factory_ozone.h index 6e7726a..1b7ba91 100644 --- a/ui/ozone/public/surface_factory_ozone.h +++ b/ui/ozone/public/surface_factory_ozone.h
@@ -99,6 +99,38 @@ gfx::BufferFormat format, const gfx::NativePixmapHandle& handle); + // A temporary solution that allows protected NativePixmap management to be + // handled outside the Ozone platform (crbug.com/771863). + // The current implementation uses dummy NativePixmaps as transparent handles + // to separate NativePixmaps with actual contents. This method takes + // a NativePixmapHandle to such a dummy pixmap, and creates a NativePixmap + // instance for it. + virtual scoped_refptr<gfx::NativePixmap> + CreateNativePixmapForProtectedBufferHandle( + gfx::AcceleratedWidget widget, + gfx::Size size, + gfx::BufferFormat format, + const gfx::NativePixmapHandle& handle); + + // This callback can be used by implementations of this interface to query + // for a NativePixmap for the given NativePixmapHandle, instead of importing + // it via standard means. This happens if an external service is maintaining + // a separate mapping of NativePixmapHandles to NativePixmaps. + // If this callback returns non-nullptr, the returned NativePixmap should + // be used instead of the NativePixmap that would have been produced by the + // standard, implementation-specific NativePixmapHandle import mechanism. + using GetProtectedNativePixmapCallback = + base::Callback<scoped_refptr<gfx::NativePixmap>( + const gfx::NativePixmapHandle&)>; + // Called by an external service to set the GetProtectedNativePixmapCallback, + // to be used by the implementation when importing NativePixmapHandles. + // TODO(posciak): crbug.com/778555, move this to platform-specific + // implementation(s) and make protected pixmap handling transparent to the + // clients of this interface, removing the need for this callback. + virtual void SetGetProtectedNativePixmapDelegate( + const GetProtectedNativePixmapCallback& + get_protected_native_pixmap_callback); + protected: SurfaceFactoryOzone(); virtual ~SurfaceFactoryOzone();