Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(9)

Issue 5936043: Speculative fix for mini_installer size increase. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years, 1 month ago by thakis
Modified:
12 years, 1 month ago
CC:
rlz-codereviews_googlegroups.com, Finnur
Base URL:
https://rlz.googlecode.com/svn/trunk
Visibility:
Public.

Description

Speculative fix for mini_installer size increase. At least on Mac and Linux, the linker loads object files from static libraries by loading a .o file that contains symbols it's missing, then adds symbols needed by that .o file to the missing symbol list, and then it repeats that process. Since uninstall.cc uses SupplementaryBranding and calls ClearProductState, that causes rlz_lib.o to be loaded. That references symbols in financial_ping.cc, which references net. In the hope of msvc's linker working the same way, move the symbols used by uninstall.cc into its own file that doesn't depend on symbols in financial_ping. That way, the linker hopefully won't pull in net and content when linking mini_installer. BUG=chromium:120427 TEST=none

Patch Set 1 #

Patch Set 2 : . #

Patch Set 3 : . #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+83 lines, -65 lines) Patch
M lib/rlz_lib.cc View 1 2 3 chunks +0 lines, -65 lines 0 comments Download
A lib/rlz_lib_clear.cc View 1 1 chunk +82 lines, -0 lines 2 comments Download
M rlz.gyp View 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 2
thakis
Not sure if this will work, but it's worth a try. https://codereview.appspot.com/5936043/diff/2001/lib/rlz_lib_clear.cc File lib/rlz_lib_clear.cc (right): ...
12 years, 1 month ago (2012-03-27 17:14:02 UTC) #1
Roger Tawa (Google)
12 years, 1 month ago (2012-03-27 17:35:57 UTC) #2
lgtm

Another solution to this problem would be to not use the chrome stack for
windows builds of chrome.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b