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

Issue 2637393002: Make mojom-generated C++ string constants really constant. (Closed)

Created:
3 years, 11 months ago by Sam McNally
Modified:
3 years, 11 months ago
Reviewers:
yzshen1
CC:
chromium-reviews, qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, yzshen+watch_chromium.org, abarth-chromium, Aaron Boodman, darin (slow to review), chrome-apps-syd-reviews_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Make mojom-generated C++ string constants really constant. Previously, string constants generated from mojoms were of the form const char* kName = "value"; This allows kName to be changed at runtime to point at other strings. This replaces those with const char kName[] = "value"; This also changes mojom-generated scalar constants to be constexpr. BUG=682102 Review-Url: https://codereview.chromium.org/2637393002 Cr-Commit-Position: refs/heads/master@{#444871} Committed: https://chromium.googlesource.com/chromium/src/+/1bcfeb4b504a90db6bd008c4e87e895f5957161f

Patch Set 1 #

Patch Set 2 : #

Patch Set 3 : #

Patch Set 4 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+66 lines, -50 lines) Patch
M mojo/public/cpp/bindings/tests/constant_unittest.cc View 1 2 3 2 chunks +23 lines, -5 lines 0 comments Download
M mojo/public/interfaces/bindings/tests/test_constants.mojom View 1 chunk +4 lines, -0 lines 0 comments Download
M mojo/public/tools/bindings/generators/cpp_templates/enum_macros.tmpl View 1 1 chunk +1 line, -1 line 0 comments Download
M mojo/public/tools/bindings/generators/cpp_templates/interface_declaration.tmpl View 1 2 2 chunks +4 lines, -8 lines 0 comments Download
M mojo/public/tools/bindings/generators/cpp_templates/interface_definition.tmpl View 1 2 1 chunk +2 lines, -5 lines 0 comments Download
M mojo/public/tools/bindings/generators/cpp_templates/module-shared-internal.h.tmpl View 1 1 chunk +1 line, -1 line 0 comments Download
M mojo/public/tools/bindings/generators/cpp_templates/module-shared.h.tmpl View 1 1 chunk +2 lines, -2 lines 0 comments Download
M mojo/public/tools/bindings/generators/cpp_templates/module.cc.tmpl View 1 2 1 chunk +4 lines, -6 lines 0 comments Download
M mojo/public/tools/bindings/generators/cpp_templates/module.h.tmpl View 1 2 2 chunks +2 lines, -7 lines 0 comments Download
M mojo/public/tools/bindings/generators/cpp_templates/struct_definition.tmpl View 1 1 chunk +1 line, -1 line 0 comments Download
M mojo/public/tools/bindings/generators/cpp_templates/wrapper_class_declaration.tmpl View 1 2 1 chunk +1 line, -5 lines 0 comments Download
M mojo/public/tools/bindings/generators/mojom_cpp_generator.py View 1 2 3 chunks +21 lines, -9 lines 0 comments Download

Messages

Total messages: 26 (22 generated)
Sam McNally
3 years, 11 months ago (2017-01-19 06:26:53 UTC) #18
yzshen1
LGTM
3 years, 11 months ago (2017-01-19 21:03:24 UTC) #21
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/2637393002/60001
3 years, 11 months ago (2017-01-19 22:41:27 UTC) #23
commit-bot: I haz the power
3 years, 11 months ago (2017-01-19 22:53:54 UTC) #26
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/1bcfeb4b504a90db6bd008c4e87e...

Powered by Google App Engine
This is Rietveld 408576698