[Media Router] Implement MediaRouterAction + MediaRouterUI tests.

chrome/browser/ui/toolbar/media_router_action.cc

Index: chrome/browser/ui/toolbar/media_router_action.cc
diff --git a/chrome/browser/ui/toolbar/media_router_action.cc b/chrome/browser/ui/toolbar/media_router_action.cc
index 6895711ead3a7ea4dc58a194228b36b34c257259..58dde6d48d7925289ecea792e5e05c8ea77710f2 100644
--- a/chrome/browser/ui/toolbar/media_router_action.cc
+++ b/chrome/browser/ui/toolbar/media_router_action.cc
@@ -6,12 +6,15 @@
 
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/ui/toolbar/toolbar_action_view_delegate.h"
+#include "chrome/browser/ui/webui/media_router/media_router_dialog_controller.h"
 #include "chrome/grit/generated_resources.h"
 #include "grit/theme_resources.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/image/image_skia.h"
 
+using media_router::MediaRouterDialogController;
+
 MediaRouterAction::MediaRouterAction()
     : id_("media_router_action"),
       name_(l10n_util::GetStringUTF16(IDS_MEDIA_ROUTER_TITLE)),
@@ -63,40 +66,53 @@ bool MediaRouterAction::IsEnabled(
 
 bool MediaRouterAction::WantsToRun(
     content::WebContents* web_contents) const {
-  NOTIMPLEMENTED();

[Peter Kasting, 2015/06/04 00:17:52]

Nit: It looks like most of these functions have trivial implementations that are
likely shared with most implementers.  Consider placing these default
implementations in the base class instead to reduce boilerplate in the
subclasses.

[imcheng (use chromium acct), 2015/06/04 01:04:20]

Acknowledged. Most other implementations seem to be different. Given the limited
number of implementations currently (3, 1 of which is test), I would defer this
until it becomes more obvious that it's better to have a default implementation
in the base class.

   return false;
 }
 
 bool MediaRouterAction::HasPopup(
     content::WebContents* web_contents) const {
-  NOTIMPLEMENTED();
   return true;
 }
 
 void MediaRouterAction::HidePopup() {
-  NOTIMPLEMENTED();
+  content::WebContents* web_contents = delegate_->GetCurrentWebContents();

[Peter Kasting, 2015/06/04 00:17:52]

Nit: Why do you not DCHECK(delegate_) here, but you do in the two functions
below?  Seems like these functions should agree on the right way.

[imcheng (use chromium acct), 2015/06/04 01:04:20]

Done.

+  DCHECK(web_contents);
+  MediaRouterDialogController* controller =
+      MediaRouterDialogController::GetOrCreateForWebContents(web_contents);

[Peter Kasting, 2015/06/04 00:17:52]

Nit: You have a bunch of code duplicated in these functions.  Consider adding a
helper like "GetMediaController()" which encapsulates everything except the
final call on the controller.  Then you can just do
"GetMediaController()->CloseMediaRouterDialog();" in this function (and
similarly below).

[imcheng (use chromium acct), 2015/06/04 01:04:20]

Done.

+  controller->CloseMediaRouterDialog();
 }
 
 gfx::NativeView MediaRouterAction::GetPopupNativeView() {
-  NOTIMPLEMENTED();
   return nullptr;
 }
 
 ui::MenuModel* MediaRouterAction::GetContextMenu() {
-  NOTIMPLEMENTED();
   return nullptr;
 }
 
 bool MediaRouterAction::CanDrag() const {
-  NOTIMPLEMENTED();
   return false;
 }
 
 bool MediaRouterAction::ExecuteAction(bool by_user) {
-  NOTIMPLEMENTED();
-  return false;
+  DCHECK(delegate_);
+  content::WebContents* web_contents = delegate_->GetCurrentWebContents();
+  DCHECK(web_contents);
+  MediaRouterDialogController* controller =
+      MediaRouterDialogController::GetOrCreateForWebContents(web_contents);
+  controller->ShowMediaRouterDialog();
+  return true;
 }
 
 void MediaRouterAction::UpdateState() {
   NOTIMPLEMENTED();
 }
+
+content::WebContents* MediaRouterAction::GetMediaRouterDialog() const {
+  DCHECK(delegate_);
+  content::WebContents* web_contents = delegate_->GetCurrentWebContents();
+  DCHECK(web_contents);
+  MediaRouterDialogController* controller =
+      MediaRouterDialogController::GetOrCreateForWebContents(web_contents);
+  return controller->GetMediaRouterDialog();
+}