Revert of Media Router GYP/GN cleanup (patchset #14 id:260001 of http…

…s://codereview.chromium.org/1136623003/) Reason for revert: Broke Linux bots on Chromium waterfall, see http://build.chromium.org/p/chromium/builders/Linux/builds/62091 http://build.chromium.org/p/chromium/buildstatus?builder=Linux%20x64&number=3231 The error in gclient runhooks is: gyp: Key 'dependencies' repeated at level 3 with key path 'targets.3' while reading /b/build/slave/Linux_x64/build/src/chrome/browser/media/router/media_router.gyp Not sure why this didn't show up on other bots or trybots. Original issue's description: > Media Router GYP/GN cleanup: > > - Refactor GYP and GN files to share source lists. > - Extract a proper test_support target. > - Fix upstream files with new target structure. > > BUG=461815,464199,464205 > > Committed: https://crrev.com/9c5e5781517823c18144b0bb5b66b66a1afd6733 > Cr-Commit-Position: refs/heads/master@{#330002} TBR=brettw@chromium.org,kmarshall@chromium.org,mfoltz@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=461815,464199,464205 Review URL: https://codereview.chromium.org/1140183002 Cr-Commit-Position: refs/heads/master@{#330009}
ltilve · May 15, 2015 · 680e2cb · 680e2cb
1 parent 9ce0a55
commit 680e2cb
Show file tree

Hide file tree

Showing 15 changed files with 165 additions and 117 deletions.
diff --git a/.gn b/.gn
@@ -101,7 +101,6 @@ exec_script_whitelist = [
   "//chrome/browser/BUILD.gn",
   "//chrome/browser/chromeos/BUILD.gn",
   "//chrome/browser/extensions/BUILD.gn",
-  "//chrome/browser/media/router/BUILD.gn",
   "//chrome/browser/ui/BUILD.gn",
   "//chrome/chrome_tests.gni",
   "//chrome/common/BUILD.gn",

diff --git a/BUILD.gn b/BUILD.gn
@@ -169,7 +169,10 @@ group("both_gn_and_gyp") {
   }
 
   if (enable_media_router) {
-    deps += [ "//chrome/browser/media/router" ]
+    deps += [
+      "//chrome/browser/media/router/",
+      "//chrome/browser/media/router:unit_tests",
+    ]
   }
 
   if (enable_remoting) {

diff --git a/build/config/BUILD.gn b/build/config/BUILD.gn
@@ -248,9 +248,6 @@ config("feature_flags") {
   } else {
     defines += [ "CHROMIUM_BUILD" ]
   }
-  if (enable_media_router) {
-    defines += [ "ENABLE_MEDIA_ROUTER=1" ]
-  }
 }
 
 # Debug/release ----------------------------------------------------------------

diff --git a/chrome/browser/BUILD.gn b/chrome/browser/BUILD.gn
@@ -795,7 +795,7 @@ source_set("browser") {
     }
   }
   if (enable_media_router) {
-    deps += [ "//chrome/browser/media/router" ]
+    defines += [ "ENABLE_MEDIA_ROUTER=1" ]
   }
 }
 

diff --git a/chrome/browser/extensions/BUILD.gn b/chrome/browser/extensions/BUILD.gn
@@ -202,4 +202,7 @@ source_set("extensions") {
   if (!use_ozone) {
     sources -= [ "global_shortcut_listener_ozone.cc" ]
   }
+  if (enable_media_router) {
+    defines += [ "ENABLE_MEDIA_ROUTER=1" ]
+  }
 }
diff --git a/chrome/browser/media/router/BUILD.gn b/chrome/browser/media/router/BUILD.gn
@@ -5,42 +5,100 @@
 import("//testing/test.gni")
 import("//third_party/mojo/src/mojo/public/tools/bindings/mojom.gni")
 
-gypi_values = exec_script("//build/gypi_to_gn.py",
-                          [ rebase_path("media_router.gypi") ],
-                          "scope",
-                          [ "media_router.gypi" ])
-
-# TODO(mfoltz): Fix problem that requires explicitly listing //skia
 source_set("router") {
+  deps = [
+    ":router_core",
+  ]
+}
+
+source_set("router_core") {
   deps = [
     ":mojo_bindings",
     "//base",
-    "//extensions/browser",
     "//components/keyed_service/core",
     "//third_party/mojo/src/mojo/public/cpp/bindings",
-    "//skia",
     "//url",
   ]
-  sources = rebase_path(gypi_values.media_router_sources,
-                        ".",
-                        "//chrome/browser/media/router")
+  sources = [
+    "create_session_request.cc",
+    "create_session_request.h",
+    "issue.cc",
+    "issue.h",
+    "issue_manager.cc",
+    "issue_manager.h",
+    "issues_observer.h",
+    "media_route.cc",
+    "media_route.h",
+    "media_route_id.h",
+    "media_router.h",
+    "media_router_type_converters.cc",
+    "media_router_type_converters.h",
+    "media_routes_observer.cc",
+    "media_routes_observer.h",
+    "media_sink.cc",
+    "media_sink.h",
+    "media_sinks_observer.cc",
+    "media_sinks_observer.h",
+    "media_source.cc",
+    "media_source.h",
+    "media_source_helper.cc",
+    "media_source_helper.h",
+    "presentation_media_sinks_observer.cc",
+    "presentation_media_sinks_observer.h",
+    "route_id_manager.cc",
+    "route_id_manager.h",
+  ]
 }
 
