Issue 160064: Push bookmarks.remove/removeAll polymorphism into c++. fix bookmarks id schema issues

Issue 160064: Push bookmarks.remove/removeAll polymorphism into c++. fix bookmarks id schema issues (Closed)

Created:
11 years, 5 months ago by rafaelw

Modified:
9 years, 7 months ago

Reviewers:
Erik does not do reviews

CC:
chromium-reviews_googlegroups.com

Visibility:
Public.

Description

Push bookmarks.remove/removeAll polymorphism into c++. fix bookmarks id schema issues (http://code.google.com/p/chromium/issues/detail?id=17562 failed to update schema types from int to string). R=erikkay BUG=17417 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=21503

Patch Set 3 : presubmit #

Created: 11 years, 5 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+76 lines, -80 lines)			Patch
M	chrome/browser/automation/automation_extension_function.h	View		2 chunks	+0 lines, -2 lines	0 comments	Download
M	chrome/browser/automation/automation_extension_function.cc	View		1 chunk	+0 lines, -4 lines	0 comments	Download
M	chrome/browser/extensions/extension_bookmarks_module.cc	View		2 chunks	+15 lines, -9 lines	0 comments	Download
M	chrome/browser/extensions/extension_function.h	View	1	2 chunks	+6 lines, -2 lines	0 comments	Download
M	chrome/browser/extensions/extension_function_dispatcher.cc	View		1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/common/common_resources.grd	View		1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/common/extensions/api/extension_api.json	View		11 chunks	+30 lines, -12 lines	0 comments	Download
M	chrome/renderer/extensions/extension_api_client_unittest.cc	View	1	3 chunks	+22 lines, -22 lines	0 comments	Download
M	chrome/renderer/renderer_resources.grd	View		1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/renderer/resources/extension_process_bindings.js	View		1 chunk	+0 lines, -26 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

Erik does not do reviews

LGTM - just nits http://codereview.chromium.org/160064/diff/1/5 File chrome/browser/extensions/extension_function.h (right): http://codereview.chromium.org/160064/diff/1/5#newcode36 Line 36: virtual void set_name(const std::string& ...

11 years, 5 months ago (2009-07-23 23:36:20 UTC) #2

rafaelw

11 years, 5 months ago (2009-07-24 00:27:28 UTC) #3

http://codereview.chromium.org/160064/diff/1/5
File chrome/browser/extensions/extension_function.h (right):

http://codereview.chromium.org/160064/diff/1/5#newcode36
Line 36: virtual void set_name(const std::string& name) {
On 2009/07/23 23:36:20, Erik Kay wrote:
> on one line

Done. Also didn't mean to leave this as virtual

http://codereview.chromium.org/160064/diff/1/5#newcode39
Line 39: const std::string name() {
On 2009/07/23 23:36:20, Erik Kay wrote:
> ditto

Done.

http://codereview.chromium.org/160064/diff/1/9
File chrome/renderer/extensions/extension_api_client_unittest.cc (right):

http://codereview.chromium.org/160064/diff/1/9#newcode442
Line 442: ExpectJsPass("chrome.bookmarks.get(\"0\", function(){});",
On 2009/07/23 23:36:20, Erik Kay wrote:
> why not just use ' instead of " for all of these strings to avoid escaping?

Done. Add fixed elsewhere in this file.

Expand Messages | Collapse Messages