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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 10694106: Added support for multiple parameters to Extension API callbacks. (Closed)

Created:
8 years, 5 months ago by Matt Tytel
Modified:
8 years, 5 months ago
CC:
chromium-reviews, mihaip-chromium-reviews_chromium.org, oshima+watch_chromium.org, nkostylev+watch_chromium.org, hashimoto+watch_chromium.org, feature-media-reviews_chromium.org, aboxhall+watch_chromium.org, yoshiki+watch_chromium.org, yuzo+watch_chromium.org, davidbarr+watch_chromium.org, Aaron Boodman, rginda+watch_chromium.org, Satish, yusukes+watch_chromium.org, dmazzoni+watch_chromium.org, dtseng+watch_chromium.org, brettw-cc_chromium.org, kinuko+watch, ctguil+watch_chromium.org, stevenjb+watch_chromium.org, davemoore+watch_chromium.org, zork+watch_chromium.org, benwells, not at google - send to devlin
Base URL:
http://git.chromium.org/chromium/src.git@master
Visibility:
Public.

Description

Added support for multiple parameters to Extension API callbacks. BUG=135269 TEST= Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=146469

Patch Set 1 #

Patch Set 2 : Indentation fixes and comment. #

Total comments: 4

Patch Set 3 : Review fixes. #

Total comments: 4

Patch Set 4 : s/SetSingleResult/SetResult/g. #

Patch Set 5 : s/GetResults/GetResultList/g. #

Patch Set 6 : Synced. #

Patch Set 7 : Synced. #

Patch Set 8 : Renamed an existing function that was called SetResult. #

Patch Set 9 : Synced and fixed chromeos compilation issue. #

