Add a "disabled" field for GPU Blacklist entries.

Add a "disabled" field for GPU Blacklist entries.

BUG=97258
TEST=unittest
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=102410

[Zhenyao Mo, 2011-09-21 20:29:00]

Please review.

[vangelis, 2011-09-21 21:27:17]

http://codereview.chromium.org/7982034/diff/5001/content/browser/gpu/gpu_blac...
File content/browser/gpu/gpu_blacklist.cc (right):

http://codereview.chromium.org/7982034/diff/5001/content/browser/gpu/gpu_blac...
content/browser/gpu/gpu_blacklist.cc:771:
active_disabled_entries_.push_back(blacklist_[i]);
nit: active_disabled_ is somewhat contradicting.  How about using
disabled_entries_ instead?

http://codereview.chromium.org/7982034/diff/5001/content/browser/gpu/gpu_blac...
content/browser/gpu/gpu_blacklist.cc:773:
flags.Combine(blacklist_[i]->GetGpuFeatureFlags());
Wouldn't it be simpler to add a disabled_ field in each entry instead of
creating a new array of entries for disabled features?  That way,
GetGpuFeatureFlagEntries could take an additional boolean parameter on whether
to return disabled entries or not.

http://codereview.chromium.org/7982034/diff/5001/content/browser/gpu/gpu_blac...
content/browser/gpu/gpu_blacklist.cc:791: void
GpuBlacklist::GetGpuFeatureFlagDisabledEntries(
Instead of replicating the code here, this function could be consolidated with
the one above it by passing the entries array (disabled or active) as a
parameter.

http://codereview.chromium.org/7982034/diff/5001/content/browser/gpu/gpu_blac...
File content/browser/gpu/gpu_blacklist.h (right):

http://codereview.chromium.org/7982034/diff/5001/content/browser/gpu/gpu_blac...
content/browser/gpu/gpu_blacklist.h:72: // DetermineGpuFeatureFlags() call if
not disabled.
I don't understand this comment...

[Zhenyao Mo, 2011-09-22 17:10:39]

Revised, please review again.

http://codereview.chromium.org/7982034/diff/5001/content/browser/gpu/gpu_blac...
File content/browser/gpu/gpu_blacklist.cc (right):

http://codereview.chromium.org/7982034/diff/5001/content/browser/gpu/gpu_blac...
content/browser/gpu/gpu_blacklist.cc:771:
active_disabled_entries_.push_back(blacklist_[i]);
On 2011/09/21 21:27:17, vangelis wrote:
> nit: active_disabled_ is somewhat contradicting.  How about using
> disabled_entries_ instead? 

removed active_disabled_emtries_

http://codereview.chromium.org/7982034/diff/5001/content/browser/gpu/gpu_blac...
content/browser/gpu/gpu_blacklist.cc:773:
flags.Combine(blacklist_[i]->GetGpuFeatureFlags());
On 2011/09/21 21:27:17, vangelis wrote:
> Wouldn't it be simpler to add a disabled_ field in each entry instead of
> creating a new array of entries for disabled features?  That way,
> GetGpuFeatureFlagEntries could take an additional boolean parameter on whether
> to return disabled entries or not.

Done.

http://codereview.chromium.org/7982034/diff/5001/content/browser/gpu/gpu_blac...
content/browser/gpu/gpu_blacklist.cc:791: void
GpuBlacklist::GetGpuFeatureFlagDisabledEntries(
On 2011/09/21 21:27:17, vangelis wrote:
> Instead of replicating the code here, this function could be consolidated with
> the one above it by passing the entries array (disabled or active) as a
> parameter. 

Done.  I used a consolidated array to store the entries and use the disabled()
entry field and the passed in boolean parameter to decide if an entry should be
added or not.

http://codereview.chromium.org/7982034/diff/5001/content/browser/gpu/gpu_blac...
File content/browser/gpu/gpu_blacklist.h (right):

http://codereview.chromium.org/7982034/diff/5001/content/browser/gpu/gpu_blac...
content/browser/gpu/gpu_blacklist.h:72: // DetermineGpuFeatureFlags() call if
not disabled.
On 2011/09/21 21:27:17, vangelis wrote:
> I don't understand this comment...

This comment and its function are removed based on review advice.

[vangelis, 2011-09-23 00:35:36]

LGTM