Issue 10836228: Convert multiple boolean params to mask in getExtensionByIdInternal.

Issue 10836228: Convert multiple boolean params to mask in getExtensionByIdInternal. (Closed)

Created:
8 years, 4 months ago by Gaurav

Modified:
8 years, 3 months ago

Reviewers:
Mihai Parparita -not on Chrome

CC:
chromium-reviews, Aaron Boodman, mihaip-chromium-reviews_chromium.org

Base URL:
http://git.chromium.org/git/chromium.git@trunk

Visibility:
Public.

More Reviews

Description

Convert multiple boolean params to mask in getExtensionByIdInternal. BUG=137143 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=151649

Patch Set 2 : comments and rebase #

Total comments: 1

Created: 8 years, 4 months ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+40 lines, -19 lines)			Patch
	M	chrome/browser/extensions/extension_service.h	View	1	1 chunk	+8 lines, -3 lines	0 comments	Download
	M	chrome/browser/extensions/extension_service.cc	View	1	9 chunks	+32 lines, -16 lines	1 comment	Download

Messages

Total messages: 6 (0 generated)

Expand Messages | Collapse Messages

Mihai Parparita -not on Chrome

LGTM https://chromiumcodereview.appspot.com/10836228/diff/1/chrome/browser/extensions/extension_service.cc File chrome/browser/extensions/extension_service.cc (right): https://chromiumcodereview.appspot.com/10836228/diff/1/chrome/browser/extensions/extension_service.cc#newcode2207 chrome/browser/extensions/extension_service.cc:2207: if (include_mask & ExtensionService::INCLUDE_ENABLED) { Nit: since you're ...

8 years, 4 months ago (2012-08-15 00:10:07 UTC) #2

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/grv@chromium.org/10836228/4001

8 years, 4 months ago (2012-08-15 01:32:10 UTC) #3

Mihai Parparita -not on Chrome

Oops, didn't catch that this didn't have a BUG=137143 line in the description. Mihai On ...

8 years, 4 months ago (2012-08-16 00:38:12 UTC) #5

Lei Zhang

8 years, 3 months ago (2012-09-11 22:05:43 UTC) #6

Post commit style comment.

https://chromiumcodereview.appspot.com/10836228/diff/4001/chrome/browser/exte...
File chrome/browser/extensions/extension_service.cc (right):

https://chromiumcodereview.appspot.com/10836228/diff/4001/chrome/browser/exte...
chrome/browser/extensions/extension_service.cc:575: int include_mask =
INCLUDE_ENABLED;
I've a pending CL to change this. FWIW, this can be written as:

int foo = BAR | QUX;

Expand Messages | Collapse Messages