Patch Set 10 : Synced. #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+368 lines, -353 lines) Patch
M chrome/browser/accessibility/accessibility_extension_api.cc View 1 2 3 2 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/bookmarks/bookmark_extension_api.cc View 1 2 3 9 chunks +9 lines, -9 lines 0 comments Download
M chrome/browser/bookmarks/bookmark_manager_extension_api.cc View 1 2 3 4 chunks +5 lines, -5 lines 0 comments Download
M chrome/browser/chromeos/extensions/echo_private_api.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/chromeos/extensions/file_browser_handler_api.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/chromeos/extensions/file_browser_handler_api_test.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/extensions/file_browser_private_api.cc View 1 2 3 4 5 6 7 8 9 22 chunks +26 lines, -26 lines 0 comments Download
M chrome/browser/chromeos/media/media_player_extension_api.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/extensions/api/alarms/alarms_api.cc View 1 2 3 2 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/extensions/api/alarms/alarms_api_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/extensions/api/app_window/app_window_api.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/extensions/api/bluetooth/bluetooth_api.cc View 1 2 3 4 5 6 7 8 7 chunks +10 lines, -10 lines 0 comments Download
M chrome/browser/extensions/api/bluetooth/bluetooth_apitest_chromeos.cc View 1 2 3 4 5 6 chunks +8 lines, -7 lines 0 comments Download
M chrome/browser/extensions/api/browsing_data/browsing_data_test.cc View 4 chunks +11 lines, -6 lines 0 comments Download
M chrome/browser/extensions/api/content_settings/content_settings_api.cc View 1 2 3 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/extensions/api/cookies/cookies_api.cc View 1 2 3 5 chunks +6 lines, -8 lines 0 comments Download
M chrome/browser/extensions/api/debugger/debugger_api.cc View 1 2 3 4 5 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/extensions/api/declarative/declarative_api.cc View 1 2 3 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/extensions/api/discovery/discovery_api_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/extensions/api/dns/dns_api.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/extensions/api/dns/dns_apitest.cc View 1 3 chunks +4 lines, -4 lines 0 comments Download
M chrome/browser/extensions/api/downloads/downloads_api.cc View 1 2 3 4 5 3 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/extensions/api/downloads/downloads_api_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/extensions/api/extension_action/extension_actions_api.cc View 1 2 3 2 chunks +5 lines, -6 lines 0 comments Download
M chrome/browser/extensions/api/file_system/file_system_api.cc View 1 2 3 4 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/extensions/api/identity/identity_api.cc View 1 2 3 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/extensions/api/idltest/idltest_api.cc View 1 2 3 2 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/extensions/api/media_gallery/media_gallery_api.cc View 1 2 3 4 3 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/extensions/api/offscreen_tabs/offscreen_tabs_api.cc View 1 2 3 3 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/extensions/api/permissions/permissions_api.cc View 1 2 3 4 6 chunks +8 lines, -8 lines 0 comments Download
M chrome/browser/extensions/api/record/record_api.cc View 1 2 3 4 5 3 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/extensions/api/record/record_api_test.cc View 1 2 3 4 5 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/extensions/api/serial/serial_api.cc View 1 2 3 4 5 6 7 8 9 8 chunks +11 lines, -11 lines 0 comments Download
M chrome/browser/extensions/api/serial/serial_apitest.cc View 1 2 3 4 5 6 7 8 9 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/extensions/api/socket/socket_api.cc View 1 2 3 12 chunks +12 lines, -12 lines 0 comments Download
M chrome/browser/extensions/api/socket/socket_apitest.cc View 1 2 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/extensions/api/tabs/tabs.cc View 1 2 3 4 5 6 7 8 18 chunks +29 lines, -30 lines 0 comments Download
M chrome/browser/extensions/api/tabs/tabs_test.cc View 1 2 3 4 19 chunks +25 lines, -25 lines 0 comments Download
M chrome/browser/extensions/api/terminal/terminal_private_api.cc View 1 2 3 4 chunks +4 lines, -4 lines 0 comments Download
M chrome/browser/extensions/api/test/test_api.cc View 1 2 3 4 5 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/extensions/api/usb/usb_api.cc View 1 2 3 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/extensions/api/web_navigation/web_navigation_api.cc View 1 2 3 4 chunks +4 lines, -4 lines 0 comments Download
M chrome/browser/extensions/api/web_socket_proxy_private/web_socket_proxy_private_api.cc View 1 2 3 3 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/extensions/api/webstore_private/webstore_private_api.h View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/extensions/api/webstore_private/webstore_private_api.cc View 1 2 3 10 chunks +21 lines, -21 lines 0 comments Download
M chrome/browser/extensions/api/webstore_private/webstore_private_apitest.cc View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/extensions/extension_chrome_auth_private_api.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/extensions/extension_font_settings_api.cc View 1 2 3 4 3 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/extensions/extension_function.h View 1 2 3 4 5 2 chunks +8 lines, -5 lines 0 comments Download
M chrome/browser/extensions/extension_function.cc View 1 2 3 4 2 chunks +11 lines, -8 lines 3 comments Download
M chrome/browser/extensions/extension_function_test_utils.h View 1 2 3 4 5 1 chunk +9 lines, -7 lines 0 comments Download
M chrome/browser/extensions/extension_function_test_utils.cc View 1 2 3 4 1 chunk +17 lines, -11 lines 0 comments Download
M chrome/browser/extensions/extension_i18n_api.cc View 1 2 3 3 chunks +4 lines, -4 lines 0 comments Download
M chrome/browser/extensions/extension_idle_api.cc View 1 2 3 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/extensions/extension_info_private_api_chromeos.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/extensions/extension_input_ime_api.cc View 1 2 3 13 chunks +18 lines, -18 lines 0 comments Download
M chrome/browser/extensions/extension_input_method_api.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/extensions/extension_managed_mode_api.cc View 1 2 3 3 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/extensions/extension_management_api.cc View 1 2 3 4 5 4 chunks +4 lines, -4 lines 0 comments Download
M chrome/browser/extensions/extension_management_api_browsertest.cc View 1 2 chunks +5 lines, -5 lines 0 comments Download
M chrome/browser/extensions/extension_module.cc View 1 2 3 4 2 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/extensions/extension_page_capture_api.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/extensions/extension_preference_api.cc View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/extensions/extension_processes_api.cc View 1 2 3 3 chunks +4 lines, -4 lines 0 comments Download
M chrome/browser/extensions/platform_app_browsertest_util.cc View 1 2 3 4 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/extensions/settings/settings_api.cc View 1 2 3 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/extensions/system/system_api.cc View 1 2 3 2 chunks +2 lines, -3 lines 0 comments Download
M chrome/browser/history/history_extension_api.cc View 1 2 3 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/history/top_sites_extension_api.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/history/top_sites_extension_test.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/infobars/infobar_extension_api.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/rlz/rlz_extension_api.cc View 1 2 3 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/speech/extension_api/tts_extension_api.cc View 1 2 3 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/speech/speech_input_extension_api.h View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/speech/speech_input_extension_api.cc View 1 2 3 4 5 6 7 1 chunk +3 lines, -3 lines 0 comments Download

Messages

