Show Media Router toolbar icon ephemerally for MR dialogs

chrome/browser/ui/webui/media_router/media_router_web_ui_test.h

+// Copyright 2015 The Chromium Authors. All rights reserved.

[mark a. foltz, 2016/11/08 21:15:53]

Update copyright.

[takumif, 2016/11/09 04:37:27]

Done.

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_UI_WEBUI_MEDIA_ROUTER_MEDIA_ROUTER_WEB_UI_TEST_H_
+#define CHROME_BROWSER_UI_WEBUI_MEDIA_ROUTER_MEDIA_ROUTER_WEB_UI_TEST_H_
+
+#include "base/macros.h"
+#include "chrome/browser/media/router/media_router_ui_service.h"
+#include "chrome/test/base/browser_with_test_window_test.h"
+#include "testing/gmock/include/gmock/gmock.h"
+
+class BrowserWindow;

[msw, 2016/11/04 00:07:17]

nit: probably not needed; should be included from
browser_with_test_window_test.h; ditto for TestingProfile

[takumif, 2016/11/09 04:37:27]

Removed.

+class TestingProfile;
+
+class MockMediaRouterActionController : public MediaRouterActionController {

[msw, 2016/11/04 00:07:16]

nit: move these other classes to chrome/browser/media/router/test_helper.*

[takumif, 2016/11/09 04:37:27]

These classes were there originally, but were moved here because they are
desktop-UI-specific.

[msw, 2016/11/09 20:32:34]

Move them into the cc test file or to separate files, as needed.

[takumif, 2016/11/10 01:53:59]

Moving MockMediaRouterUIService into the .cc file and moving
MockMediaRouterActionController into its own file.

+ public:
+  explicit MockMediaRouterActionController(Profile* profile);
+  ~MockMediaRouterActionController() override;
+
+  MOCK_METHOD1(OnIssueUpdated, void(const media_router::Issue* issue));
+  MOCK_METHOD2(OnRoutesUpdated,
+               void(const std::vector<media_router::MediaRoute>& routes,
+                    const std::vector<media_router::MediaRoute::Id>&
+                        joinable_route_ids));
+  MOCK_METHOD0(OnDialogShown, void());
+  MOCK_METHOD0(OnDialogHidden, void());
+  MOCK_METHOD0(MaybeAddOrRemoveAction, void());
+};
+
+class MockMediaRouterUIService : public media_router::MediaRouterUIService {
+ public:
+  explicit MockMediaRouterUIService(Profile* profile);
+  ~MockMediaRouterUIService() override;
+
+  MediaRouterActionController* action_controller() const override;
+
+ private:
+  std::unique_ptr<MediaRouterActionController> action_controller_;

[msw, 2016/11/04 00:07:17]

nit: avoid unique_ptr and use a plain MockMediaRouterActionController member

[takumif, 2016/11/09 04:37:27]

That would make the action_controller() getter return &action_controller_, which
is const and wouldn't work.

[msw, 2016/11/09 20:32:34]

Ditto to my earlier question about the correctness of a const accessor returning
a mutable member... this doesn't seem right.

[takumif, 2016/11/10 01:53:59]

Fixed.

+};
+
+class MediaRouterWebUITest : public BrowserWithTestWindowTest {
+ public:
+  // |require_mock_ui_service_| defaults to false in the default ctor.

[msw, 2016/11/04 00:07:17]

optional nit: remove default ctor; add default value for require_mock_ui_service
= false below.

[takumif, 2016/11/09 04:37:27]

I feel like a default ctor is cleaner than always having to pass in a flag.

[msw, 2016/11/09 20:32:34]

A default value, as I suggested, would avoid needing to always pass a flag.

+  MediaRouterWebUITest();
+  explicit MediaRouterWebUITest(bool require_mock_ui_service);
+  ~MediaRouterWebUITest() override;
+
+  // BrowserWithTestWindowTest:

[msw, 2016/11/04 00:07:17]

optional nit: make both overrides protected or public

[takumif, 2016/11/09 04:37:27]

Made them both protected.

+  TestingProfile* CreateProfile() override;
+
+ protected:
+  // BrowserWithTestWindowTest:
+  BrowserWindow* CreateBrowserWindow() override;
+
+ private:
+  // When this is set to true, MockMediaRouterUIService is instantiated.
+  // Otherwise, no MediaRouterUIService is instantiated.
+  bool require_mock_ui_service_;
+
+  DISALLOW_COPY_AND_ASSIGN(MediaRouterWebUITest);
+};
+
+#endif  // CHROME_BROWSER_UI_WEBUI_MEDIA_ROUTER_MEDIA_ROUTER_WEB_UI_TEST_H_