Skip to content

Commit

Permalink
Reorganize Media Router ID type aliases into subtypes.
Browse files Browse the repository at this point in the history
Types like "MediaRouteId" turn into "MediaRoute::Id", which
declutters the global namespace and makes better use of the class
structure that's already in place.

R=imcheng@chromium.org
BUG=

Review URL: https://codereview.chromium.org/1130763008

Cr-Commit-Position: refs/heads/master@{#332621}
  • Loading branch information
kmarshall authored and Commit bot committed Jun 3, 2015
1 parent c78e58d commit 5fa0263
Show file tree
Hide file tree
Showing 21 changed files with 87 additions and 104 deletions.
2 changes: 1 addition & 1 deletion chrome/browser/media/router/issue.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ Issue::Issue(const std::string& title,
const std::string& message,
const IssueAction& default_action,
const std::vector<IssueAction>& secondary_actions,
const MediaRouteId& route_id,
const MediaRoute::Id& route_id,
const Issue::Severity severity,
bool is_blocking,
const std::string& help_url)
Expand Down
10 changes: 5 additions & 5 deletions chrome/browser/media/router/issue.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class IssueAction {
// Contains the information relevant to an issue.
class Issue {
public:
using IssueId = std::string;
using Id = std::string;

enum Severity { FATAL, WARNING, NOTIFICATION };

Expand All @@ -51,7 +51,7 @@ class Issue {
const std::string& message,
const IssueAction& default_action,
const std::vector<IssueAction>& secondary_actions,
const MediaRouteId& route_id,
const MediaRoute::Id& route_id,
const Severity severity,
bool is_blocking,
const std::string& helpUrl);
Expand All @@ -65,9 +65,9 @@ class Issue {
const std::vector<IssueAction>& secondary_actions() const {
return secondary_actions_;
}
MediaRouteId route_id() const { return route_id_; }
MediaRoute::Id route_id() const { return route_id_; }
Severity severity() const { return severity_; }
const IssueId& id() const { return id_; }
const Issue::Id& id() const { return id_; }
bool is_blocking() const { return is_blocking_; }
bool is_global() const { return route_id_.empty(); }
const GURL& help_url() const { return help_url_; }
Expand All @@ -81,7 +81,7 @@ class Issue {
std::vector<IssueAction> secondary_actions_;
std::string route_id_;
Severity severity_;
IssueId id_;
Issue::Id id_;
bool is_blocking_;
GURL help_url_;
};
Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/media/router/issue_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ void IssueManager::AddIssue(const Issue& issue) {
MaybeUpdateTopIssue();
}

void IssueManager::ClearIssue(const Issue::IssueId& issue_id) {
void IssueManager::ClearIssue(const Issue::Id& issue_id) {
DCHECK(thread_checker_.CalledOnValidThread());
issues_.erase(std::remove_if(issues_.begin(), issues_.end(),
[&issue_id](const Issue& issue) {
Expand Down Expand Up @@ -55,7 +55,7 @@ void IssueManager::ClearGlobalIssues() {
MaybeUpdateTopIssue();
}

void IssueManager::ClearIssuesWithRouteId(const MediaRouteId& route_id) {
void IssueManager::ClearIssuesWithRouteId(const MediaRoute::Id& route_id) {
DCHECK(thread_checker_.CalledOnValidThread());
issues_.erase(std::remove_if(issues_.begin(), issues_.end(),
[&route_id](const Issue& issue) {
Expand Down
10 changes: 5 additions & 5 deletions chrome/browser/media/router/issue_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ class IssueManager {
void AddIssue(const Issue& issue);

// Removes an issue when user has noted it is resolved.
// |issue_id|: Issue::IssueId of the issue to be removed.
void ClearIssue(const Issue::IssueId& issue_id);
// |issue_id|: Issue::Id of the issue to be removed.
void ClearIssue(const Issue::Id& issue_id);

// Gets the number of unresolved issues.
size_t GetIssueCount() const;
Expand All @@ -41,8 +41,8 @@ class IssueManager {
void ClearGlobalIssues();

// Removes all unresolved issues with RouteId.
// |route_id|: MediaRouteId of the issues to be removed.
void ClearIssuesWithRouteId(const MediaRouteId& route_id);
// |route_id|: ID of the media route whose issues are to be cleared.
void ClearIssuesWithRouteId(const MediaRoute::Id& route_id);

// Registers an issue observer |observer|. The observer will be triggered
// when the highest priority issue changes.
Expand All @@ -67,7 +67,7 @@ class IssueManager {
base::ObserverList<IssuesObserver> issues_observers_;

// The ID of the current top issue.
Issue::IssueId top_issue_id_;
Issue::Id top_issue_id_;

base::ThreadChecker thread_checker_;

Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/media/router/media_route.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

namespace media_router {

MediaRoute::MediaRoute(const MediaRouteId& media_route_id,
MediaRoute::MediaRoute(const MediaRoute::Id& media_route_id,
const MediaSource& media_source,
const MediaSink& media_sink,
const std::string& description,
Expand Down
9 changes: 5 additions & 4 deletions chrome/browser/media/router/media_route.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
#include "base/gtest_prod_util.h"
#include "base/logging.h"
#include "base/values.h"
#include "chrome/browser/media/router/media_route_id.h"
#include "chrome/browser/media/router/media_sink.h"
#include "chrome/browser/media/router/media_source.h"
#include "url/gurl.h"
Expand All @@ -33,21 +32,23 @@ enum MediaRouteState {
// be retrieved as new MediaRoute objects from the Media Router.
class MediaRoute {
public:
using Id = std::string;

// |media_route_id|: ID of the route. New route IDs should be created
// by the RouteIdManager class.
// |media_source|: Description of source of the route.
// |media_sink|: The sink that is receiving the media.
// |description|: Description of the route to be displayed.
// |is_local|: true if the route was created from this browser.
MediaRoute(const MediaRouteId& media_route_id,
MediaRoute(const MediaRoute::Id& media_route_id,
const MediaSource& media_source,
const MediaSink& media_sink,
const std::string& description,
bool is_local);
~MediaRoute();

// The media route identifier.
const MediaRouteId& media_route_id() const { return media_route_id_; }
const MediaRoute::Id& media_route_id() const { return media_route_id_; }

// The state of the media route.
MediaRouteState state() const { return state_; }
Expand All @@ -70,7 +71,7 @@ class MediaRoute {
bool Equals(const MediaRoute& other) const;

private:
MediaRouteId media_route_id_;
MediaRoute::Id media_route_id_;
MediaSource media_source_;
MediaSink media_sink_;
std::string description_;
Expand Down
16 changes: 0 additions & 16 deletions chrome/browser/media/router/media_route_id.h

This file was deleted.

13 changes: 6 additions & 7 deletions chrome/browser/media/router/media_router.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
#include "base/callback.h"
#include "chrome/browser/media/router/issue.h"
#include "chrome/browser/media/router/media_route.h"
#include "chrome/browser/media/router/media_route_id.h"
#include "chrome/browser/media/router/media_sink.h"
#include "chrome/browser/media/router/media_source.h"

Expand Down Expand Up @@ -40,29 +39,29 @@ class MediaRouter {

// Creates a media route from |source_id| to |sink_id|.
// |callback| is invoked with a response indicating success or failure.
virtual void CreateRoute(const MediaSourceId& source_id,
const MediaSinkId& sink_id,
virtual void CreateRoute(const MediaSource::Id& source_id,
const MediaSink::Id& sink_id,
const MediaRouteResponseCallback& callback) = 0;

// Closes the media route specified by |route_id|.
virtual void CloseRoute(const MediaRouteId& route_id) = 0;
virtual void CloseRoute(const MediaRoute::Id& route_id) = 0;

// Posts |message| to a MediaSink connected via MediaRoute with |route_id|.
// TODO(imcheng): Support additional data types: Blob, ArrayBuffer,
// ArrayBufferView.
virtual void PostMessage(const MediaRouteId& route_id,
virtual void PostMessage(const MediaRoute::Id& route_id,
const std::string& message) = 0;

// Clears the issue with the id |issue_id|.
virtual void ClearIssue(const Issue::IssueId& issue_id) = 0;
virtual void ClearIssue(const Issue::Id& issue_id) = 0;

// Receives updates from a MediaRouter instance.
class Delegate {
public:
// Called when there is a message from a route.
// |route_id|: The route ID.
// |message|: The message.
virtual void OnMessage(const MediaRouteId& route_id,
virtual void OnMessage(const MediaRoute::Id& route_id,
const std::string& message) = 0;
};

Expand Down
22 changes: 11 additions & 11 deletions chrome/browser/media/router/media_router_mojo_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ namespace {
// Converts the callback result of calling Mojo CreateRoute() into a local
// callback.
void CreateRouteFinished(const MediaSinkId& sink_id,
void CreateRouteFinished(const MediaSink::Id& sink_id,
const MediaRouteResponseCallback& callback,
interfaces::MediaRoutePtr media_route,
const mojo::String& error_text) {
Expand Down Expand Up @@ -158,31 +158,31 @@ void MediaRouterMojoImpl::OnRoutesUpdated(
}
void MediaRouterMojoImpl::CreateRoute(
const MediaSourceId& source_id,
const MediaSinkId& sink_id,
const MediaSource::Id& source_id,
const MediaSink::Id& sink_id,
const MediaRouteResponseCallback& callback) {
DCHECK(thread_checker_.CalledOnValidThread());
RunOrDefer(base::Bind(&MediaRouterMojoImpl::DoCreateRoute,
base::Unretained(this), source_id, sink_id, callback));
}
void MediaRouterMojoImpl::CloseRoute(const MediaRouteId& route_id) {
void MediaRouterMojoImpl::CloseRoute(const MediaRoute::Id& route_id) {
DCHECK(thread_checker_.CalledOnValidThread());
RunOrDefer(base::Bind(&MediaRouterMojoImpl::DoCloseRoute,
base::Unretained(this), route_id));
}
void MediaRouterMojoImpl::PostMessage(const MediaRouteId& route_id,
void MediaRouterMojoImpl::PostMessage(const MediaRoute::Id& route_id,
const std::string& message) {
DCHECK(thread_checker_.CalledOnValidThread());
RunOrDefer(base::Bind(&MediaRouterMojoImpl::DoPostMessage,
base::Unretained(this), route_id, message));
}
void MediaRouterMojoImpl::ClearIssue(const Issue::IssueId& issue_id) {
void MediaRouterMojoImpl::ClearIssue(const Issue::Id& issue_id) {
DCHECK(thread_checker_.CalledOnValidThread());
RunOrDefer(base::Bind(&MediaRouterMojoImpl::DoClearIssue,
Expand Down Expand Up @@ -268,26 +268,26 @@ void MediaRouterMojoImpl::RemoveIssuesObserver(IssuesObserver* observer) {
}
void MediaRouterMojoImpl::DoCreateRoute(
const MediaSourceId& source_id,
const MediaSinkId& sink_id,
const MediaSource::Id& source_id,
const MediaSink::Id& sink_id,
const MediaRouteResponseCallback& callback) {
DVLOG_WITH_INSTANCE(1) << "CreateRoute " << source_id << "=>" << sink_id;
mojo_media_router_->CreateRoute(
source_id, sink_id, base::Bind(&CreateRouteFinished, sink_id, callback));
}
void MediaRouterMojoImpl::DoCloseRoute(const MediaRouteId& route_id) {
void MediaRouterMojoImpl::DoCloseRoute(const MediaRoute::Id& route_id) {
DVLOG_WITH_INSTANCE(1) << "CloseRoute " << route_id;
mojo_media_router_->CloseRoute(route_id);
}
void MediaRouterMojoImpl::DoPostMessage(const MediaRouteId& route_id,
void MediaRouterMojoImpl::DoPostMessage(const MediaRoute::Id& route_id,
const std::string& message) {
DVLOG_WITH_INSTANCE(1) << "PostMessage " << route_id;
mojo_media_router_->PostMessage(route_id, message);
}
void MediaRouterMojoImpl::DoClearIssue(const Issue::IssueId& issue_id) {
void MediaRouterMojoImpl::DoClearIssue(const Issue::Id& issue_id) {
DVLOG_WITH_INSTANCE(1) << "ClearIssue " << issue_id;
mojo_media_router_->ClearIssue(issue_id);
}
Expand Down
23 changes: 12 additions & 11 deletions chrome/browser/media/router/media_router_mojo_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,13 @@ class MediaRouterMojoImpl : public MediaRouter,
// MediaRouter implementation.
// Execution of the requests is delegated to the Do* methods, which can be
// enqueued for later use if the extension is temporarily suspended.
void CreateRoute(const MediaSourceId& source_id,
const MediaSinkId& sink_id,
void CreateRoute(const MediaSource::Id& source_id,
const MediaSink::Id& sink_id,
const MediaRouteResponseCallback& callback) override;
void CloseRoute(const MediaRouteId& route_id) override;
void PostMessage(const MediaRouteId& route_id,
void CloseRoute(const MediaRoute::Id& route_id) override;
void PostMessage(const MediaRoute::Id& route_id,
const std::string& message) override;
void ClearIssue(const Issue::IssueId& issue_id) override;
void ClearIssue(const Issue::Id& issue_id) override;
void RegisterMediaSinksObserver(MediaSinksObserver* observer) override;
void UnregisterMediaSinksObserver(MediaSinksObserver* observer) override;
void RegisterMediaRoutesObserver(MediaRoutesObserver* observer) override;
Expand Down Expand Up @@ -107,12 +107,13 @@ class MediaRouterMojoImpl : public MediaRouter,
void ExecutePendingRequests();

// These calls invoke methods in the component extension via Mojo.
void DoCreateRoute(const MediaSourceId& source_id,
const MediaSinkId& sink_id,
void DoCreateRoute(const MediaSource::Id& source_id,
const MediaSink::Id& sink_id,
const MediaRouteResponseCallback& callback);
void DoCloseRoute(const MediaRouteId& route_id);
void DoPostMessage(const MediaRouteId& route_id, const std::string& message);
void DoClearIssue(const Issue::IssueId& issue_id);
void DoCloseRoute(const MediaRoute::Id& route_id);
void DoPostMessage(const MediaRoute::Id& route_id,
const std::string& message);
void DoClearIssue(const Issue::Id& issue_id);
void DoStartObservingMediaSinks(const std::string& source_id);
void DoStopObservingMediaSinks(const std::string& source_id);
void DoStartObservingMediaRoutes();
Expand All @@ -139,7 +140,7 @@ class MediaRouterMojoImpl : public MediaRouter,
// becomes ready.
std::vector<base::Closure> pending_requests_;

base::ScopedPtrHashMap<MediaSourceId,
base::ScopedPtrHashMap<MediaSource::Id,
scoped_ptr<base::ObserverList<MediaSinksObserver>>>
sinks_observers_;

Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/media/router/media_sink.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

namespace media_router {

MediaSink::MediaSink(const MediaSinkId& sink_id, const std::string& name)
MediaSink::MediaSink(const MediaSink::Id& sink_id, const std::string& name)
: sink_id_(sink_id), name_(name) {
}

Expand Down
13 changes: 5 additions & 8 deletions chrome/browser/media/router/media_sink.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,29 +7,26 @@

#include <string>

#include "chrome/browser/media/router/media_route_id.h"

namespace media_router {

using MediaSinkId = std::string;

// Represents a sink to which media can be routed.
class MediaSink {
public:
MediaSink(const MediaSinkId& sink_id,
const std::string& name);
using Id = std::string;

MediaSink(const MediaSink::Id& sink_id, const std::string& name);

~MediaSink();

const MediaSinkId& id() const { return sink_id_; }
const MediaSink::Id& id() const { return sink_id_; }
const std::string& name() const { return name_; }

bool Equals(const MediaSink& other) const;
bool Empty() const;

private:
// Unique identifier for the MediaSink.
MediaSinkId sink_id_;
MediaSink::Id sink_id_;
// Descriptive name of the MediaSink.
// Optional, can use an empty string if no sink name is available.
std::string name_;
Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/media/router/media_source.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,15 @@

namespace media_router {

MediaSource::MediaSource(const MediaSourceId& source_id) : id_(source_id) {
MediaSource::MediaSource(const MediaSource::Id& source_id) : id_(source_id) {
}

MediaSource::MediaSource() {
}

MediaSource::~MediaSource() {}

MediaSourceId MediaSource::id() const {
MediaSource::Id MediaSource::id() const {
return id_;
}

Expand Down
Loading

0 comments on commit 5fa0263

Please sign in to comment.