Updated OSX to blacklist multisampling when multiple monitors are connected

Updated OSX to blacklist multisampling when multiple monitors are connected

BUG=237931
R=joi@chromium.org, kbr@chromium.org, zmo@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=203827

[bajones, 2013-05-03 21:18:45]

This CL is dependent on the changes in Issue #14840015 and should not be landed
before it. Doing so may break multisampling blacklisting.

[Ken Russell (switch to Gerrit), 2013-05-06 21:59:03]

https://codereview.chromium.org/14947002/diff/3001/content/browser/gpu/gpu_da...
File content/browser/gpu/gpu_data_manager_impl.cc (right):

https://codereview.chromium.org/14947002/diff/3001/content/browser/gpu/gpu_da...
content/browser/gpu/gpu_data_manager_impl.cc:150: return true;
I thought per our earlier discussion that this was going to be handled
differently -- that instead of blacklisting and unblacklisting the feature, that
GpuDataManagerImpl::UpdateRendererWebPrefs was going to be modified. That way
the change in src/gpu/command_buffer/service/feature_info.cc wouldn't be
necessary; that one is concerning.

[bajones, 2013-05-07 21:48:51]

Multiple monitors no longer affects blacklist, only preferences. This allows
this issue to be landed prior to #14840015 without the risk of breaking
anything, although multisampling would be enabled/disabled based on how many
monitors were connected when the WebGL context was created.

[Ken Russell (switch to Gerrit), 2013-05-08 01:57:59]

This looks good overall, but a couple of minor issues.

https://codereview.chromium.org/14947002/diff/8001/content/browser/gpu/gpu_da...
File content/browser/gpu/gpu_data_manager_impl.h (right):

https://codereview.chromium.org/14947002/diff/8001/content/browser/gpu/gpu_da...
content/browser/gpu/gpu_data_manager_impl.h:176: // Number of displays, to be
used in GPU feature detection
Nit: here and throughout: end with ".".

https://codereview.chromium.org/14947002/diff/8001/content/browser/gpu/gpu_da...
content/browser/gpu/gpu_data_manager_impl.h:177: unsigned int display_count_;
Public fields violate the style guide:
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Access_Control .
Add accessors. Also, this field is uninitialized in the constructor.

[bajones, 2013-05-08 23:49:07]

https://codereview.chromium.org/14947002/diff/8001/content/browser/gpu/gpu_da...
File content/browser/gpu/gpu_data_manager_impl.h (right):

https://codereview.chromium.org/14947002/diff/8001/content/browser/gpu/gpu_da...
content/browser/gpu/gpu_data_manager_impl.h:176: // Number of displays, to be
used in GPU feature detection
On 2013/05/08 01:58:00, kbr wrote:
> Nit: here and throughout: end with ".".

Done.

https://codereview.chromium.org/14947002/diff/8001/content/browser/gpu/gpu_da...
content/browser/gpu/gpu_data_manager_impl.h:177: unsigned int display_count_;
On 2013/05/08 01:58:00, kbr wrote:
> Public fields violate the style guide:
> http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Access_Control
.
> Add accessors. Also, this field is uninitialized in the constructor.

Done.

[Ken Russell (switch to Gerrit), 2013-05-08 23:53:23]

LGTM with a couple of issues addressed.

https://codereview.chromium.org/14947002/diff/12001/content/browser/gpu/gpu_d...
File content/browser/gpu/gpu_data_manager_impl.cc (right):

https://codereview.chromium.org/14947002/diff/12001/content/browser/gpu/gpu_d...
content/browser/gpu/gpu_data_manager_impl.cc:82: return; // This call contains
no information about the display change
End with ".".

https://codereview.chromium.org/14947002/diff/12001/content/browser/gpu/gpu_d...
File content/browser/gpu/gpu_data_manager_impl.h (right):