-mojom("mojo_bindings") {
+source_set("unit_tests") {
+  testonly = true
+  deps = [
+    ":router_core",
+    ":unit_tests_support",
+    "//base/test:test_support",
+    "//chrome/test:test_support",
+    "//testing/gmock",
+    "//testing/gtest",
+    "//third_party/cld_2:cld2_static",
+    "//third_party/mojo/src/mojo/public/cpp/bindings",
+  ]
   sources = [
-    "media_router.mojom",
+    "media_route_unittest.cc",
+    "media_router_type_converters_unittest.cc",
+    "media_sink_unittest.cc",
+    "media_source_unittest.cc",
+    "presentation_media_sinks_observer_unittest.cc",
+    "route_id_manager_unittest.cc",
   ]
 }
 
-source_set("test_support") {
+source_set("unit_tests_support") {
   testonly = true
   deps = [
-    ":mojo_bindings",
-    ":router",
-    "//chrome/test:test_support",
+    ":router_core",
     "//testing/gmock",
   ]
-  sources = rebase_path(gypi_values.media_router_test_support_sources,
-                        ".",
-                        "//chrome/browser/media/router")
+  sources = [
+    "mock_media_router.cc",
+    "mock_media_router.h",
+    "mock_screen_availability_listener.cc",
+    "mock_screen_availability_listener.h",
+  ]
+}
+
+mojom("mojo_bindings") {
+  sources = [
+    "media_router.mojom",
+  ]
+}
+
+# Optional standalone test binary, for faster isolated builds.
+test("unit_tests_main") {
+  deps = [
+    ":unit_tests",
+    "//testing/gmock:gmock_main",
+  ]
+  sources = [
+    ":unittest_files",
+  ]
 }
diff --git a/chrome/browser/media/router/media_router.gyp b/chrome/browser/media/router/media_router.gyp
@@ -3,12 +3,8 @@
 # found in the LICENSE file.
 
 {
-  'includes': [
-    'media_router.gypi',
-  ],
   'targets': [
     {
-      # GN version: //chrome/browser/media/router:router
       'target_name': 'media_router',
       'type': 'static_library',
       'include_dirs': [
@@ -20,17 +16,40 @@
         ':media_router_mojo_gen',
         ':media_router_mojo',
         '<(DEPTH)/base/base.gyp:base',
-        '<(DEPTH)/components/components.gyp:keyed_service_core',
-        '<(DEPTH)/extensions/extensions.gyp:extensions_browser',
-        '<(DEPTH)/skia/skia.gyp:skia',
         '<(DEPTH)/url/url.gyp:url_lib',
       ],
       'sources': [
-        '<@(media_router_sources)',
+        'create_session_request.cc',
+        'create_session_request.h',
+        'issue.cc',
+        'issue.h',
+        'issue_manager.cc',
+        'issue_manager.h',
+        'issue_observer.h',
+        'media_route.cc',
+        'media_route.h',
+        'media_route_id.h',
+        'media_router.h',
+        'media_router_type_converters.cc',
+        'media_router_type_converters.h',
+        'media_routes_observer.cc',
+        'media_routes_observer.h',
+        'media_sink.cc',
+        'media_sink.h',
+        'media_sinks_observer.cc',
+        'media_sinks_observer.h',
+        'media_source.cc',
+        'media_source.h',
+        'media_source_helper.cc',
+        'media_source_helper.h',
+        'presentation_media_sinks_observer.cc',
+        'presentation_media_sinks_observer.h',
+        'route_id_manager.cc',
+        'route_id_manager.h',
       ],
     },
     {
-      # Mojo compiler for the Media Router internal API.
+      # Mojo bindings for the Media Router internal API.
       'target_name': 'media_router_mojo_gen',
       'type': 'none',
       'sources': [
@@ -48,32 +67,33 @@
       ],
       'dependencies': [
         'media_router_mojo_gen',
+        '<(DEPTH)/mojo/mojo_base.gyp:mojo_common_lib',
+        '<(DEPTH)/mojo/mojo_base.gyp:mojo_environment_chromium',
+        '<(DEPTH)/third_party/mojo/mojo_public.gyp:mojo_cpp_bindings',
       ],
       'sources': [
         '<(SHARED_INTERMEDIATE_DIR)/chrome/browser/media/router/media_router.mojom.cc',
         '<(SHARED_INTERMEDIATE_DIR)/chrome/browser/media/router/media_router.mojom.h',
       ],
     },
     {
-      # GN version: //chrome/browser/media/router:test_support
       'target_name': 'media_router_test_support',
       'type': 'static_library',
       'include_dirs': [
         '<(DEPTH)',
       ],
       'dependencies': [
         ':media_router',
-        '<(DEPTH)/third_party/mojo/src',
-      ],
-      'dependencies': [
-        ':media_router',
-        ':media_router_mojo',
         '<(DEPTH)/base/base.gyp:base',
         '<(DEPTH)/testing/gmock.gyp:gmock',
       ],
       'sources': [
-        '<@(media_router_test_support_sources)',
+        'mock_media_router.cc',
+        'mock_media_router.h',
+        'mock_screen_availability_listener.cc',
+        'mock_screen_availability_listener.h',
       ],
     },
+
   ],
 }
diff --git a/chrome/browser/media/router/media_router.gypi b/chrome/browser/media/router/media_router.gypi
diff --git a/chrome/browser/media/router/media_router_tests.gypi b/chrome/browser/media/router/media_router_tests.gypi
@@ -0,0 +1,24 @@
+# Copyright 2015 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.
+
+{
+  'sources': [
+    '../../ui/webui/media_router/media_cast_mode_unittest.cc',
+    '../../ui/webui/media_router/media_router_test.cc',
+    '../../ui/webui/media_router/media_router_test.h',
+    '../../ui/webui/media_router/media_router_dialog_controller_unittest.cc',
+    'issue_manager_unittest.cc',
+    'issue_unittest.cc',
+    'media_route_unittest.cc',
+    'media_router_type_converters_unittest.cc',
+    'media_sink_unittest.cc',
+    'media_source_helper_unittest.cc',
+    'media_source_unittest.cc',
+    "presentation_media_sinks_observer_unittest.cc",
+    'route_id_manager_unittest.cc',
+  ],
+  'dependencies': [
+    'browser/media/router/media_router.gyp:media_router_test_support',
+  ],
+}
diff --git a/chrome/browser/ui/BUILD.gn b/chrome/browser/ui/BUILD.gn
@@ -427,7 +427,8 @@ source_set("ui") {
     sources += rebase_path(gypi_values.chrome_browser_ui_media_router_sources,
                            ".",
                            "//chrome")
-    deps += [ "//chrome/browser/media/router" ]
+    deps += [ "//chrome/browser/media/router:router_core" ]
+    defines += [ "ENABLE_MEDIA_ROUTER=1" ]
   }
   if (enable_webrtc) {
     sources += rebase_path(gypi_values.chrome_browser_ui_webrtc_sources,

diff --git a/chrome/chrome_browser_ui.gypi b/chrome/chrome_browser_ui.gypi
@@ -3154,7 +3154,7 @@
         }],
         ['enable_media_router==1', {
           'dependencies': [
-            'browser/media/router/media_router.gyp:media_router',
+              'browser/media/router/media_router.gyp:media_router',
           ],
           'sources': [ '<@(chrome_browser_ui_media_router_sources)' ],
         }],

diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
@@ -843,7 +843,7 @@
       'browser/ui/app_list/search/webstore/webstore_provider_browsertest.cc',
       'browser/ui/app_list/speech_recognizer_browsertest.cc',
     ],
-    'chrome_browser_tests_media_router_sources': [
+    'chrome_browser_tests_media_router_webui_sources': [
       'browser/ui/webui/media_router/media_router_dialog_controller_browsertest.cc',
     ],
     # Javascript sources. These are combined with the .cc files in the GYP build
@@ -2246,8 +2246,7 @@
          ],
         }],
         ['enable_media_router==1', {
-          'sources': [ '<@(chrome_browser_tests_media_router_sources)' ],
-          'dependencies': [ 'browser/media/router/media_router.gyp:media_router_test_support' ],
+          'sources': [ '<@(chrome_browser_tests_media_router_webui_sources)' ],
         }],
         ['OS=="win"', {
           'sources': [
-Original file line number
+Diff line change
@@ Expand Up / @@ -795,7 +795,7 @@ source_set("browser") { @@
         }
       }
       if (enable_media_router) {
-        deps += [ "//chrome/browser/media/router" ]
+        defines += [ "ENABLE_MEDIA_ROUTER=1" ]
       }
     }
@@ Expand Down @@