Fix a bug so GPU blacklist for accelerated 2d canvas is honored.

Fix a bug so GPU blacklist for accelerated 2d canvas is honored.

BUG=97640
TEST=accelerated canvas 2d is truly blocked in linux
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=102400

[Zhenyao Mo, 2011-09-22 18:12:17]

Please review.

[jamesr, 2011-09-22 21:14:36]

Do we need both changes?  I'm still not sure which bits are necessary and which
aren't.

http://codereview.chromium.org/7995011/diff/1/content/browser/renderer_host/b...
File content/browser/renderer_host/browser_render_process_host.cc (right):

http://codereview.chromium.org/7995011/diff/1/content/browser/renderer_host/b...
content/browser/renderer_host/browser_render_process_host.cc:537:
switches::kDisableAccelerated2dCanvas,
what does this line actually do?

[Zhenyao Mo, 2011-09-22 21:32:01]

On 2011/09/22 21:14:36, jamesr wrote:
> Do we need both changes?  I'm still not sure which bits are necessary and
which
> aren't.
> 
>
http://codereview.chromium.org/7995011/diff/1/content/browser/renderer_host/b...
> File content/browser/renderer_host/browser_render_process_host.cc (right):
> 
>
http://codereview.chromium.org/7995011/diff/1/content/browser/renderer_host/b...
> content/browser/renderer_host/browser_render_process_host.cc:537:
> switches::kDisableAccelerated2dCanvas,
> what does this line actually do?

We only need the preference to make this work.  But I want to make the
information consistent, because having some switches passed down but not others
could lead to bugs.  Ultimately, I think we don't need the switches, so we
should take all of them out in another cleanup CL.

What do you think?

[jamesr, 2011-09-22 21:40:02]

On 2011/09/22 21:32:01, Zhenyao Mo wrote:
> 
> We only need the preference to make this work.  But I want to make the
> information consistent, because having some switches passed down but not
others
> could lead to bugs.  Ultimately, I think we don't need the switches, so we
> should take all of them out in another cleanup CL.
> 
> What do you think?

I think it's really scary to make changes that don't do anything, or that we
aren't fully confident about.  There should be on mechanism by which
blacklisting works and that afaik that doesn't involve flags.  If there is
something else that needs this flags change, then we should figure out what that
is, file a new bug, and post a new patch that does that.  I don't like changing
both things at once if we don't think they are needed for this bug, especially
since we're going to want to merge this to m15.

[Zhenyao Mo, 2011-09-22 21:48:17]

OK, removed the extra change.  Please review again.

[jamesr, 2011-09-22 21:52:14]

lgtm