Generalzie GpuBlacklist to GpuControlList.

Generalzie GpuBlacklist to GpuControlList.

This CL refactor the GpuBlacklist so it can be configured to handle a different set of features.

So it can handle not only blacklist, but dual gpu switching, and driver bugs.

I will add the code to truly manage driver bug workarounds in the next CL.

BUG=222857
TEST=content_unittests
TBR=asargent
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=189889

[Zhenyao Mo, 2013-03-21 20:08:13]

gman: please take a look.

(This is a huge CL, but the only meaningful change is I change GpuBlacklist to
GpuControlList, and make it configurable so it can recognize a different set of
features, then make a GpuBlacklist, GpuSwitchingList, GpuDriverBugList out of
it.)

[greggman, 2013-03-22 00:30:51]

lgtm

https://codereview.chromium.org/12976004/diff/11002/content/browser/gpu/gpu_c...
File content/browser/gpu/gpu_control_list.h (right):

https://codereview.chromium.org/12976004/diff/11002/content/browser/gpu/gpu_c...
content/browser/gpu/gpu_control_list.h:55: int MakeDecision(
nit: I hope this isn't bikeshedding but the description above doesn't really
seem to match a function called "MakeDecision". What decision is being made?

https://codereview.chromium.org/12976004/diff/11002/content/browser/gpu/gpu_c...
content/browser/gpu/gpu_control_list.h:58: // Collects the active entries from
the last MakeBlacklistDecision() call.
s/MakeBlacklistDecision/MakeDecision/?

https://codereview.chromium.org/12976004/diff/11002/content/browser/gpu/gpu_c...
content/browser/gpu/gpu_control_list.h:81: // Check if we needs more gpu info to
make the decisions.
s/needs/need/

https://codereview.chromium.org/12976004/diff/11002/content/browser/gpu/gpu_c...
content/browser/gpu/gpu_control_list.h:113: class VersionInfo {
It seems like these classes should have unit tests.

put "friend class VersionInfoTest;" at line 96

Then in unit tests VersionInfoTest can make instances of
GPUControlList::VersionInfoTest.

https://codereview.chromium.org/12976004/diff/11002/content/browser/gpu/gpu_c...
content/browser/gpu/gpu_control_list.h:164: class OsInfo {
same as above. Seems like a few simple tests would be good

https://codereview.chromium.org/12976004/diff/11002/content/browser/gpu/gpu_c...
content/browser/gpu/gpu_control_list.h:188: class StringInfo {
same as above. Seems like a few simple tests would be good

https://codereview.chromium.org/12976004/diff/11002/content/browser/gpu/gpu_c...
content/browser/gpu/gpu_control_list.h:214: class FloatInfo {
same as above. Seems like a few simple tests would be good

https://codereview.chromium.org/12976004/diff/11002/content/browser/gpu/gpu_c...
content/browser/gpu/gpu_control_list.h:232: class IntInfo {
same as above. Seems like a few simple tests would be good

https://codereview.chromium.org/12976004/diff/11002/content/browser/gpu/gpu_c...
content/browser/gpu/gpu_control_list.h:250: class MachineModelInfo {
same as above. Seems like a few simple tests would be good

https://codereview.chromium.org/12976004/diff/11002/content/browser/gpu/gpu_c...
content/browser/gpu/gpu_control_list.h:275: class GpuControlListEntry : public
base::RefCounted<GpuControlListEntry> {
same as above. Seems like a few simple tests would be good

https://codereview.chromium.org/12976004/diff/11002/content/browser/gpu/gpu_s...
File content/browser/gpu/gpu_switching_list_unittest.cc (right):

https://codereview.chromium.org/12976004/diff/11002/content/browser/gpu/gpu_s...
content/browser/gpu/gpu_switching_list_unittest.cc:75: const std::string json =
Just FYI,  I believe you can do this

   #define LONG_STRING_CONST(...)  #__VA_ARGS__

   const std::stirng json = LONG_STRING_CONST(
      {
        "name": "gpu switch list",
        "version": "0.1",
        "entries": [
           ..etc...
        ]
     }
    );

Here and in other files. Might be easier in the future than having to escape
every quote and add a \n all over the place.

[Zhenyao Mo, 2013-03-22 20:00:04]

Committed patchset #2 manually as r189889 (presubmit successful).

[Zhenyao Mo, 2013-03-22 20:00:10]

https://codereview.chromium.org/12976004/diff/11002/content/browser/gpu/gpu_c...
File content/browser/gpu/gpu_control_list.h (right):

https://codereview.chromium.org/12976004/diff/11002/content/browser/gpu/gpu_c...
content/browser/gpu/gpu_control_list.h:55: int MakeDecision(
On 2013/03/22 00:30:51, greggman wrote:
> nit: I hope this isn't bikeshedding but the description above doesn't really
> seem to match a function called "MakeDecision". What decision is being made?

Revised to make it clearer.

https://codereview.chromium.org/12976004/diff/11002/content/browser/gpu/gpu_c...
content/browser/gpu/gpu_control_list.h:58: // Collects the active entries from
the last MakeBlacklistDecision() call.
On 2013/03/22 00:30:51, greggman wrote:
> s/MakeBlacklistDecision/MakeDecision/?

Done.

https://codereview.chromium.org/12976004/diff/11002/content/browser/gpu/gpu_c...
content/browser/gpu/gpu_control_list.h:81: // Check if we needs more gpu info to
make the decisions.
On 2013/03/22 00:30:51, greggman wrote:
> s/needs/need/

Done.

https://codereview.chromium.org/12976004/diff/11002/content/browser/gpu/gpu_c...
content/browser/gpu/gpu_control_list.h:113: class VersionInfo {
On 2013/03/22 00:30:51, greggman wrote:
> It seems like these classes should have unit tests.
> 
> put "friend class VersionInfoTest;" at line 96
> 
> Then in unit tests VersionInfoTest can make instances of
> GPUControlList::VersionInfoTest.

Sure.  Will add tests for this class in a follow-up CL.  Same for the other
classes.

https://codereview.chromium.org/12976004/diff/11002/content/browser/gpu/gpu_s...
File content/browser/gpu/gpu_switching_list_unittest.cc (right):

https://codereview.chromium.org/12976004/diff/11002/content/browser/gpu/gpu_s...
content/browser/gpu/gpu_switching_list_unittest.cc:75: const std::string json =
On 2013/03/22 00:30:51, greggman wrote:
> Just FYI,  I believe you can do this
> 
>    #define LONG_STRING_CONST(...)  #__VA_ARGS__
> 
>    const std::stirng json = LONG_STRING_CONST(
>       {
>         "name": "gpu switch list",
>         "version": "0.1",
>         "entries": [
>            ..etc...
>         ]
>      }
>     );
> 
> Here and in other files. Might be easier in the future than having to escape
> every quote and add a \n all over the place.

Wow, this is really awesome! Why I haven't heard of this before?  :)