Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(9)

Issue 6954001: Add "Keep chrome running in background" preference. (Closed)

Created:
7 years, 6 months ago by Andrew T Wilson (Slow)
Modified:
7 years, 5 months ago
CC:
chromium-reviews
Visibility:
Public.

Description

Add "Keep chrome running in background" preference. BUG=76828 TEST=Install background app, disable "Keep chrome running in background", see status tray icon disappear. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=85318

Patch Set 1 #

Patch Set 2 : Added preferences. #

Patch Set 3 : Fixed formatting nits #

Total comments: 5

Patch Set 4 : Review feedback + added tests. #

Total comments: 12

Patch Set 5 : More review feedback. #

Patch Set 6 : Merged with ToT. #

Patch Set 7 : Fixed problem that was making UI visible on Mac. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+275 lines, -20 lines) Patch
M chrome/app/chrome_command_ids.h View 1 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/app/chromium_strings.grd View 1 2 3 4 5 1 chunk +9 lines, -0 lines 0 comments Download
M chrome/app/generated_resources.grd View 1 2 3 4 5 6 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/app/google_chrome_strings.grd View 1 2 3 4 5 1 chunk +9 lines, -0 lines 0 comments Download
M chrome/browser/background_mode_manager.h View 1 2 3 3 chunks +19 lines, -5 lines 0 comments Download
M chrome/browser/background_mode_manager.cc View 1 2 3 4 8 chunks +89 lines, -13 lines 0 comments Download
M chrome/browser/background_mode_manager_unittest.cc View 1 2 3 4 3 chunks +62 lines, -2 lines 0 comments Download
M chrome/browser/resources/options/advanced_options.html View 1 2 3 4 5 6 1 chunk +11 lines, -0 lines 0 comments Download
M chrome/browser/resources/options/advanced_options.js View 1 2 3 4 2 chunks +11 lines, -0 lines 0 comments Download
M chrome/browser/ui/webui/options/advanced_options_handler.h View 1 2 chunks +13 lines, -0 lines 0 comments Download
M chrome/browser/ui/webui/options/advanced_options_handler.cc View 1 2 3 4 6 chunks +42 lines, -0 lines 0 comments Download
M chrome/common/pref_names.h View 1 1 chunk +1 line, -0 lines 0 comments Download
M chrome/common/pref_names.cc View 1 1 chunk +3 lines, -0 lines 0 comments Download

Messages

Total messages: 8 (0 generated)
Andrew T Wilson (Slow)
Please take a look - you can ignore the advanced_options* files if you like - ...
7 years, 6 months ago (2011-05-11 18:31:50 UTC) #1
Rick Campbell
LGTM with some wording nits. http://codereview.chromium.org/6954001/diff/5001/chrome/app/chromium_strings.grd File chrome/app/chromium_strings.grd (right): http://codereview.chromium.org/6954001/diff/5001/chrome/app/chromium_strings.grd#newcode339 chrome/app/chromium_strings.grd:339: Continue running background apps ...
7 years, 6 months ago (2011-05-11 20:33:01 UTC) #2
Andrew T Wilson (Slow)
Please take another look - I've updated the unit tests. Evan, can you take a ...
7 years, 6 months ago (2011-05-11 23:00:48 UTC) #3
Rick Campbell
LGTM if one comment is fixed. I suggest one new DCHECK as well. http://codereview.chromium.org/6954001/diff/9001/chrome/browser/background_mode_manager.cc File ...
7 years, 6 months ago (2011-05-12 05:42:42 UTC) #4
Evan Stade
http://codereview.chromium.org/6954001/diff/9001/chrome/browser/resources/options/advanced_options.html File chrome/browser/resources/options/advanced_options.html (right): http://codereview.chromium.org/6954001/diff/9001/chrome/browser/resources/options/advanced_options.html#newcode219 chrome/browser/resources/options/advanced_options.html:219: <span i18n-content="backgroundModeCheckbox"></span> why can't you use pref= here? http://codereview.chromium.org/6954001/diff/9001/chrome/browser/resources/options/advanced_options.js ...
7 years, 6 months ago (2011-05-12 18:36:31 UTC) #5
Andrew T Wilson (Slow)
Please take another look. http://codereview.chromium.org/6954001/diff/9001/chrome/browser/background_mode_manager.cc File chrome/browser/background_mode_manager.cc (right): http://codereview.chromium.org/6954001/diff/9001/chrome/browser/background_mode_manager.cc#newcode236 chrome/browser/background_mode_manager.cc:236: // If background mode should ...
7 years, 6 months ago (2011-05-12 23:34:30 UTC) #6
Evan Stade
lgtm
7 years, 6 months ago (2011-05-13 20:33:24 UTC) #7
non.chromium.rickcam
7 years, 6 months ago (2011-05-13 21:23:41 UTC) #8
LGTM as well

Powered by Google App Engine
This is Rietveld 408576698