Skip to content

Commit

Permalink
reland of http://crrev.com/212230 Create top-level separate targets f…
Browse files Browse the repository at this point in the history
…or...

Original CL here: https://codereview.chromium.org/17619005/

Changed since previous landing is diff between ps1 and ps2.
Diff since previous landing is a bit noisy, but in those files
against original is relatively small. The conditions for the
defines were incorrect and are simpler (and correct) now.

Previously:

Create top-level separate targets for browser and child dlls

The general idea is that there's top level targets chrome and chrome_child,
and corresponding content_app and content_app_child that depend on only
the subtargets that should be included in the appropriate dll.

Currently (probably) Windows-only and requires setting chrome_multiple_dll=1
for gyp.

Links, but Blink is still included in browser.

Single-process mode is currently disabled when chrome_multiple_dll is set.

Current graph is at:
http://commondatastorage.googleapis.com/chromelinkgraph/deps.html generated by
"python tools\win\split_link\graph_dependencies.py deps.html"

Remove the previous hacky-er attempt at this that was named "split dll".

TBR=jam@chromium.org
BUG=237249, 256965

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@212415 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
scottmg@chromium.org committed Jul 18, 2013
1 parent 841ce9e commit 1ff6429
Show file tree
Hide file tree
Showing 23 changed files with 144 additions and 142 deletions.
12 changes: 0 additions & 12 deletions base/debug/trace_event.h
Original file line number Diff line number Diff line change
Expand Up @@ -788,17 +788,13 @@
// Defines visibility for classes in trace_event.h
#define TRACE_EVENT_API_CLASS_EXPORT BASE_EXPORT

// Not supported in split-dll build. http://crbug.com/256965
#if !defined(CHROME_SPLIT_DLL)
// The thread buckets for the sampling profiler.
TRACE_EVENT_API_CLASS_EXPORT extern \
TRACE_EVENT_API_ATOMIC_WORD g_trace_state[3];

#define TRACE_EVENT_API_THREAD_BUCKET(thread_bucket) \
g_trace_state[thread_bucket]

#endif

////////////////////////////////////////////////////////////////////////////////

// Implementation detail: trace event macros create temporary variables
Expand Down Expand Up @@ -1489,23 +1485,15 @@ class TraceEventSamplingStateScope {
}

static inline const char* Current() {
// Not supported in split-dll build. http://crbug.com/256965
#if !defined(CHROME_SPLIT_DLL)
return reinterpret_cast<const char*>(TRACE_EVENT_API_ATOMIC_LOAD(
g_trace_state[BucketNumber]));
#else
return NULL;
#endif
}

static inline void Set(const char* category_and_name) {
// Not supported in split-dll build. http://crbug.com/256965
#if !defined(CHROME_SPLIT_DLL)
TRACE_EVENT_API_ATOMIC_STORE(
g_trace_state[BucketNumber],
reinterpret_cast<TRACE_EVENT_API_ATOMIC_WORD>(
const_cast<char*>(category_and_name)));
#endif
}

private:
Expand Down
6 changes: 0 additions & 6 deletions base/debug/trace_event_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,8 @@ class DeleteTraceLogForTesting {
}
};

// Not supported in split-dll build. http://crbug.com/237249
#if !defined(CHROME_SPLIT_DLL)
// The thread buckets for the sampling profiler.
BASE_EXPORT TRACE_EVENT_API_ATOMIC_WORD g_trace_state[3];
#endif

