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

Issue 6254018: Allow chrome.send to pass number, boolean, null and arrays of those (Closed)

Created:
9 years, 11 months ago by arv (Not doing code reviews)
Modified:
9 years, 3 months ago
Reviewers:
Evan Martin, evanm
CC:
chromium-reviews, arv (Not doing code reviews), darin-cc_chromium.org, brettw-cc_chromium.org, Paweł Hajdan Jr.
Visibility:
Public.

Description

DOMUI: Allow chrome.send to pass number, boolean, null and arrays of those BUG=None TEST=cpp_variant_unittest covers most of the new functionality. I also verified that NTP works correctly.

Patch Set 1 #

Patch Set 2 : WIP #

Patch Set 3 : Rename function and fix indentation #

Total comments: 1

Patch Set 4 : Whitespace and comments #

Total comments: 7

Patch Set 5 : Windows needs type casts #

Patch Set 6 : Code review feedback fixes #

Patch Set 7 : Now with a test #

Patch Set 8 : Update copyright years #

Patch Set 9 : Use CppVariant so which uses a destructor which is cleaner #

Patch Set 10 : Use base::strdup instead of strdup #

Total comments: 2

Patch Set 11 : Use malloc+strcpy instead of strdup #

Total comments: 4

Patch Set 12 : Use base::strlcpy instead of strcpy #

Total comments: 1

Patch Set 13 : code review changes #

Unified diffs Side-by-side diffs Delta from patch set Stats (+130 lines, -37 lines) Patch
M chrome/browser/dom_ui/shown_sections_handler.cc View 1 2 3 4 5 6 7 2 chunks +4 lines, -3 lines 0 comments Download
M chrome/browser/resources/new_new_tab.js View 1 2 3 4 5 6 7 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/renderer/dom_ui_bindings.cc View 1 2 3 4 5 6 7 2 chunks +36 lines, -7 lines 0 comments Download
M webkit/glue/cpp_variant.h View 1 2 3 4 5 6 2 chunks +6 lines, -5 lines 0 comments Download
M webkit/glue/cpp_variant.cc View 1 2 3 4 5 6 7 8 9 10 3 chunks +14 lines, -19 lines 0 comments Download
M webkit/glue/cpp_variant_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +68 lines, -1 line 0 comments Download

Messages

Total messages: 13 (0 generated)
arv (Not doing code reviews)
Feedback wanted http://codereview.chromium.org/6254018/diff/4001/chrome/browser/dom_ui/shown_sections_handler.cc File chrome/browser/dom_ui/shown_sections_handler.cc (right): http://codereview.chromium.org/6254018/diff/4001/chrome/browser/dom_ui/shown_sections_handler.cc#newcode95 chrome/browser/dom_ui/shown_sections_handler.cc:95: CHECK(args->GetReal(0, &mode)); I'm going to do another ...
9 years, 11 months ago (2011-01-25 18:17:04 UTC) #1
evanm
Can you add a test to cpp_variant_unittest (if it exists?) http://codereview.chromium.org/6254018/diff/7001/chrome/renderer/dom_ui_bindings.cc File chrome/renderer/dom_ui_bindings.cc (right): http://codereview.chromium.org/6254018/diff/7001/chrome/renderer/dom_ui_bindings.cc#newcode17 ...
9 years, 11 months ago (2011-01-25 18:24:56 UTC) #2
arv (Not doing code reviews)
http://codereview.chromium.org/6254018/diff/7001/chrome/renderer/dom_ui_bindings.cc File chrome/renderer/dom_ui_bindings.cc (right): http://codereview.chromium.org/6254018/diff/7001/chrome/renderer/dom_ui_bindings.cc#newcode31 chrome/renderer/dom_ui_bindings.cc:31: // We currently assume all objects are arrays. On ...
9 years, 11 months ago (2011-01-26 00:50:32 UTC) #3
arv (Not doing code reviews)
PTAL
9 years, 11 months ago (2011-01-26 18:06:46 UTC) #4
Evan Martin
LGTM http://codereview.chromium.org/6254018/diff/2002/webkit/glue/cpp_variant.cc File webkit/glue/cpp_variant.cc (right): http://codereview.chromium.org/6254018/diff/2002/webkit/glue/cpp_variant.cc#newcode238 webkit/glue/cpp_variant.cc:238: CppVariant index_value; Just to confirm, by using CppVariant ...
9 years, 11 months ago (2011-01-26 19:21:09 UTC) #5
Evan Martin
thanks for writing the test, too!
9 years, 11 months ago (2011-01-26 19:21:17 UTC) #6
arv (Not doing code reviews)
http://codereview.chromium.org/6254018/diff/2002/webkit/glue/cpp_variant.cc File webkit/glue/cpp_variant.cc (right): http://codereview.chromium.org/6254018/diff/2002/webkit/glue/cpp_variant.cc#newcode238 webkit/glue/cpp_variant.cc:238: CppVariant index_value; On 2011/01/26 19:21:10, Evan Martin wrote: > ...
9 years, 11 months ago (2011-01-26 19:38:58 UTC) #7
arv (Not doing code reviews)
PTAL
9 years, 11 months ago (2011-01-27 22:04:49 UTC) #8
Evan Martin
LGTM http://codereview.chromium.org/6254018/diff/1007/webkit/glue/cpp_variant_unittest.cc File webkit/glue/cpp_variant_unittest.cc (right): http://codereview.chromium.org/6254018/diff/1007/webkit/glue/cpp_variant_unittest.cc#newcode447 webkit/glue/cpp_variant_unittest.cc:447: char* mem = static_cast<char*>(malloc((length + 1) * sizeof(char))); ...
9 years, 11 months ago (2011-01-27 22:12:57 UTC) #9
arv (Not doing code reviews)
http://codereview.chromium.org/6254018/diff/1007/webkit/glue/cpp_variant_unittest.cc File webkit/glue/cpp_variant_unittest.cc (right): http://codereview.chromium.org/6254018/diff/1007/webkit/glue/cpp_variant_unittest.cc#newcode447 webkit/glue/cpp_variant_unittest.cc:447: char* mem = static_cast<char*>(malloc((length + 1) * sizeof(char))); On ...
9 years, 11 months ago (2011-01-27 23:35:10 UTC) #10
Evan Martin
LGTM++ http://codereview.chromium.org/6254018/diff/31001/webkit/glue/cpp_variant_unittest.cc File webkit/glue/cpp_variant_unittest.cc (right): http://codereview.chromium.org/6254018/diff/31001/webkit/glue/cpp_variant_unittest.cc#newcode448 webkit/glue/cpp_variant_unittest.cc:448: char* mem = static_cast<char*>(malloc((length + 1) * sizeof(char))); ...
9 years, 11 months ago (2011-01-27 23:36:55 UTC) #11
arv (Not doing code reviews)
On Thu, Jan 27, 2011 at 15:36, <evan@chromium.org> wrote: > LGTM++ > > > http://codereview.chromium.org/6254018/diff/31001/webkit/glue/cpp_variant_unittest.cc ...
9 years, 11 months ago (2011-01-27 23:41:56 UTC) #12
arv (Not doing code reviews)
9 years, 10 months ago (2011-01-31 18:50:08 UTC) #13

Powered by Google App Engine
This is Rietveld 408576698