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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 176363002: Rename Start/EndKeepAlive to Increment/DecrementKeepAliveCount (Closed)

Created:
6 years, 10 months ago by James Cook
Modified:
6 years, 10 months ago
Reviewers:
miket_OOO, sky
CC:
chromium-reviews, chrome-apps-syd-reviews_chromium.org, vsevik, sadrul, nkostylev+watch_chromium.org, jennb, tfarina, ben+ash_chromium.org, jianli, Dmitry Titov, yurys, marja+watch_chromium.org, kalyank, paulirish+reviews_chromium.org, pfeldman, oshima+watch_chromium.org, devtools-reviews_chromium.org, chromium-apps-reviews_chromium.org, dcheng, aandrey+blink_chromium.org, stevenjb+watch_chromium.org, davemoore+watch_chromium.org
Visibility:
Public.

Description

Rename Start/EndKeepAlive to Increment/DecrementKeepAliveCount Keep alive is implemented as a reference count. This name makes it clear that a call to EndKeepAlive does not immediately end keep-alive, which came up in code review. BUG=345918 TEST=compiles, no functional changes Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=253165

Patch Set 1 #

Total comments: 1

Patch Set 2 : rebase (rename_keep_alive) #

Unified diffs Side-by-side diffs Delta from patch set Stats (+87 lines, -86 lines) Patch
M apps/app_keep_alive_service.cc View 3 chunks +3 lines, -3 lines 0 comments Download
M apps/app_keep_alive_service_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M apps/app_window.cc View 1 2 chunks +2 lines, -2 lines 0 comments Download
M apps/apps_client.h View 1 1 chunk +4 lines, -4 lines 0 comments Download
M apps/shell/browser/shell_apps_client.h View 1 1 chunk +2 lines, -2 lines 0 comments Download
M apps/shell/browser/shell_apps_client.cc View 1 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/app_controller_mac.mm View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/apps/chrome_apps_client.h View 1 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/apps/chrome_apps_client.cc View 1 1 chunk +4 lines, -4 lines 0 comments Download
M chrome/browser/background/background_mode_manager.h View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/background/background_mode_manager.cc View 6 chunks +8 lines, -8 lines 0 comments Download
M chrome/browser/background/background_mode_manager_unittest.cc View 8 chunks +18 lines, -14 lines 0 comments Download
M chrome/browser/chromeos/login/login_display_host_impl.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/policy/device_local_account_browsertest.cc View 5 chunks +5 lines, -5 lines 0 comments Download
M chrome/browser/devtools/devtools_sanity_browsertest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/lifetime/application_lifetime.h View 1 chunk +5 lines, -4 lines 0 comments Download
M chrome/browser/lifetime/application_lifetime.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/lifetime/application_lifetime_aura.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/lifetime/browser_close_manager_browsertest.cc View 3 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/printing/print_dialog_cloud.cc View 2 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/sessions/better_session_restore_browsertest.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/ui/app_list/keep_alive_service_impl.cc View 1 chunk +2 lines, -6 lines 0 comments Download
M chrome/browser/ui/ash/ash_init.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/panels/panel.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/ui/views/user_manager_view.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/ui/window_sizer/window_sizer_ash_uitest.cc View 4 chunks +6 lines, -6 lines 0 comments Download

Messages

Total messages: 13 (0 generated)
James Cook
miket, PTAL
6 years, 10 months ago (2014-02-22 15:25:45 UTC) #1
miket_OOO
I think this reads much better than before. Thanks, and LGTM. https://codereview.chromium.org/176363002/diff/1/chrome/browser/lifetime/application_lifetime.h File chrome/browser/lifetime/application_lifetime.h (right): ...
6 years, 10 months ago (2014-02-24 16:41:31 UTC) #2
James Cook
sky, can I get OWNERS for this rename?
6 years, 10 months ago (2014-02-24 17:32:51 UTC) #3
sky
LGTM
6 years, 10 months ago (2014-02-24 20:17:34 UTC) #4
James Cook
The CQ bit was checked by jamescook@chromium.org
6 years, 10 months ago (2014-02-24 20:34:39 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jamescook@chromium.org/176363002/200001
6 years, 10 months ago (2014-02-24 20:36:09 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jamescook@chromium.org/176363002/200001
6 years, 10 months ago (2014-02-24 21:16:49 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jamescook@chromium.org/176363002/200001
6 years, 10 months ago (2014-02-25 01:27:40 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jamescook@chromium.org/176363002/200001
6 years, 10 months ago (2014-02-25 07:30:46 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jamescook@chromium.org/176363002/200001
6 years, 10 months ago (2014-02-25 09:07:42 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jamescook@chromium.org/176363002/200001
6 years, 10 months ago (2014-02-25 09:25:55 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jamescook@chromium.org/176363002/200001
6 years, 10 months ago (2014-02-25 13:32:59 UTC) #12
commit-bot: I haz the power
6 years, 10 months ago (2014-02-25 15:17:30 UTC) #13
Message was sent while issue was closed. 
Change committed as 253165

Powered by Google App Engine
This is Rietveld 408576698