New issue
Advanced search Search tips

Issue 737320 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task

Blocking:
issue 751269



Sign in to add a comment

[Media Router] Factor extension-related logic out of MediaRouterMojoImpl

Project Member Reported by taku...@chromium.org, Jun 28 2017

Issue description

Event page waking and request queueing should be moved out of MediaRouterMojoImpl to allow its reuse by MediaRouteController.

Design doc: https://docs.google.com/document/d/1PEcNLc9TTaRbeC1jZwVX7E3Rm3bJrqV8049cKXQ7Yho/edit
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0e3adbf3d7c3a0436a5d4007505fd3590489049e

commit 0e3adbf3d7c3a0436a5d4007505fd3590489049e
Author: takumif <takumif@chromium.org>
Date: Thu Jul 06 18:38:25 2017

[Media Router] Factor extension-related logic out of MediaRouterMojoImpl

We take the component-extension-waking and request-queueing logic out of
MediaRouterMojoImpl and put it in its own class, EventPageRequestManager. In a
future patch the part of MediaRouterMojoImpl that interfaces with the request
manager will be further refactored into its own class, as described in the
design doc below.

Currently MediaRouterMojoImpl is the only client of the request manager, but the
MediaRouteController will also use it in the future (see design doc).

Design doc:
https://docs.google.com/document/d/1PEcNLc9TTaRbeC1jZwVX7E3Rm3bJrqV8049cKXQ7Yho/edit

BUG= 737320 
BUG= 739516 

Review-Url: https://codereview.chromium.org/2949933002
Cr-Commit-Position: refs/heads/master@{#484688}

[modify] https://crrev.com/0e3adbf3d7c3a0436a5d4007505fd3590489049e/chrome/browser/media/router/BUILD.gn
[add] https://crrev.com/0e3adbf3d7c3a0436a5d4007505fd3590489049e/chrome/browser/media/router/event_page_request_manager.cc
[add] https://crrev.com/0e3adbf3d7c3a0436a5d4007505fd3590489049e/chrome/browser/media/router/event_page_request_manager.h
[add] https://crrev.com/0e3adbf3d7c3a0436a5d4007505fd3590489049e/chrome/browser/media/router/event_page_request_manager_factory.cc
[add] https://crrev.com/0e3adbf3d7c3a0436a5d4007505fd3590489049e/chrome/browser/media/router/event_page_request_manager_factory.h
[add] https://crrev.com/0e3adbf3d7c3a0436a5d4007505fd3590489049e/chrome/browser/media/router/event_page_request_manager_unittest.cc
[modify] https://crrev.com/0e3adbf3d7c3a0436a5d4007505fd3590489049e/chrome/browser/media/router/media_router_factory.cc
[modify] https://crrev.com/0e3adbf3d7c3a0436a5d4007505fd3590489049e/chrome/browser/media/router/mojo/media_router_mojo_impl.cc
[modify] https://crrev.com/0e3adbf3d7c3a0436a5d4007505fd3590489049e/chrome/browser/media/router/mojo/media_router_mojo_impl.h
[modify] https://crrev.com/0e3adbf3d7c3a0436a5d4007505fd3590489049e/chrome/browser/media/router/mojo/media_router_mojo_impl_unittest.cc
[modify] https://crrev.com/0e3adbf3d7c3a0436a5d4007505fd3590489049e/chrome/browser/media/router/mojo/media_router_mojo_test.cc
[modify] https://crrev.com/0e3adbf3d7c3a0436a5d4007505fd3590489049e/chrome/browser/media/router/mojo/media_router_mojo_test.h
[modify] https://crrev.com/0e3adbf3d7c3a0436a5d4007505fd3590489049e/chrome/test/BUILD.gn

Blocking: 751269
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dfc23d41391b293aa4dde0e41f912f2bca23fe22

commit dfc23d41391b293aa4dde0e41f912f2bca23fe22
Author: Takumi Fujimoto <takumif@chromium.org>
Date: Thu Aug 03 22:32:48 2017

[Media Router] Factor out calls to EventPageRequestManager out of MediaRouterMojoImpl into its own class

On desktop, the calls to the MRP are queued with EventPageRequestManager until
the Mojo connections with the extension are established. This CL factors out the
calls to EventPageRequestManager into MediaRouterDesktop, a subclass of
MediaRouterMojoImpl, so that MediaRouterMojoImpl is not aware of extension-
related details.

Changes to unit tests:
- Move tests from MRMojoImplTest to MRMojoTest so that they can shared with
  MRDesktopTest
- Merge MRMojoExtensionTest into MRDesktopTest

Bug:  737320 
Change-Id: I8271932b0418af2490a2bbea49e444120d9e2aa6
Reviewed-on: https://chromium-review.googlesource.com/572501
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Derek Cheng <imcheng@chromium.org>
Commit-Queue: Takumi Fujimoto <takumif@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491861}
[modify] https://crrev.com/dfc23d41391b293aa4dde0e41f912f2bca23fe22/chrome/browser/extensions/chrome_extensions_interface_registration.cc
[modify] https://crrev.com/dfc23d41391b293aa4dde0e41f912f2bca23fe22/chrome/browser/media/router/BUILD.gn
[modify] https://crrev.com/dfc23d41391b293aa4dde0e41f912f2bca23fe22/chrome/browser/media/router/event_page_request_manager.cc
[modify] https://crrev.com/dfc23d41391b293aa4dde0e41f912f2bca23fe22/chrome/browser/media/router/event_page_request_manager.h
[modify] https://crrev.com/dfc23d41391b293aa4dde0e41f912f2bca23fe22/chrome/browser/media/router/media_router_factory.cc
[add] https://crrev.com/dfc23d41391b293aa4dde0e41f912f2bca23fe22/chrome/browser/media/router/mojo/media_router_desktop.cc
[add] https://crrev.com/dfc23d41391b293aa4dde0e41f912f2bca23fe22/chrome/browser/media/router/mojo/media_router_desktop.h
[add] https://crrev.com/dfc23d41391b293aa4dde0e41f912f2bca23fe22/chrome/browser/media/router/mojo/media_router_desktop_unittest.cc
[modify] https://crrev.com/dfc23d41391b293aa4dde0e41f912f2bca23fe22/chrome/browser/media/router/mojo/media_router_mojo_impl.cc
[modify] https://crrev.com/dfc23d41391b293aa4dde0e41f912f2bca23fe22/chrome/browser/media/router/mojo/media_router_mojo_impl.h
[modify] https://crrev.com/dfc23d41391b293aa4dde0e41f912f2bca23fe22/chrome/browser/media/router/mojo/media_router_mojo_impl_unittest.cc
[modify] https://crrev.com/dfc23d41391b293aa4dde0e41f912f2bca23fe22/chrome/browser/media/router/mojo/media_router_mojo_test.cc
[modify] https://crrev.com/dfc23d41391b293aa4dde0e41f912f2bca23fe22/chrome/browser/media/router/mojo/media_router_mojo_test.h
[modify] https://crrev.com/dfc23d41391b293aa4dde0e41f912f2bca23fe22/chrome/test/BUILD.gn

Status: Fixed (was: Assigned)

Sign in to add a comment