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

Issue 2407993002: Remove unnecessary keybinding logging (Closed)

Created:
4 years, 2 months ago by Mike Wittman
Modified:
4 years, 2 months ago
Reviewers:
Finnur
CC:
chromium-reviews, chromium-apps-reviews_chromium.org, extensions-reviews_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Remove unnecessary keybinding logging This is logging slowing down pre-message-loop startup. The average slowdown over the whole population is ~620μs, which is significant considering the vast majority of users likely do not hit this code path. BUG=654576 Committed: https://crrev.com/e28a5860367f57a4a769f84f9d4c295b5b92f869 Cr-Commit-Position: refs/heads/master@{#425058}

Patch Set 1 #

Total comments: 2

Patch Set 2 : remove additional logging statement #

Unified diffs Side-by-side diffs Delta from patch set Stats (+0 lines, -6 lines) Patch
M chrome/browser/extensions/extension_commands_global_registry.cc View 1 2 chunks +0 lines, -6 lines 0 comments Download

Messages

Total messages: 12 (6 generated)
Mike Wittman
Hi Finnur, please take a look. Looks like you added this long ago in this ...
4 years, 2 months ago (2016-10-10 22:39:46 UTC) #3
Finnur
LGTM, one nit. https://codereview.chromium.org/2407993002/diff/1/chrome/browser/extensions/extension_commands_global_registry.cc File chrome/browser/extensions/extension_commands_global_registry.cc (right): https://codereview.chromium.org/2407993002/diff/1/chrome/browser/extensions/extension_commands_global_registry.cc#newcode99 chrome/browser/extensions/extension_commands_global_registry.cc:99: VLOG(0) << "Removing keybinding for " ...
4 years, 2 months ago (2016-10-13 09:59:16 UTC) #4
Mike Wittman
Thanks! https://codereview.chromium.org/2407993002/diff/1/chrome/browser/extensions/extension_commands_global_registry.cc File chrome/browser/extensions/extension_commands_global_registry.cc (right): https://codereview.chromium.org/2407993002/diff/1/chrome/browser/extensions/extension_commands_global_registry.cc#newcode99 chrome/browser/extensions/extension_commands_global_registry.cc:99: VLOG(0) << "Removing keybinding for " << command_name.c_str(); ...
4 years, 2 months ago (2016-10-13 16:07:35 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2407993002/20001
4 years, 2 months ago (2016-10-13 16:08:01 UTC) #8
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years, 2 months ago (2016-10-13 16:44:14 UTC) #10
commit-bot: I haz the power
4 years, 2 months ago (2016-10-13 16:47:15 UTC) #12
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/e28a5860367f57a4a769f84f9d4c295b5b92f869
Cr-Commit-Position: refs/heads/master@{#425058}

Powered by Google App Engine
This is Rietveld 408576698