Rework GN sessions component
The main problem with the sessions build was that the test_support source set depended on the core source set, which had symbols set up to be exported. This caused duplicate symbols when both test_support and sessions_content were linked into the same target (like Chrome unit_tests does).
This removes the TODOs about the list of sources being duplicated. This actually confused me and set me back anbout an hour on this patch. The only thing is there are different targets for ios and non-iOS, and these files need to be shared. I don't think this is a problem or weird. The comment implied to me there were multiple static libraries in the same build sharing those sources, which is not the case.
I renamed the GN targets and did some cleanup to be consistent. There's no need for separate naming under iOS and non-iOS, for example.
Removes the SESSIONS_IMPLEMENTATION define for the test support target. This is not a component and should instead import those symbols from the component.
Implements a skeleton versions of sessions_ios.
The content/test change is unrelated but the bot threw an error (not sure why this doesn't always happen). But the gpu_blink dependency is definitely used, so adding it should be correct.
Adds a missing dependency on the win_window target. This is in the GYP build but missing in GN. I also renamed this target to use the typical naming convention while I was there.
This adds an export on the SessionsBackend class. This is depended on by the sessions test support target. I speculate the GYP build doesn't run into this because the sessions test support is a static library and the object file that references this symbol is actually unused, so it is stripped. In GN it's a source set so won't get this stripping.
CQ_EXTRA_TRYBOTS=tryserver.chromium.linux:android_chromium_gn_compile_dbg,android_chromium_gn_compile_rel;tryserver.chromium.win:win8_chromium_gn_rel,win8_chromium_gn_dbg;tryserver.chromium.mac:mac_chromium_gn_rel,mac_chromium_gn_dbg
Review URL: https://codereview.chromium.org/1114543005
Cr-Commit-Position: refs/heads/master@{#327778}
diff --git a/ash/BUILD.gn b/ash/BUILD.gn
index 0eae612..f41e4b43 100644
--- a/ash/BUILD.gn
+++ b/ash/BUILD.gn
@@ -202,6 +202,7 @@
deps += [
"//ipc",
"//ui/metro_viewer",
+ "//ui/platform_window/win",
"//win8:metro_viewer",
"//win8:test_support_win8",
"//win8:test_registrar",
diff --git a/chrome/browser/BUILD.gn b/chrome/browser/BUILD.gn
index a353e22..9bb4270 100644
--- a/chrome/browser/BUILD.gn
+++ b/chrome/browser/BUILD.gn
@@ -253,7 +253,7 @@
"//components/password_manager/content/browser",
"//components/plugins/common",
"//components/precache/content",
- "//components/sessions:sessions_content",
+ "//components/sessions",
"//components/storage_monitor",
"//components/translate/content/browser",
"//components/url_matcher",
diff --git a/components/BUILD.gn b/components/BUILD.gn
index 005928c..f52f6202 100644
--- a/components/BUILD.gn
+++ b/components/BUILD.gn
@@ -87,7 +87,7 @@
"//components/search",
"//components/search_engines",
"//components/search_provider_logos",
- "//components/sessions:sessions_content",
+ "//components/sessions",
"//components/signin/core/browser",
"//components/startup_metric_utils",
"//components/strings",
@@ -200,7 +200,7 @@
"//components/renderer_context_menu", # Blocked on content.
"//components/search_engines", # Should work, needs checking.
"//components/search_provider_logos", # Should work, needs checking.
- "//components/sessions:sessions_content", # Blocked on content.
+ "//components/sessions", # Blocked on content.
"//components/signin/core/browser", # Should work, needs checking.
"//components/translate/content/browser", # Blocked on content.
"//components/translate/content/common", # Blocked on content.
diff --git a/components/native_viewport/BUILD.gn b/components/native_viewport/BUILD.gn
index 71ad605..5b27f1d6 100644
--- a/components/native_viewport/BUILD.gn
+++ b/components/native_viewport/BUILD.gn
@@ -105,6 +105,6 @@
}
if (is_win) {
- deps += [ "//ui/platform_window/win:win_window" ]
+ deps += [ "//ui/platform_window/win" ]
}
}
diff --git a/components/sessions.gypi b/components/sessions.gypi
index 25dd69a..b6e6616 100644
--- a/components/sessions.gypi
+++ b/components/sessions.gypi
@@ -4,15 +4,9 @@
{
'variables': {
- # Core sources shared by sessions_content and sessions_ios.
- #
- # TODO(rohitrao): We are including these sources directly into each
- # individual target in order to avoid the complications associated with
- # making a separate sessions_core target. The files in sessions/core
- # declare a static function that they do not define, which means that a
- # sessions_core target would not link as a shared_library. It would also be
- # unsuitable as a static_library because it would be linked into multiple
- # shared libraries. Revisit this setup if necessary.
+ # Core sources shared by sessions_content and sessions_ios. These can't
+ # be a separate shared library since one symbol is implemented higher up in
+ # the sessions_content/ios layer.
'sessions_core_sources': [
'sessions/base_session_service.cc',
'sessions/base_session_service.h',
@@ -39,7 +33,6 @@
# GN version: //components/sessions:test_support
'target_name': 'sessions_test_support',
'type': 'static_library',
- 'defines!': ['SESSIONS_IMPLEMENTATION'],
'dependencies': [
'../skia/skia.gyp:skia',
'../sync/sync.gyp:sync',
@@ -55,12 +48,10 @@
],
'conditions': [
['OS!="ios" and OS!="android"', {
- 'sources': [
- 'sessions/base_session_service_test_helper.cc',
- 'sessions/base_session_service_test_helper.h',
- 'sessions/session_backend.cc',
- 'sessions/session_backend.h',
- ]
+ 'sources': [
+ 'sessions/base_session_service_test_helper.cc',
+ 'sessions/base_session_service_test_helper.h',
+ ],
}],
],
},
@@ -71,7 +62,7 @@
['OS!="ios"', {
'targets': [
{
- # GN version: //components/sessions:sessions_content
+ # GN version: //components/sessions
'target_name': 'sessions_content',
'type': '<(component)',
'dependencies': [
diff --git a/components/sessions/BUILD.gn b/components/sessions/BUILD.gn
index 6d8c4a6..bcc7366 100644
--- a/components/sessions/BUILD.gn
+++ b/components/sessions/BUILD.gn
@@ -6,11 +6,53 @@
import("//build/config/android/config.gni")
}
-# TODO(rohitrao): sessions_core is defined as a source_set because it declares a
-# static function that it does not define. This prevents it from linking as a
-# shared_library. It also cannot be a static_library because it will be linked
-# into multiple shared libraries. Revisit this setup if necessary.
-source_set("sessions_core") {
+config("implementation") {
+ defines = [ "SESSIONS_IMPLEMENTATION" ]
+}
+
+if (!is_ios) {
+ # GYP version: components/sessions.gypi:sessions_content
+ component("sessions") {
+ sources = [
+ "content/content_serialized_navigation_builder.cc",
+ "content/content_serialized_navigation_builder.h",
+ "content/content_serialized_navigation_driver.cc",
+ "content/content_serialized_navigation_driver.h",
+ ]
+
+ configs += [ ":implementation" ]
+
+ deps = [
+ ":shared",
+ "//base",
+ "//base/third_party/dynamic_annotations",
+ "//content/public/browser",
+ "//ui/base",
+ "//url",
+ ]
+ }
+} else {
+ source_set("sessions") {
+ sources = [
+ "ios/ios_serialized_navigation_builder.cc",
+ "ios/ios_serialized_navigation_builder.h",
+ "ios/ios_serialized_navigation_driver.cc",
+ "ios/ios_serialized_navigation_driver.h",
+ ]
+
+ deps = [
+ ":shared",
+ "//base",
+
+ # '../ios/web/ios_web.gyp:ios_web', TODO(GYP) iOS.
+ ]
+ }
+}
+
+# Sources shared between the content and iOS implementations.
+source_set("shared") {
+ visibility = [ ":*" ]
+
sources = [
"base_session_service.cc",
"base_session_service.h",
@@ -32,7 +74,7 @@
"session_types.h",
]
- defines = [ "SESSIONS_IMPLEMENTATION" ]
+ configs += [ ":implementation" ]
deps = [
"//base",
@@ -53,8 +95,10 @@
"serialized_navigation_entry_test_helper.h",
]
+ public_deps = [
+ ":sessions",
+ ]
deps = [
- ":sessions_core",
"//skia",
"//sync",
"//testing/gtest",
@@ -68,28 +112,6 @@
}
}
-if (!is_ios) {
- component("sessions_content") {
- sources = [
- "content/content_serialized_navigation_builder.cc",
- "content/content_serialized_navigation_builder.h",
- "content/content_serialized_navigation_driver.cc",
- "content/content_serialized_navigation_driver.h",
- ]
-
- defines = [ "SESSIONS_IMPLEMENTATION" ]
-
- deps = [
- ":sessions_core",
- "//base",
- "//base/third_party/dynamic_annotations",
- "//content/public/browser",
- "//ui/base",
- "//url",
- ]
- }
-}
-
if (!is_ios && !is_android) {
source_set("unit_tests") {
testonly = true
@@ -98,7 +120,7 @@
"session_types_unittest.cc",
]
deps = [
- ":sessions_content",
+ ":sessions",
"//base/test:test_support",
"//testing/gtest",
"//third_party/protobuf:protobuf_lite",
diff --git a/components/sessions/session_backend.h b/components/sessions/session_backend.h
index 138fc584..8679793ca 100644
--- a/components/sessions/session_backend.h
+++ b/components/sessions/session_backend.h
@@ -12,6 +12,7 @@
#include "base/task/cancelable_task_tracker.h"
#include "components/sessions/base_session_service.h"
#include "components/sessions/session_command.h"
+#include "components/sessions/sessions_export.h"
namespace base {
class File;
@@ -31,7 +32,8 @@
// BaseSessionService. A command consists of a unique id and a stream of bytes.
// SessionBackend does not use the id in anyway, that is used by
// BaseSessionService.
-class SessionBackend : public base::RefCountedThreadSafe<SessionBackend> {
+class SESSIONS_EXPORT SessionBackend
+ : public base::RefCountedThreadSafe<SessionBackend> {
public:
typedef sessions::SessionCommand::id_type id_type;
typedef sessions::SessionCommand::size_type size_type;
diff --git a/content/test/BUILD.gn b/content/test/BUILD.gn
index 593533c..806052e 100644
--- a/content/test/BUILD.gn
+++ b/content/test/BUILD.gn
@@ -707,6 +707,7 @@
"//base/allocator",
"//base/test:test_support",
"//content/public/common",
+ "//gpu/blink",
"//testing/gtest",
"//third_party/WebKit/public:blink",
"//ui/base",
diff --git a/extensions/browser/BUILD.gn b/extensions/browser/BUILD.gn
index 6a643160..5f98946f 100644
--- a/extensions/browser/BUILD.gn
+++ b/extensions/browser/BUILD.gn
@@ -14,7 +14,7 @@
"//components/keyed_service/content",
"//components/keyed_service/core",
"//components/pref_registry",
- "//components/sessions:sessions_content",
+ "//components/sessions",
"//components/ui/zoom:ui_zoom",
"//components/web_cache/browser",
"//components/web_modal",
diff --git a/ui/aura/BUILD.gn b/ui/aura/BUILD.gn
index abe3e2e..2acb7e6 100644
--- a/ui/aura/BUILD.gn
+++ b/ui/aura/BUILD.gn
@@ -133,7 +133,7 @@
deps += [
"//ipc",
"//ui/metro_viewer",
- "//ui/platform_window/win:win_window",
+ "//ui/platform_window/win",
]
}
diff --git a/ui/platform_window/win/BUILD.gn b/ui/platform_window/win/BUILD.gn
index 26eb0aaa..e7226e1 100644
--- a/ui/platform_window/win/BUILD.gn
+++ b/ui/platform_window/win/BUILD.gn
@@ -2,7 +2,9 @@
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
-component("win_window") {
+component("win") {
+ output_name = "win_window"
+
deps = [
"//base",
"//skia",