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

Issue 2887293003: Modernize some extensions code. (Closed)

Created:
3 years, 7 months ago by Lei Zhang
Modified:
3 years, 7 months ago
Reviewers:
Devlin, Ilya Sherman
CC:
chromium-reviews, chromium-apps-reviews_chromium.org, extensions-reviews_chromium.org, lazyboy
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Modernize some extensions code. - Use more unique_ptrs and avoid foo.reset(new Foo). - Use more C++11 features in general. - Unindent some code. - Mark more member variables const. - Remove an obsolete TODO. Review-Url: https://codereview.chromium.org/2887293003 Cr-Commit-Position: refs/heads/master@{#474097} Committed: https://chromium.googlesource.com/chromium/src/+/7ade5b542808909fcf16f86935d6a22e22f35ffd

Patch Set 1 #

Patch Set 2 : build #

Patch Set 3 : cros, more #

Patch Set 4 : more #

Total comments: 16

Patch Set 5 : address comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+235 lines, -267 lines) Patch
M chrome/browser/extensions/api/file_system/file_system_apitest_chromeos.cc View 1 2 3 4 1 chunk +8 lines, -6 lines 0 comments Download
M chrome/browser/extensions/api/runtime/chrome_runtime_api_delegate_unittest.cc View 1 4 chunks +9 lines, -8 lines 0 comments Download
M extensions/browser/event_listener_map.h View 3 chunks +6 lines, -6 lines 0 comments Download
M extensions/browser/event_listener_map.cc View 1 2 3 4 10 chunks +55 lines, -62 lines 0 comments Download
M extensions/browser/event_listener_map_unittest.cc View 1 2 15 chunks +68 lines, -82 lines 0 comments Download
M extensions/browser/event_router.h View 1 2 3 4 8 chunks +19 lines, -16 lines 0 comments Download
M extensions/browser/event_router.cc View 1 2 3 4 6 chunks +45 lines, -56 lines 0 comments Download
M extensions/browser/event_router_unittest.cc View 1 2 3 8 chunks +25 lines, -28 lines 0 comments Download
M extensions/browser/extension_event_histogram_value.h View 1 chunk +0 lines, -3 lines 0 comments Download

Messages

Total messages: 35 (24 generated)
Lei Zhang
Just more cleanup. No rush.
3 years, 7 months ago (2017-05-19 23:35:59 UTC) #12
Devlin
lgtm % nits. Thanks for doing this! +lazyboy as FYI since this is likely to ...
3 years, 7 months ago (2017-05-20 00:49:16 UTC) #13
Lei Zhang
https://codereview.chromium.org/2887293003/diff/60001/chrome/browser/extensions/api/file_system/file_system_apitest_chromeos.cc File chrome/browser/extensions/api/file_system/file_system_apitest_chromeos.cc (right): https://codereview.chromium.org/2887293003/diff/60001/chrome/browser/extensions/api/file_system/file_system_apitest_chromeos.cc#newcode96 chrome/browser/extensions/api/file_system/file_system_apitest_chromeos.cc:96: base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, callback_); On 2017/05/20 00:49:15, Devlin (catching up) wrote: ...
3 years, 7 months ago (2017-05-20 01:04:14 UTC) #14
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/2887293003/80001
3 years, 7 months ago (2017-05-20 05:40:58 UTC) #21
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/443171)
3 years, 7 months ago (2017-05-20 05:48:48 UTC) #23
Lei Zhang
+isherman for extensions/browser/extension_event_histogram_value.h
3 years, 7 months ago (2017-05-22 19:12:04 UTC) #25
Ilya Sherman
LGTM
3 years, 7 months ago (2017-05-22 20:36:46 UTC) #26
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/2887293003/80001
3 years, 7 months ago (2017-05-22 22:04:40 UTC) #28
commit-bot: I haz the power
Try jobs failed on following builders: win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_rel_ng/builds/433544)
3 years, 7 months ago (2017-05-23 00:00:01 UTC) #30
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/2887293003/80001
3 years, 7 months ago (2017-05-23 21:36:44 UTC) #32
commit-bot: I haz the power
3 years, 7 months ago (2017-05-23 23:13:53 UTC) #35
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/7ade5b542808909fcf16f86935d6...

Powered by Google App Engine
This is Rietveld 408576698