https://codereview.chromium.org/14947002/diff/12001/content/browser/gpu/gpu_d...
content/browser/gpu/gpu_data_manager_impl.h:299: unsigned int display_count_;
This won't compile on non-Mac platforms. Given the number of #ifdefs that would
be required to make it compile I think it would be easier to just comment that
it's currently used only on OS X, and define it on all platforms.

[bajones, 2013-05-17 21:38:11]

Rebased on the GpuDataManager refactor. Would appreciate a quick review from
zmo@

[Zhenyao Mo, 2013-05-17 22:27:55]

LGTM

https://codereview.chromium.org/14947002/diff/20001/content/browser/gpu/gpu_d...
File content/browser/gpu/gpu_data_manager_impl_private.h (right):

https://codereview.chromium.org/14947002/diff/20001/content/browser/gpu/gpu_d...
content/browser/gpu/gpu_data_manager_impl_private.h:106: unsigned int
GetDisplayCount() const;
FYI: this is actually against code style
accessor functions should be set_display_count() and display_count().

However, considering the pattern we use here (GpuDataManagerImpl and
GpuDataManagerImplPrivate have the same functions), I think the names are OK as
they are.

[commit-bot: I haz the power, 2013-05-18 01:05:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bajones@chromium.org/14947002/20001

[commit-bot: I haz the power, 2013-05-18 01:50:41]

Retried try job too often on linux_chromeos for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...

[commit-bot: I haz the power, 2013-05-20 16:56:26]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bajones@chromium.org/14947002/20001

[commit-bot: I haz the power, 2013-05-20 17:19:43]

Retried try job too often on linux_rel for step(s) interactive_ui_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2013-05-29 21:39:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bajones@chromium.org/14947002/70001

[commit-bot: I haz the power, 2013-05-29 23:01:30]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-05-30 20:00:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bajones@chromium.org/14947002/88001

[commit-bot: I haz the power, 2013-05-30 20:07:18]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[bajones, 2013-05-30 20:36:22]

Previously code was failing to build on Windows bots with the following:

error C2220: warning treated as error - no 'object' file generated
warning C4275: non dll-interface class 'content::GpuDataManagerObserver' used as
base for dll-interface class 'content::RenderProcessHostImpl'

This error can be avoided by adding CONTENT_EXPORT to the
GpuDataManagerObserver, as is done in the latest patch, but I'm not certain
that's the "right" solution.

[bajones, 2013-06-03 17:47:54]

On 2013/05/30 20:36:22, bajones wrote:
> Previously code was failing to build on Windows bots with the following:
> 
> error C2220: warning treated as error - no 'object' file generated
> warning C4275: non dll-interface class 'content::GpuDataManagerObserver' used
as
> base for dll-interface class 'content::RenderProcessHostImpl'
> 
> This error can be avoided by adding CONTENT_EXPORT to the
> GpuDataManagerObserver, as is done in the latest patch, but I'm not certain
> that's the "right" solution.

Ping @joi

[Jói, 2013-06-03 18:00:27]

LGTM for content/public.
On Jun 3, 2013 5:47 PM, <bajones@chromium.org> wrote:

> On 2013/05/30 20:36:22, bajones wrote:
>
>> Previously code was failing to build on Windows bots with the following:
>>
>
>  error C2220: warning treated as error - no 'object' file generated
>> warning C4275: non dll-interface class 'content::**GpuDataManagerObserver'
>> used
>>
> as
>
>> base for dll-interface class 'content::**RenderProcessHostImpl'
>>
>
>  This error can be avoided by adding CONTENT_EXPORT to the
>> GpuDataManagerObserver, as is done in the latest patch, but I'm not
>> certain
>> that's the "right" solution.
>>
>
> Ping @joi
>
>
https://codereview.chromium.**org/14947002/<https://codereview.chromium.org/1...
>

[commit-bot: I haz the power, 2013-06-03 18:26:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bajones@chromium.org/14947002/88001

[bajones, 2013-06-04 03:40:04]

Committed patchset #9 manually as r203827 (presubmit successful).