Total messages: 21 (0 generated)
Matt Tytel
Round 1. I'd like your input on testing this though, since there are no APIs ...
8 years, 5 months ago (2012-07-06 20:24:23 UTC) #1
Aaron Boodman
Sweet cleanup. Someone besides me should review this so you get exposure to more people. ...
8 years, 5 months ago (2012-07-09 04:39:31 UTC) #2
Matt Tytel
Fixes and ++kalman.
8 years, 5 months ago (2012-07-09 18:10:20 UTC) #3
Matt Perry
https://chromiumcodereview.appspot.com/10694106/diff/6001/chrome/browser/extensions/extension_function.h File chrome/browser/extensions/extension_function.h (right): https://chromiumcodereview.appspot.com/10694106/diff/6001/chrome/browser/extensions/extension_function.h#newcode112 chrome/browser/extensions/extension_function.h:112: void SetSingleResult(base::Value* result); IMO, Single is redundant here given ...
8 years, 5 months ago (2012-07-09 23:52:25 UTC) #4
Matt Tytel
https://chromiumcodereview.appspot.com/10694106/diff/1073/chrome/browser/extensions/extension_function.h File chrome/browser/extensions/extension_function.h (right): https://chromiumcodereview.appspot.com/10694106/diff/1073/chrome/browser/extensions/extension_function.h#newcode115 chrome/browser/extensions/extension_function.h:115: const base::ListValue* GetResultsListValue(); On 2012/07/09 04:39:31, Aaron Boodman wrote: ...
8 years, 5 months ago (2012-07-10 18:50:44 UTC) #5
Aaron Boodman
lgtm https://chromiumcodereview.appspot.com/10694106/diff/6001/chrome/browser/extensions/extension_function.h File chrome/browser/extensions/extension_function.h (right): https://chromiumcodereview.appspot.com/10694106/diff/6001/chrome/browser/extensions/extension_function.h#newcode112 chrome/browser/extensions/extension_function.h:112: void SetSingleResult(base::Value* result); On 2012/07/10 18:50:45, Matt Tytel ...
8 years, 5 months ago (2012-07-10 20:34:22 UTC) #6
Matt Perry
lgtm
8 years, 5 months ago (2012-07-10 20:39:09 UTC) #7
Matt Tytel
sky - could I get a (super quick) review for chrome/browser/history and chrome/browser/bookmarks? https://chromiumcodereview.appspot.com/10694106/diff/6001/chrome/browser/extensions/extension_function.h File ...
8 years, 5 months ago (2012-07-10 20:54:28 UTC) #8
Matt Tytel
Hi Brett, could I get a quick review for chrome/browser/history and chrome/browser/bookmarks in place of ...
8 years, 5 months ago (2012-07-10 21:58:40 UTC) #9
brettw
history and bookmarks LGTM
8 years, 5 months ago (2012-07-12 05:01:05 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mtytel@chromium.org/10694106/3076
8 years, 5 months ago (2012-07-12 06:32:37 UTC) #11
commit-bot: I haz the power
Try job failure for 10694106-3076 (retry) on linux_chromeos for step "compile" (clobber build). It's a ...
8 years, 5 months ago (2012-07-12 06:54:56 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mtytel@chromium.org/10694106/20001
8 years, 5 months ago (2012-07-12 07:46:45 UTC) #13
commit-bot: I haz the power
Try job failure for 10694106-20001 (retry) on linux_chromeos for step "compile" (clobber build). It's a ...
8 years, 5 months ago (2012-07-12 08:11:04 UTC) #14
Matt Tytel
8 years, 5 months ago (2012-07-12 21:13:41 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mtytel@chromium.org/10694106/4005
8 years, 5 months ago (2012-07-12 21:13:47 UTC) #16
commit-bot: I haz the power
Change committed as 146469
8 years, 5 months ago (2012-07-12 22:39:12 UTC) #17
koz (OOO until 15th September)
https://chromiumcodereview.appspot.com/10694106/diff/4005/chrome/browser/extensions/extension_function.cc File chrome/browser/extensions/extension_function.cc (right): https://chromiumcodereview.appspot.com/10694106/diff/4005/chrome/browser/extensions/extension_function.cc#newcode142 chrome/browser/extensions/extension_function.cc:142: routing_id, request_id_, success, *results_.release(), GetError())); Is this a memory ...
8 years, 5 months ago (2012-07-13 04:48:16 UTC) #18
Matt Tytel
https://chromiumcodereview.appspot.com/10694106/diff/4005/chrome/browser/extensions/extension_function.cc File chrome/browser/extensions/extension_function.cc (right): https://chromiumcodereview.appspot.com/10694106/diff/4005/chrome/browser/extensions/extension_function.cc#newcode142 chrome/browser/extensions/extension_function.cc:142: routing_id, request_id_, success, *results_.release(), GetError())); :( Yes that is. ...
8 years, 5 months ago (2012-07-13 05:14:39 UTC) #19
not at google - send to devlin
https://chromiumcodereview.appspot.com/10694106/diff/4005/chrome/browser/extensions/extension_function.cc File chrome/browser/extensions/extension_function.cc (right): https://chromiumcodereview.appspot.com/10694106/diff/4005/chrome/browser/extensions/extension_function.cc#newcode142 chrome/browser/extensions/extension_function.cc:142: routing_id, request_id_, success, *results_.release(), GetError())); On 2012/07/13 05:14:39, Matt ...
8 years, 5 months ago (2012-07-13 05:20:05 UTC) #20
Matt Tytel
8 years, 5 months ago (2012-07-13 05:36:11 UTC) #21 
Yea that will work. Would you do me a favor and revert this? I'm not a
committer.

Powered by Google App Engine
This is Rietveld 408576698