Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(217)

Issue 451283002: Drop the bool parameter from InitSharedInstanceWithPakFileRegion(). (Closed)

Created:
6 years, 4 months ago by tfarina
Modified:
6 years, 4 months ago
CC:
chromium-reviews, darin-cc_chromium.org, jam, android-webview-reviews_chromium.org, jochen+watch_chromium.org, oshima
Project:
chromium
Visibility:
Public.

Description

Drop the bool parameter from InitSharedInstanceWithPakFileRegion(). We always pass false to it, so we will never load the common resources. The benefit of doing this is that it allow us to remove a call to LoadCommonResources(), which we are trying to remove. BUG=176960 TEST=None R=tony@chromium.org TBR=ben@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=288978

Patch Set 1 : #

Patch Set 2 : REBASE #

Unified diffs Side-by-side diffs Delta from patch set Stats (+6 lines, -18 lines) Patch
M android_webview/browser/aw_browser_main_parts.cc View 1 chunk +1 line, -3 lines 0 comments Download
M chrome/app/chrome_main_delegate.cc View 1 1 chunk +1 line, -3 lines 0 comments Download
M content/shell/app/shell_main_delegate.cc View 1 chunk +1 line, -1 line 0 comments Download
M ui/base/resource/resource_bundle.h View 1 chunk +1 line, -4 lines 0 comments Download
M ui/base/resource/resource_bundle.cc View 1 chunk +2 lines, -7 lines 0 comments Download

Messages

Total messages: 8 (0 generated)
tfarina
Remove a call to LoadCommonResources(). It should help with https://codereview.chromium.org/419093002/
6 years, 4 months ago (2014-08-09 16:20:09 UTC) #1
tony
LGTM. I can't recall why we added that parameter. If you can find the reason ...
6 years, 4 months ago (2014-08-11 16:20:38 UTC) #2
tfarina
Found it! The parameter was added by commit https://chromium.googlesource.com/chromium/src/+/6c26d9cf2d0ee13c4ab67040923c953c50e0d41b%5E%21/ The remaining callers are different now.
6 years, 4 months ago (2014-08-11 17:49:10 UTC) #3
tfarina
TBRing Ben for this...
6 years, 4 months ago (2014-08-12 04:12:24 UTC) #4
tfarina
The CQ bit was checked by tfarina@chromium.org
6 years, 4 months ago (2014-08-12 04:12:30 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/tfarina@chromium.org/451283002/40001
6 years, 4 months ago (2014-08-12 04:15:03 UTC) #6
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: win_chromium_x64_rel on tryserver.chromium.win ...
6 years, 4 months ago (2014-08-12 10:24:43 UTC) #7
commit-bot: I haz the power
6 years, 4 months ago (2014-08-12 15:26:37 UTC) #8
Message was sent while issue was closed.
Change committed as 288978

Powered by Google App Engine
This is Rietveld 408576698