namespace base {
namespace debug {
Expand Down Expand Up @@ -981,8 +978,6 @@ void TraceLog::SetEnabled(const CategoryFilter& category_filter,
category_filter_ = CategoryFilter(category_filter);
EnableIncludedCategoryGroups();

// Not supported in split-dll build. http://crbug.com/237249
#if !defined(CHROME_SPLIT_DLL)
if (options & ENABLE_SAMPLING) {
sampling_thread_.reset(new TraceSamplingThread);
sampling_thread_->RegisterSampleBucket(
Expand All @@ -1002,7 +997,6 @@ void TraceLog::SetEnabled(const CategoryFilter& category_filter,
DCHECK(false) << "failed to create thread";
}
}
#endif

dispatching_to_observer_list_ = true;
observer_list = enabled_state_observer_list_;
Expand Down
3 changes: 0 additions & 3 deletions base/debug/trace_event_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1677,8 +1677,6 @@ TEST_F(TraceEventTestFixture, TraceOptionsParsing) {
"record-continuously,enable-sampling"));
}

// Not supported in split dll build. http://crbug.com/256965
#if !defined(CHROME_SPLIT_DLL)
TEST_F(TraceEventTestFixture, TraceSampling) {
event_watch_notification_ = 0;
TraceLog::GetInstance()->SetEnabled(
Expand Down Expand Up @@ -1738,7 +1736,6 @@ TEST_F(TraceEventTestFixture, TraceSamplingScope) {

EndTraceAndFlush();
}
#endif // !CHROME_SPLIT_DLL

class MyData : public base::debug::ConvertableToTraceFormat {
public:
Expand Down
13 changes: 5 additions & 8 deletions build/common.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -938,9 +938,9 @@
# to get incremental linking to be faster in debug builds.
'incremental_chrome_dll%': '0',

# Experimental setting to break chrome.dll in to multiple parts (currently
# two, split primarily along browser/render lines).
'chrome_split_dll%': '0',
# Experimental setting to break chrome.dll into multiple pieces based on
# process type.
'chrome_multiple_dll%': '0',

# The default settings for third party code for treating
# warnings-as-errors. Ideally, this would not be required, however there
Expand Down Expand Up @@ -1868,11 +1868,8 @@
'<(DEPTH)/base/allocator/allocator.gyp:type_profiler',
],
}],
['chrome_split_dll', {
'variables': {
'chrome_split_dll': '<!(python <(DEPTH)/tools/win/split_link/check_installed.py)',
},
'defines': ['CHROME_SPLIT_DLL'],
['chrome_multiple_dll', {
'defines': ['CHROME_MULTIPLE_DLL'],
}],
['OS=="linux" and clang==1 and host_arch=="ia32"', {
# TODO(dmikurube): Remove -Wno-sentinel when Clang/LLVM is fixed.
Expand Down
39 changes: 30 additions & 9 deletions chrome/app/chrome_main_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -93,14 +93,19 @@
#include "chrome/app/breakpad_linux.h"
#endif

#if !defined(CHROME_MULTIPLE_DLL) || defined(CHROME_MULTIPLE_DLL_BROWSER)
base::LazyInstance<chrome::ChromeContentBrowserClient>
g_chrome_content_browser_client = LAZY_INSTANCE_INITIALIZER;
#endif

#if !defined(CHROME_MULTIPLE_DLL) || defined(CHROME_MULTIPLE_DLL_CHILD)
base::LazyInstance<chrome::ChromeContentRendererClient>
g_chrome_content_renderer_client = LAZY_INSTANCE_INITIALIZER;
base::LazyInstance<chrome::ChromeContentUtilityClient>
g_chrome_content_utility_client = LAZY_INSTANCE_INITIALIZER;
base::LazyInstance<chrome::ChromeContentPluginClient>
g_chrome_content_plugin_client = LAZY_INSTANCE_INITIALIZER;
#endif // !CHROME_MULTIPLE_DLL || CHROME_MULTIPLE_DLL_CHILD

#if defined(OS_POSIX)
base::LazyInstance<chrome::ChromeBreakpadClient>::Leaky
Expand Down Expand Up @@ -388,7 +393,7 @@ bool ChromeMainDelegate::BasicStartupComplete(int* exit_code) {
// not though: it still uses string16. As there is no easily accessible command
// line on Android, I'm not sure this is a big deal, at least for purposes of
// troubleshooting with a customer.
#if !defined(OS_ANDROID)
#if !defined(OS_ANDROID) && !defined(CHROME_MULTIPLE_DLL_CHILD)
// If we are in diagnostics mode this is the end of the line: after the
// diagnostics are run the process will invariably exit.
if (command_line.HasSwitch(switches::kDiagnostics)) {
Expand Down Expand Up @@ -673,15 +678,17 @@ int ChromeMainDelegate::RunProcess(
const content::MainFunctionParams& main_function_params) {
// ANDROID doesn't support "service", so no ServiceProcessMain, and arraysize
// doesn't support empty array. So we comment out the block for Android.
#if !defined(OS_ANDROID)
#if !defined(OS_ANDROID) && \
(!defined(CHROME_MULTIPLE_DLL) || defined(CHROME_MULTIPLE_DLL_BROWSER))
static const MainFunction kMainFunctions[] = {
{ switches::kServiceProcess, ServiceProcessMain },
#if defined(OS_MACOSX)
{ switches::kRelauncherProcess,
mac_relauncher::internal::RelauncherMain },
#endif
// TODO(scottmg): http://crbug.com/237249 NaCl -> child.
#if !defined(DISABLE_NACL)

#if !defined(DISABLE_NACL) && \
(!defined(CHROME_MULTIPLE_DLL) || defined(CHROME_MULTIPLE_DLL_CHILD))
{ switches::kNaClLoaderProcess, NaClMain },
#endif // DISABLE_NACL
};
Expand Down Expand Up @@ -750,22 +757,36 @@ void ChromeMainDelegate::ZygoteForked() {
#endif // OS_MACOSX

content::ContentBrowserClient*
ChromeMainDelegate::CreateContentBrowserClient() {
ChromeMainDelegate::CreateContentBrowserClient() {
#if defined(CHROME_MULTIPLE_DLL_CHILD)
return NULL;
#else
return &g_chrome_content_browser_client.Get();
#endif
}

content::ContentPluginClient* ChromeMainDelegate::CreateContentPluginClient() {
// TODO(scottmg): http://crbug.com/237249 This will have to be split out into
// browser and child parts.
#if defined(CHROME_MULTIPLE_DLL_BROWSER)
return NULL;
#else
return &g_chrome_content_plugin_client.Get();
#endif
}

content::ContentRendererClient*
ChromeMainDelegate::CreateContentRendererClient() {
ChromeMainDelegate::CreateContentRendererClient() {
#if defined(CHROME_MULTIPLE_DLL_BROWSER)
return NULL;
#else
return &g_chrome_content_renderer_client.Get();
#endif
}

content::ContentUtilityClient*
ChromeMainDelegate::CreateContentUtilityClient() {
ChromeMainDelegate::CreateContentUtilityClient() {
#if defined(CHROME_MULTIPLE_DLL_BROWSER)
return NULL;
#else
return &g_chrome_content_utility_client.Get();
#endif
}
20 changes: 0 additions & 20 deletions chrome/app/client_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -74,18 +74,6 @@ void ClearDidRun(const string16& dll_path) {
GoogleUpdateSettings::UpdateDidRunState(false, system_level);
}

#if defined(CHROME_SPLIT_DLL)
// Deferred initialization entry point for chrome1.dll.
typedef BOOL (__stdcall *DoDeferredCrtInitFunc)(HINSTANCE hinstance);

bool InitSplitChromeDll(HMODULE mod) {
if (!mod)
return false;
DoDeferredCrtInitFunc init = reinterpret_cast<DoDeferredCrtInitFunc>(
::GetProcAddress(mod, "_DoDeferredCrtInit@4"));
return (init(mod) == TRUE);
}
#endif
} // namespace

string16 GetExecutablePath() {
Expand Down Expand Up @@ -166,14 +154,6 @@ HMODULE MainDllLoader::Load(string16* out_version, string16* out_file) {

DCHECK(dll);

#if defined(CHROME_SPLIT_DLL)
// In split dlls mode, we need to manually initialize both DLLs because
// the circular dependencies between them make the loader not call the
// Dllmain for DLL_PROCESS_ATTACH.
InitSplitChromeDll(dll);
InitSplitChromeDll(::GetModuleHandleA("chrome1.dll"));
#endif

return dll;
}

Expand Down
5 changes: 1 addition & 4 deletions chrome/chrome.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
],
'chromium_child_dependencies': [
'common',
'../content/content.gyp:content_app',
'../sync/sync.gyp:sync',
],
'allocator_target': '../base/allocator/allocator.gyp:allocator',
Expand All @@ -30,9 +29,8 @@
'conditions': [
['OS!="ios"', {
'chromium_browser_dependencies': [
'debugger',
'../content/content.gyp:content_ppapi_plugin',
'../printing/printing.gyp:printing',
'../ppapi/ppapi_internal.gyp:ppapi_host',
],
'chromium_child_dependencies': [
'debugger',
Expand All @@ -42,7 +40,6 @@
'../content/content.gyp:content_gpu',
'../content/content.gyp:content_ppapi_plugin',
'../content/content.gyp:content_worker',
'../printing/printing.gyp:printing',
'../third_party/WebKit/Source/devtools/devtools.gyp:devtools_frontend_resources',
],
}],
Expand Down
1 change: 1 addition & 0 deletions chrome/chrome_common.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,7 @@
'common/extensions/api/extension_api_stub.cc',
],
'dependencies': [
'../device/bluetooth/bluetooth.gyp:device_bluetooth',
'../device/usb/usb.gyp:device_usb',
],
}, { # enable_extensions == 0
Expand Down
57 changes: 43 additions & 14 deletions chrome/chrome_dll.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@
],
},
}], # OS=="mac"
['chrome_multiple_dll==1', {
'dependencies': [
'chrome_child_dll',
],
}],
['incremental_chrome_dll==1', {
# Linking to a different directory and then hardlinking back
# to OutDir is a workaround to avoid having the .ilk for
Expand Down Expand Up @@ -75,8 +80,6 @@
},
'dependencies': [
'<@(chromium_browser_dependencies)',
'<@(chromium_child_dependencies)',
'../content/content.gyp:content_worker',
'app/policy/cloud_policy_codegen.gyp:policy',
],
'conditions': [
Expand Down Expand Up @@ -221,24 +224,24 @@
'AdditionalManifestFiles': '$(ProjectDir)\\app\\chrome.dll.manifest',
},
},
}], # OS=="win"
}],
['chrome_multiple_dll==1', {
'defines': [
'CHROME_MULTIPLE_DLL_BROWSER',
],
}, {
'dependencies': [
'<@(chromium_child_dependencies)',
'../content/content.gyp:content_app_child',
'../content/content.gyp:content_worker',
],
}],
['OS=="mac" and component!="shared_library"', {
'includes': [ 'chrome_dll_bundle.gypi' ],
}],
['OS=="mac" and component=="shared_library"', {
'xcode_settings': { 'OTHER_LDFLAGS': [ '-Wl,-ObjC' ], },
}],
['chrome_split_dll', {
'sources': [
# See comment in .cc for explanation.
'split_dll_fake_entry.cc',
],
'msvs_settings': {
'VCLinkerTool': {
'AdditionalOptions': ['/splitlink'],
},
}
}],
['OS=="mac"', {
'xcode_settings': {
# Define the order of symbols within the framework. This
Expand Down Expand Up @@ -330,5 +333,31 @@
},
],
}],
['chrome_multiple_dll', {
'targets': [
{
'target_name': 'chrome_child_dll',
'type': 'shared_library',
'product_name': 'chrome_child',
'variables': {
'enable_wexit_time_destructors': 1,
},
'dependencies': [
'<@(chromium_child_dependencies)',
'../content/content.gyp:content_app_child',
'../content/content.gyp:content_worker',
'policy_path_parser',
],
'defines': [
'CHROME_MULTIPLE_DLL_CHILD',
],
'sources': [
'app/chrome_main.cc',
'app/chrome_main_delegate.cc',
'app/chrome_main_delegate.h',
],
}, # target chrome_child_dll
],
}],
],
}
2 changes: 1 addition & 1 deletion chrome/chrome_syzygy.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
# found in the LICENSE file.
{
'conditions': [
['OS=="win" and fastbuild==0 and chrome_split_dll==0', {
['OS=="win" and fastbuild==0 and chrome_multiple_dll==0', {
# Reorder or instrument the initial chrome DLL executable, placing the
# optimized output and corresponding PDB file into the "syzygy"
# subdirectory.
Expand Down
2 changes: 1 addition & 1 deletion chrome/installer/mini_installer_syzygy.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
],
'conditions': [
# This target won't build in fastbuild, since there are no PDBs.
['OS=="win" and fastbuild==0 and chrome_split_dll==0', {
['OS=="win" and fastbuild==0 and chrome_multiple_dll==0', {
'targets': [
{
'target_name': 'mini_installer_syzygy',
Expand Down
Loading

0 comments on commit 1ff6429

Please sign in to comment.