Enable GPU control lists in tests on Windows.

Enable GPU control lists in tests on Windows (other OSes will follow, but this is high priority for Windows and can't afford to be blocked on other OS' failures anymore).

Reland of https://codereview.chromium.org/23534006/ (see that and the bug for more details).

Also adding INFO-level logging to spell out each control list rule being applied on the bots (makes debugging and understanding failures much easier).

R=piman@chromium.org
TBR=jcivelli, piman, zmo
BUG=295799
Previously Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=224810
Reverted: https://src.chromium.org/viewvc/chrome?view=rev&revision=224845

Previously Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=227977
Reverted: https://src.chromium.org/viewvc/chrome?view=rev&revision=228014

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

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

[gab, 2013-09-20 20:10:05]

TBR: zmo, piman, jcivelli

FYI, this is a reland of https://codereview.chromium.org/23534006/; I added some
more descriptive logging as to which control list is take which actions.

I will wait for https://codereview.chromium.org/23726050/ to land before landing
this.

Cheers!
Gab

[piman, 2013-09-20 21:27:35]

https://codereview.chromium.org/23703017/diff/5001/chrome/browser/gpu/chrome_...
File chrome/browser/gpu/chrome_gpu_util.cc (left):

https://codereview.chromium.org/23703017/diff/5001/chrome/browser/gpu/chrome_...
chrome/browser/gpu/chrome_gpu_util.cc:47: #endif
We don't want to run GPU field trials on Chrome OS. Why are you removing this?

[gab, 2013-09-21 03:40:09]

https://codereview.chromium.org/23703017/diff/5001/chrome/browser/gpu/chrome_...
File chrome/browser/gpu/chrome_gpu_util.cc (left):

https://codereview.chromium.org/23703017/diff/5001/chrome/browser/gpu/chrome_...
chrome/browser/gpu/chrome_gpu_util.cc:47: #endif
On 2013/09/20 21:27:35, piman wrote:
> We don't want to run GPU field trials on Chrome OS. Why are you removing this?

Right, but we never make it to the field trial code on OS_CHROMEOS because we
force an early return to true in compositor_util if USE_AURA:
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/gp...

I can leave this here if it makes you feel safer, I cleaned it up because it's
never reached and I'm trying to make this code smaller and smaller as we want to
get rid of it (statically enabling FCM/TCM on platforms where they are currently
already on at 100% via Finch).

[piman, 2013-09-23 21:07:29]

lgtm

[gab, 2013-09-23 21:07:52]

https://codereview.chromium.org/23703017/diff/5001/chrome/browser/gpu/chrome_...
File chrome/browser/gpu/chrome_gpu_util.cc (left):

https://codereview.chromium.org/23703017/diff/5001/chrome/browser/gpu/chrome_...
chrome/browser/gpu/chrome_gpu_util.cc:47: #endif
On 2013/09/21 03:40:09, gab wrote:
> On 2013/09/20 21:27:35, piman wrote:
> > We don't want to run GPU field trials on Chrome OS. Why are you removing
this?
> 
> Right, but we never make it to the field trial code on OS_CHROMEOS because we
> force an early return to true in compositor_util if USE_AURA:
>
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/gp...
> 
> I can leave this here if it makes you feel safer, I cleaned it up because it's
> never reached and I'm trying to make this code smaller and smaller as we want
to
> get rid of it (statically enabling FCM/TCM on platforms where they are
currently
> already on at 100% via Finch).

As per offline discussion, I'll leave the OS_CHROMEOS bit in although it will
never be hit; this is a slightly safer approach.

[gab, 2013-09-23 22:22:49]

Will go ahead and commit with no CQ as known failures from this aren't caught by
CQ (ref. previous attempts @ https://codereview.chromium.org/23534006/).

Still launching try jobs for cross-reference if failure there is...

Previous XP failures were due to webview+software compositor: being worked on by
jbauman; tests have been disabled for now in http://crrev.com/224807 so this is
worth another shot **crossing-fingers**!

Cheers,
Gab

[gab, 2013-09-23 22:23:54]

Committed patchset #6 manually as r224810 (presubmit successful).

[commit-bot: I haz the power, 2013-10-07 13:53:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gab@chromium.org/23703017/90001

[commit-bot: I haz the power, 2013-10-07 14:37:01]

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-10-07 17:29:06]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gab@chromium.org/23703017/90001

[commit-bot: I haz the power, 2013-10-07 18:45:10]

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

[commit-bot: I haz the power, 2013-10-07 19:02:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gab@chromium.org/23703017/90001

[commit-bot: I haz the power, 2013-10-07 20:46:33]

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

[commit-bot: I haz the power, 2013-10-07 21:14:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gab@chromium.org/23703017/90001

[commit-bot: I haz the power, 2013-10-07 22:39:08]

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

[commit-bot: I haz the power, 2013-10-07 22:51:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gab@chromium.org/23703017/90001

[commit-bot: I haz the power, 2013-10-08 00:21:00]

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

[commit-bot: I haz the power, 2013-10-08 14:31:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gab@chromium.org/23703017/90001

[commit-bot: I haz the power, 2013-10-08 15:53:16]

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

[commit-bot: I haz the power, 2013-10-08 20:18:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gab@chromium.org/23703017/142001

[commit-bot: I haz the power, 2013-10-08 21:03:45]

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

[commit-bot: I haz the power, 2013-10-08 21:07:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gab@chromium.org/23703017/142001

[commit-bot: I haz the power, 2013-10-08 21:07:14]

Failed to apply patch for gpu/config/software_rendering_list_json.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file gpu/config/software_rendering_list_json.cc
  Hunk #1 FAILED at 18.
  Hunk #2 succeeded at 1140 with fuzz 2 (offset 11 lines).
  Hunk #3 succeeded at 1162 (offset 11 lines).
  1 out of 3 hunks FAILED -- saving rejects to file
gpu/config/software_rendering_list_json.cc.rej

Patch:       gpu/config/software_rendering_list_json.cc
Index: gpu/config/software_rendering_list_json.cc
diff --git a/gpu/config/software_rendering_list_json.cc
b/gpu/config/software_rendering_list_json.cc
index
5256740334385c45d473d88355f934a79e2b6cc1..bd4f1ce1bd1d5fedcc223b84fb79df469c07cfe1
100644
--- a/gpu/config/software_rendering_list_json.cc
+++ b/gpu/config/software_rendering_list_json.cc
@@ -18,7 +18,7 @@ const char kSoftwareRenderingListJson[] = LONG_STRING_CONST(
 {
   "name": "software rendering list",
   // Please update the version number whenever you change this file.
-  "version": "6.11",
+  "version": "6.12",
   "entries": [
     {
       "id": 1,
@@ -1129,6 +1129,21 @@ LONG_STRING_CONST(
       "features": [
         "accelerated_video_decode"
       ]
+    },
+    {
+      "id": 79,
+      "description": "Disable force compositing mode on all Windows versions
prior to Vista.",
+      "cr_bugs": [273920],
+      "os": {
+        "type": "win",
+        "version": {
+          "op": "<",
+          "value": "6.0"
+        }
+      },
+      "features": [
+        "force_compositing_mode"
+      ]
     }
   ]
 }
@@ -1136,4 +1151,3 @@ LONG_STRING_CONST(
 );  // LONG_STRING_CONST macro
 
 }  // namespace gpu
-

[Vangelis Kokkevis, 2013-10-08 23:09:47]

https://codereview.chromium.org/23703017/diff/142001/content/browser/gpu/gpu_...
File content/browser/gpu/gpu_data_manager_impl_private.cc (right):

https://codereview.chromium.org/23703017/diff/142001/content/browser/gpu/gpu_...
content/browser/gpu/gpu_data_manager_impl_private.cc:564:
gfx::kGLImplementationOSMesaName;  // Bypass rule #74.
I think that it would be better to modify the blacklist instead of hardcoding
these in the code. For example, we can add exceptions to these entries for
driver_vendor == osmesa since that's a configuration that will only be used in
testing.

[commit-bot: I haz the power, 2013-10-09 17:19:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gab@chromium.org/23703017/171001

[gab, 2013-10-09 18:13:01]

Vangelis, PTAL.

Thanks!
Gab

https://codereview.chromium.org/23703017/diff/142001/content/browser/gpu/gpu_...
File content/browser/gpu/gpu_data_manager_impl_private.cc (right):

https://codereview.chromium.org/23703017/diff/142001/content/browser/gpu/gpu_...
content/browser/gpu/gpu_data_manager_impl_private.cc:564:
gfx::kGLImplementationOSMesaName;  // Bypass rule #74.
On 2013/10/08 23:09:48, Vangelis Kokkevis wrote:
> I think that it would be better to modify the blacklist instead of hardcoding
> these in the code. For example, we can add exceptions to these entries for
> driver_vendor == osmesa since that's a configuration that will only be used in
> testing. 

The problem is that if we leave gpu_info empty, these are simply undefined, and
the blacklist is such that if it needs a value that couldn't be resolved in
gpu_info, it assumes a match (i.e., it's conservative):
https://code.google.com/p/chromium/codesearch#chromium/src/gpu/config/gpu_con...

But I'm fine with only hardcoding driver_vendor and changing the rules below to
have exceptions for the driver_vendor==osmesa instead of hardcoding date and
version as well. Is that what you meant?

Or would you prefer not hardcoding driver_vendor and relying on
vendor_id/device_id = 0xffff?

[vangelis, 2013-10-09 18:49:14]

On 2013/10/09 18:13:01, gab wrote:
> Vangelis, PTAL.
> 
> Thanks!
> Gab
> 
>
https://codereview.chromium.org/23703017/diff/142001/content/browser/gpu/gpu_...
> File content/browser/gpu/gpu_data_manager_impl_private.cc (right):
> 
>
https://codereview.chromium.org/23703017/diff/142001/content/browser/gpu/gpu_...
> content/browser/gpu/gpu_data_manager_impl_private.cc:564:
> gfx::kGLImplementationOSMesaName;  // Bypass rule #74.
> On 2013/10/08 23:09:48, Vangelis Kokkevis wrote:
> > I think that it would be better to modify the blacklist instead of
hardcoding
> > these in the code. For example, we can add exceptions to these entries for
> > driver_vendor == osmesa since that's a configuration that will only be used
in
> > testing. 
> 
> The problem is that if we leave gpu_info empty, these are simply undefined,
and
> the blacklist is such that if it needs a value that couldn't be resolved in
> gpu_info, it assumes a match (i.e., it's conservative):
>
https://code.google.com/p/chromium/codesearch#chromium/src/gpu/config/gpu_con...
> 
> But I'm fine with only hardcoding driver_vendor and changing the rules below
to
> have exceptions for the driver_vendor==osmesa instead of hardcoding date and
> version as well. Is that what you meant?

I like the idea of hardcoding the driver_vendor to osmesa. Your latest changes
LGTM (except for the blacklist entry).
 
> 
> Or would you prefer not hardcoding driver_vendor and relying on
> vendor_id/device_id = 0xffff?

[commit-bot: I haz the power, 2013-10-09 19:01:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gab@chromium.org/23703017/205001

[commit-bot: I haz the power, 2013-10-10 00:37:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gab@chromium.org/23703017/205001

[commit-bot: I haz the power, 2013-10-10 00:37:15]

Failed to apply patch for gpu/config/software_rendering_list_json.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file gpu/config/software_rendering_list_json.cc
  Hunk #1 FAILED at 18.
  Hunk #7 succeeded at 1149 with fuzz 2 (offset -11 lines).
  Hunk #8 succeeded at 1171 (offset -11 lines).
  1 out of 8 hunks FAILED -- saving rejects to file
gpu/config/software_rendering_list_json.cc.rej

Patch:       gpu/config/software_rendering_list_json.cc
Index: gpu/config/software_rendering_list_json.cc
diff --git a/gpu/config/software_rendering_list_json.cc
b/gpu/config/software_rendering_list_json.cc
index
94587d8c1537eedde0af042f9ffa4905766d0ee5..a494eca4dd9bb48f7cda384edeef14387a59cf93
100644
--- a/gpu/config/software_rendering_list_json.cc
+++ b/gpu/config/software_rendering_list_json.cc
@@ -18,7 +18,7 @@ const char kSoftwareRenderingListJson[] = LONG_STRING_CONST(
 {
   "name": "software rendering list",
   // Please update the version number whenever you change this file.
-  "version": "6.12",
+  "version": "6.13",
   "entries": [
     {
       "id": 1,
@@ -138,6 +138,12 @@ const char kSoftwareRenderingListJson[] =
LONG_STRING_CONST(
             "op": ">=",
             "value": "7.15.10.1624"
           }
+        },
+        {
+          "driver_vendor": {
+            "op": "=",
+            "value": "osmesa"
+          }
         }
       ],
       "features": [
@@ -329,6 +335,14 @@ const char kSoftwareRenderingListJson[] =
LONG_STRING_CONST(
         "op": "<",
         "value": "7.11"
       },
+      "exceptions": [
+        {
+          "driver_vendor": {
+            "op": "=",
+            "value": "osmesa"
+          }
+        }
+      ],
       "features": [
         "all"
       ]
@@ -761,6 +775,12 @@ const char kSoftwareRenderingListJson[] =
LONG_STRING_CONST(
             "op": ">=",
             "value": "7.15.10.1624"
           }
+        },
+        {
+          "driver_vendor": {
+            "op": "=",
+            "value": "osmesa"
+          }
         }
       ],
       "features": [
@@ -1064,6 +1084,8 @@ const char kSoftwareRenderingListJson[] =
LONG_STRING_CONST(
         "all"
       ]
     },
+)  // String split to avoid MSVC char limit.
+LONG_STRING_CONST(
     {
       "id": 75,
       "description": "Texture sharing not supported on AMD Switchable GPUs due
to driver issues",
@@ -1076,8 +1098,6 @@ const char kSoftwareRenderingListJson[] =
LONG_STRING_CONST(
         "texture_sharing"
       ]
     },
-)  // String split to avoid MSVC char limit.
-LONG_STRING_CONST(
     {
       "id": 76,
       "description": "WebGL is disabled on Android unless GPU reset
notification is supported",
@@ -1140,6 +1160,21 @@ LONG_STRING_CONST(
       "features": [
         "texture_sharing"
       ]
+    },
+    {
+      "id": 80,
+      "description": "Disable force compositing mode on all Windows versions
prior to Vista.",
+      "cr_bugs": [273920],
+      "os": {
+        "type": "win",
+        "version": {
+          "op": "<",
+          "value": "6.0"
+        }
+      },
+      "features": [
+        "force_compositing_mode"
+      ]
     }
   ]
 }
@@ -1147,4 +1182,3 @@ LONG_STRING_CONST(
 );  // LONG_STRING_CONST macro
 
 }  // namespace gpu
-

[commit-bot: I haz the power, 2013-10-10 09:05:56]

No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer or
a lowly provisional committer, _not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[commit-bot: I haz the power, 2013-10-10 14:58:41]

No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer or
a lowly provisional committer, _not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[commit-bot: I haz the power, 2013-10-10 17:54:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gab@chromium.org/23703017/225001

[commit-bot: I haz the power, 2013-10-10 19:59:58]

Change committed as 227977

[gab, 2013-10-10 22:50:14]

On 2013/10/10 19:59:58, I haz the power (commit-bot) wrote:
> Change committed as 227977

The saga continues. FTR, the last commit was patch set 15.

[commit-bot: I haz the power, 2013-10-12 16:23:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gab@chromium.org/23703017/250001

[commit-bot: I haz the power, 2013-10-12 19:10:37]

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

[commit-bot: I haz the power, 2013-10-13 07:46:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gab@chromium.org/23703017/293001

[commit-bot: I haz the power, 2013-10-13 07:46:20]

Failed to apply patch for gpu/config/software_rendering_list_json.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file gpu/config/software_rendering_list_json.cc
  Hunk #1 FAILED at 18.
  Hunk #7 succeeded at 1154 with fuzz 2 (offset -11 lines).
  Hunk #8 succeeded at 1176 (offset -11 lines).
  1 out of 8 hunks FAILED -- saving rejects to file
gpu/config/software_rendering_list_json.cc.rej

Patch:       gpu/config/software_rendering_list_json.cc
Index: gpu/config/software_rendering_list_json.cc
diff --git a/gpu/config/software_rendering_list_json.cc
b/gpu/config/software_rendering_list_json.cc
index
498d867138cd9331954f6dfc2c9e18a4e2b32dc2..063ba7a0ee0eea342a5edb1d6e68df5bb844b1b4
100644
--- a/gpu/config/software_rendering_list_json.cc
+++ b/gpu/config/software_rendering_list_json.cc
@@ -18,7 +18,7 @@ const char kSoftwareRenderingListJson[] = LONG_STRING_CONST(
 {
   "name": "software rendering list",
   // Please update the version number whenever you change this file.
-  "version": "6.12",
+  "version": "6.13",
   "entries": [
     {
       "id": 1,
@@ -138,6 +138,12 @@ const char kSoftwareRenderingListJson[] =
LONG_STRING_CONST(
             "op": ">=",
             "value": "7.15.10.1624"
           }
+        },
+        {
+          "driver_vendor": {
+            "op": "=",
+            "value": "osmesa"
+          }
         }
       ],
       "features": [
@@ -329,6 +335,14 @@ const char kSoftwareRenderingListJson[] =
LONG_STRING_CONST(
         "op": "<",
         "value": "7.11"
       },
+      "exceptions": [
+        {
+          "driver_vendor": {
+            "op": "=",
+            "value": "osmesa"
+          }
+        }
+      ],
       "features": [
         "all"
       ]
@@ -766,6 +780,12 @@ const char kSoftwareRenderingListJson[] =
LONG_STRING_CONST(
             "op": ">=",
             "value": "7.15.10.1624"
           }
+        },
+        {
+          "driver_vendor": {
+            "op": "=",
+            "value": "osmesa"
+          }
         }
       ],
       "features": [
@@ -1069,6 +1089,8 @@ const char kSoftwareRenderingListJson[] =
LONG_STRING_CONST(
         "all"
       ]
     },
+)  // String split to avoid MSVC char limit.
+LONG_STRING_CONST(
     {
       "id": 75,
       "description": "Texture sharing not supported on AMD Switchable GPUs due
to driver issues",
@@ -1081,8 +1103,6 @@ const char kSoftwareRenderingListJson[] =
LONG_STRING_CONST(
         "texture_sharing"
       ]
     },
-)  // String split to avoid MSVC char limit.
-LONG_STRING_CONST(
     {
       "id": 76,
       "description": "WebGL is disabled on Android unless GPU reset
notification is supported",
@@ -1145,6 +1165,21 @@ LONG_STRING_CONST(
       "features": [
         "texture_sharing"
       ]
+    },
+    {
+      "id": 80,
+      "description": "Disable force compositing mode on all Windows versions
prior to Vista.",
+      "cr_bugs": [273920],
+      "os": {
+        "type": "win",
+        "version": {
+          "op": "<",
+          "value": "6.0"
+        }
+      },
+      "features": [
+        "force_compositing_mode"
+      ]
     }
   ]
 }
@@ -1152,4 +1187,3 @@ LONG_STRING_CONST(
 );  // LONG_STRING_CONST macro
 
 }  // namespace gpu
-

[commit-bot: I haz the power, 2013-10-13 07:50:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gab@chromium.org/23703017/294012

[gab, 2013-10-13 17:43:47]

_gpu bots failures are the same as the ones currently on the waterfall, e.g.,
http://build.chromium.org/p/chromium.gpu/builders/Win7%20Release%20%28NVIDIA%...

Other failures, see details on https://codereview.chromium.org/26906004/ have
been resolved (most failures were caused by software compositing and those tests
were thus disabled temporarily when in software compositing mode).

Recommitting!
Gab

[commit-bot: I haz the power, 2013-10-13 17:44:06]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gab@chromium.org/23703017/294012

[commit-bot: I haz the power, 2013-10-13 18:12:16]

Change committed as 228401

[tapted, 2013-10-14 04:25:02]

On 2013/10/13 18:12:16, I haz the power (commit-bot) wrote:
> Change committed as 228401

This might be the cause of a persistent TestLostContext failure on WinXP and
Vista waterfall bots (Win7 seems happy). However that test was also added fairly
recently by piman, in http://crrev.com/226610 . Don't know if I have the heart
to revert this poor CL.. but now might be a good time while the tree is quiet..

Builds:

http://build.chromium.org/p/chromium.win/builders/XP%20Tests%20%281%29/builds...
http://build.chromium.org/p/chromium.win/builders/XP%20Tests%20%28dbg%29%282%...
http://build.chromium.org/p/chromium.win/builders/Vista%20Tests%20%281%29/bui...

[tapted, 2013-10-14 08:10:26]

On 2013/10/14 04:25:02, tapted wrote:
> On 2013/10/13 18:12:16, I haz the power (commit-bot) wrote:
> > Change committed as 228401
> 
> This might be the cause of a persistent TestLostContext failure on WinXP and
> Vista waterfall bots (Win7 seems happy). However that test was also added
fairly
> recently by piman, in http://crrev.com/226610 . Don't know if I have the heart
> to revert this poor CL.. but now might be a good time while the tree is
quiet..

update: Reverted in http://crrev.com/228431 (sorry!)
ImageTransportFactoryBrowserTest.TestLostContext went green afterwards.

[gab, 2013-10-14 21:23:57]

On 2013/10/14 08:10:26, tapted wrote:
> On 2013/10/14 04:25:02, tapted wrote:
> > On 2013/10/13 18:12:16, I haz the power (commit-bot) wrote:
> > > Change committed as 228401
> > 
> > This might be the cause of a persistent TestLostContext failure on WinXP and
> > Vista waterfall bots (Win7 seems happy). However that test was also added
> fairly
> > recently by piman, in http://crrev.com/226610 . Don't know if I have the
heart
> > to revert this poor CL.. but now might be a good time while the tree is
> quiet..
> 
> update: Reverted in http://crrev.com/228431 (sorry!)
> ImageTransportFactoryBrowserTest.TestLostContext went green afterwards.

Thanks, disabled this test for software compositing mode in
http://crrev.com/228492

Trying this one again!!

[commit-bot: I haz the power, 2013-10-14 21:24:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gab@chromium.org/23703017/294012

[commit-bot: I haz the power, 2013-10-14 21:26:43]

Change committed as 228530

[eroman, 2013-10-15 00:39:26]

There is a failure on the bot, which I think is related to this change:

http://build.chromium.org/p/chromium.win/buildstatus?builder=Win%20Aura%20Tes...

TabDragging/DetachToBrowserTabDragControllerTest.DragAll/0: 
[3976:3328:1014/171016:2072442:ERROR:gpu_info_collector_win.cc(102)] Can't
retrieve a valid WinSAT assessment.
[3976:3328:1014/171016:2072520:INFO:gpu_control_list.cc(1038)] Control list
match for rule #12 in gpu_blacklist.
[3976:3328:1014/171016:2072520:INFO:gpu_control_list.cc(1038)] Control list
match for rule #55 in gpu_blacklist.
[3976:3328:1014/171016:2072520:INFO:gpu_control_list.cc(1038)] Control list
match for rule #68 in gpu_blacklist.
[3976:3328:1014/171016:2072520:INFO:gpu_control_list.cc(1038)] Control list
match for rule #74 in gpu_blacklist.
[3976:3328:1014/171016:2072520:INFO:gpu_control_list.cc(1038)] Control list
match for rule #3 in gpu_driver_bug_list.
[3976:3328:1014/171016:2072520:INFO:gpu_control_list.cc(1038)] Control list
match for rule #4 in gpu_driver_bug_list.
[3976:3328:1014/171016:2072520:INFO:gpu_control_list.cc(1038)] Control list
match for rule #17 in gpu_driver_bug_list.
[3976:3328:1014/171017:2073097:ERROR:chrome_views_delegate.cc(158)] NOT
IMPLEMENTED
[3976:3328:1014/171017:2073097:ERROR:desktop_root_window_host_win.cc(694)] NOT
IMPLEMENTED
[3976:3328:1014/171047:2103158:ERROR:desktop_root_window_host_win.cc(694)] NOT
IMPLEMENTED
[295/326] TabDragging/DetachToBrowserTabDragControllerTest.DragAll/0 (TIMED OUT)
Note: Google Test filter =
TabDragging/DetachToBrowserTabDragControllerTest.DragAllToSeparateWindow/0
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from TabDragging/DetachToBrowserTabDragControllerTest


Do you think it is related to this change? (Will tentatively roll back this
change if I don't come up with better ideas).

Thanks

[gab, 2013-10-15 01:35:54]

On 2013/10/15 00:39:26, eroman wrote:
> There is a failure on the bot, which I think is related to this change:
> 
>
http://build.chromium.org/p/chromium.win/buildstatus?builder=Win%2520Aura%252...
> 
> TabDragging/DetachToBrowserTabDragControllerTest.DragAll/0: 
> [3976:3328:1014/171016:2072442:ERROR:gpu_info_collector_win.cc(102)] Can't
> retrieve a valid WinSAT assessment.
> [3976:3328:1014/171016:2072520:INFO:gpu_control_list.cc(1038)] Control list
> match for rule #12 in gpu_blacklist.
> [3976:3328:1014/171016:2072520:INFO:gpu_control_list.cc(1038)] Control list
> match for rule #55 in gpu_blacklist.
> [3976:3328:1014/171016:2072520:INFO:gpu_control_list.cc(1038)] Control list
> match for rule #68 in gpu_blacklist.
> [3976:3328:1014/171016:2072520:INFO:gpu_control_list.cc(1038)] Control list
> match for rule #74 in gpu_blacklist.
> [3976:3328:1014/171016:2072520:INFO:gpu_control_list.cc(1038)] Control list
> match for rule #3 in gpu_driver_bug_list.
> [3976:3328:1014/171016:2072520:INFO:gpu_control_list.cc(1038)] Control list
> match for rule #4 in gpu_driver_bug_list.
> [3976:3328:1014/171016:2072520:INFO:gpu_control_list.cc(1038)] Control list
> match for rule #17 in gpu_driver_bug_list.
> [3976:3328:1014/171017:2073097:ERROR:chrome_views_delegate.cc(158)] NOT
> IMPLEMENTED
> [3976:3328:1014/171017:2073097:ERROR:desktop_root_window_host_win.cc(694)] NOT
> IMPLEMENTED
> [3976:3328:1014/171047:2103158:ERROR:desktop_root_window_host_win.cc(694)] NOT
> IMPLEMENTED
> [295/326] TabDragging/DetachToBrowserTabDragControllerTest.DragAll/0 (TIMED
OUT)
> Note: Google Test filter =
> TabDragging/DetachToBrowserTabDragControllerTest.DragAllToSeparateWindow/0
> [==========] Running 1 test from 1 test case.
> [----------] Global test environment set-up.
> [----------] 1 test from TabDragging/DetachToBrowserTabDragControllerTest
> 
> 
> Do you think it is related to this change? (Will tentatively roll back this
> change if I don't come up with better ideas).

As discussed offline, this test seems to have passed in the next run and it
passes 5/5 locally with my CL on Win8.

Also note that Win Aura is now redudant with Windows bots as trunk switched to
Aura by default (those will soon switch to Win-NonAura bots in fact, but I think
this has yet to land).

Cheers!
Gab

> 
> Thanks

[gab, 2013-10-16 15:12:08]

@vangelis: some thoughts here as to the only I think this could possibly have
caused http://crbug.com/307495

WDYT? Can someone on the GPU team take this for investigation?

Cheers!
Gab

https://codereview.chromium.org/23703017/diff/294012/chrome/browser/gpu/chrom...
File chrome/browser/gpu/chrome_gpu_util.cc (left):

https://codereview.chromium.org/23703017/diff/294012/chrome/browser/gpu/chrom...
chrome/browser/gpu/chrome_gpu_util.cc:53: #endif
This could perhaps have caused it if the blacklist isn't used on the perf bots
(i.e. this was removed in this CL as the blacklist is now supposed to enforce
that before the field trials check even kicks in).

https://codereview.chromium.org/23703017/diff/294012/content/browser/gpu/gpu_...
File content/browser/gpu/gpu_data_manager_impl_private.cc (right):

https://codereview.chromium.org/23703017/diff/294012/content/browser/gpu/gpu_...
content/browser/gpu/gpu_data_manager_impl_private.cc:545: if
(command_line->HasSwitch(switches::kSkipGpuDataLoading))
Otherwise I just realized that this lost the previous condition of

&& !command_line->HasSwitch(switches::kUseGpuInTests)

do you think that should stay here? Could this have caused the perf regression?

[Zhenyao Mo, 2013-10-16 19:35:22]

Add tonyg here.

One possibility is that the perf xp bot does have a real gpu (tonyg can you
confirm?) and it happens to be one of the GPUs that's on the blacklist (say,
Intel 945).

https://codereview.chromium.org/23703017/diff/294012/content/browser/gpu/gpu_...
File content/browser/gpu/gpu_data_manager_impl_private.cc (right):

https://codereview.chromium.org/23703017/diff/294012/content/browser/gpu/gpu_...
content/browser/gpu/gpu_data_manager_impl_private.cc:545: if
(command_line->HasSwitch(switches::kSkipGpuDataLoading))
On 2013/10/16 15:12:09, gab wrote:
> Otherwise I just realized that this lost the previous condition of
> 
> && !command_line->HasSwitch(switches::kUseGpuInTests)
> 
> do you think that should stay here? Could this have caused the perf
regression?

I don't think perf bots run with kUseGpuInTests switch.

[gab, 2013-10-16 20:03:35]

On 2013/10/16 19:35:22, Zhenyao Mo wrote:
> Add tonyg here.
> 
> One possibility is that the perf xp bot does have a real gpu (tonyg can you
> confirm?) and it happens to be one of the GPUs that's on the blacklist (say,
> Intel 945).

Right, but if so they should have already been blacklisted before, this CL
shouldn't have changed anything.

> 
>
https://codereview.chromium.org/23703017/diff/294012/content/browser/gpu/gpu_...
> File content/browser/gpu/gpu_data_manager_impl_private.cc (right):
> 
>
https://codereview.chromium.org/23703017/diff/294012/content/browser/gpu/gpu_...
> content/browser/gpu/gpu_data_manager_impl_private.cc:545: if
> (command_line->HasSwitch(switches::kSkipGpuDataLoading))
> On 2013/10/16 15:12:09, gab wrote:
> > Otherwise I just realized that this lost the previous condition of
> > 
> > && !command_line->HasSwitch(switches::kUseGpuInTests)
> > 
> > do you think that should stay here? Could this have caused the perf
> regression?
> 
> I don't think perf bots run with kUseGpuInTests switch.

[Zhenyao Mo, 2013-10-16 20:13:13]

On Wed, Oct 16, 2013 at 1:03 PM,  <gab@chromium.org> wrote:
> On 2013/10/16 19:35:22, Zhenyao Mo wrote:
>>
>> Add tonyg here.
>
>
>> One possibility is that the perf xp bot does have a real gpu (tonyg can
>> you
>> confirm?) and it happens to be one of the GPUs that's on the blacklist
>> (say,
>> Intel 945).
>
>
> Right, but if so they should have already been blacklisted before, this CL
> shouldn't have changed anything.

This CL tries to go down a different settings with osmesa.   What I
said was it may not always be what you want if it happens to be a GPU
that was blacklisted.

Before we bypass blacklist through the kSkipGpuDataLoading switch.  In
this CL, you removed the switch on windows, so blacklist was turned
on, which could cause a different behavior.

Just a possibility.  I am not sure if this CL is causing the perf change or
not.

>
>
>
>
>
https://codereview.chromium.org/23703017/diff/294012/content/browser/gpu/gpu_...
>>
>> File content/browser/gpu/gpu_data_manager_impl_private.cc (right):
>
>
>
>
https://codereview.chromium.org/23703017/diff/294012/content/browser/gpu/gpu_...
>>
>> content/browser/gpu/gpu_data_manager_impl_private.cc:545: if
>> (command_line->HasSwitch(switches::kSkipGpuDataLoading))
>> On 2013/10/16 15:12:09, gab wrote:
>> > Otherwise I just realized that this lost the previous condition of
>> >
>> > && !command_line->HasSwitch(switches::kUseGpuInTests)
>> >
>> > do you think that should stay here? Could this have caused the perf
>> regression?
>
>
>> I don't think perf bots run with kUseGpuInTests switch.
>
>
>
>
> https://codereview.chromium.org/23703017/

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[gab, 2013-10-16 20:29:26]

On 2013/10/16 20:13:13, Zhenyao Mo wrote:
> On Wed, Oct 16, 2013 at 1:03 PM, 
<https://mail.google.com/mail/?view=cm&fs=1&tf=1&to=gab@chromium.org> wrote:
> > On 2013/10/16 19:35:22, Zhenyao Mo wrote:
> >>
> >> Add tonyg here.
> >
> >
> >> One possibility is that the perf xp bot does have a real gpu (tonyg can
> >> you
> >> confirm?) and it happens to be one of the GPUs that's on the blacklist
> >> (say,
> >> Intel 945).
> >
> >
> > Right, but if so they should have already been blacklisted before, this CL
> > shouldn't have changed anything.
> 
> This CL tries to go down a different settings with osmesa.   What I
> said was it may not always be what you want if it happens to be a GPU
> that was blacklisted.
> 
> Before we bypass blacklist through the kSkipGpuDataLoading switch.  In
> this CL, you removed the switch on windows, so blacklist was turned
> on, which could cause a different behavior.

Not quite, in this CL I removed the kSkipGpuDataLoading switch from
browser_tests.. it was already unused in all other contexts.

I doubt the perf bots are running browser_tests, right? Therefore this should
not have had an impact on what is blacklisted.


> 
> Just a possibility.  I am not sure if this CL is causing the perf change or
> not.
> 
> >
> >
> >
> >
> >
>
https://codereview.chromium.org/23703017/diff/294012/content/browser/gpu/gpu_...
> >>
> >> File content/browser/gpu/gpu_data_manager_impl_private.cc (right):
> >
> >
> >
> >
>
https://codereview.chromium.org/23703017/diff/294012/content/browser/gpu/gpu_...
> >>
> >> content/browser/gpu/gpu_data_manager_impl_private.cc:545: if
> >> (command_line->HasSwitch(switches::kSkipGpuDataLoading))
> >> On 2013/10/16 15:12:09, gab wrote:
> >> > Otherwise I just realized that this lost the previous condition of
> >> >
> >> > && !command_line->HasSwitch(switches::kUseGpuInTests)
> >> >
> >> > do you think that should stay here? Could this have caused the perf
> >> regression?
> >
> >
> >> I don't think perf bots run with kUseGpuInTests switch.
> >

[Zhenyao Mo, 2013-10-16 20:40:46]

On Wed, Oct 16, 2013 at 1:29 PM,  <gab@chromium.org> wrote:
> On 2013/10/16 20:13:13, Zhenyao Mo wrote:
>>
>> On Wed, Oct 16, 2013 at 1:03 PM,
>
> <https://mail.google.com/mail/?view=cm&fs=1&tf=1&to=gab@chromium.org> wrote:
>>
>> > On 2013/10/16 19:35:22, Zhenyao Mo wrote:
>> >>
>> >> Add tonyg here.
>> >
>> >
>> >> One possibility is that the perf xp bot does have a real gpu (tonyg can
>> >> you
>> >> confirm?) and it happens to be one of the GPUs that's on the blacklist
>> >> (say,
>> >> Intel 945).
>> >
>> >
>> > Right, but if so they should have already been blacklisted before, this
>> > CL
>> > shouldn't have changed anything.
>
>
>> This CL tries to go down a different settings with osmesa.   What I
>> said was it may not always be what you want if it happens to be a GPU
>> that was blacklisted.
>
>
>> Before we bypass blacklist through the kSkipGpuDataLoading switch.  In
>> this CL, you removed the switch on windows, so blacklist was turned
>> on, which could cause a different behavior.
>
>
> Not quite, in this CL I removed the kSkipGpuDataLoading switch from
> browser_tests.. it was already unused in all other contexts.
>
> I doubt the perf bots are running browser_tests, right? Therefore this
> should
> not have had an impact on what is blacklisted.
>

BrowserPerfTest inherits from InProcessBrowserTest, which inherits
from content::BrowserTestBase, which is affected by this CL.

Again, I am not saying this is the cause, but it's a possibility.

>
>
>
>> Just a possibility.  I am not sure if this CL is causing the perf change
>> or
>> not.
>
>
>> >
>> >
>> >
>> >
>> >
>
>
>
https://codereview.chromium.org/23703017/diff/294012/content/browser/gpu/gpu_...
>>
>> >>
>> >> File content/browser/gpu/gpu_data_manager_impl_private.cc (right):
>> >
>> >
>> >
>> >
>
>
>
https://codereview.chromium.org/23703017/diff/294012/content/browser/gpu/gpu_...
>>
>> >>
>> >> content/browser/gpu/gpu_data_manager_impl_private.cc:545: if
>> >> (command_line->HasSwitch(switches::kSkipGpuDataLoading))
>> >> On 2013/10/16 15:12:09, gab wrote:
>> >> > Otherwise I just realized that this lost the previous condition of
>> >> >
>> >> > && !command_line->HasSwitch(switches::kUseGpuInTests)
>> >> >
>> >> > do you think that should stay here? Could this have caused the perf
>> >> regression?
>> >
>> >
>> >> I don't think perf bots run with kUseGpuInTests switch.
>> >
>
>
> https://codereview.chromium.org/23703017/

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[gab, 2013-10-16 20:43:47]

On 2013/10/16 20:40:46, Zhenyao Mo wrote:
> On Wed, Oct 16, 2013 at 1:29 PM, 
<https://mail.google.com/mail/?view=cm&fs=1&tf=1&to=gab@chromium.org> wrote:
> > On 2013/10/16 20:13:13, Zhenyao Mo wrote:
> >>
> >> On Wed, Oct 16, 2013 at 1:03 PM,
> >
> > <https://mail.google.com/mail/?view=cm&fs=1&tf=1&to=gab%40chromium.org>
wrote:
> >>
> >> > On 2013/10/16 19:35:22, Zhenyao Mo wrote:
> >> >>
> >> >> Add tonyg here.
> >> >
> >> >
> >> >> One possibility is that the perf xp bot does have a real gpu (tonyg can
> >> >> you
> >> >> confirm?) and it happens to be one of the GPUs that's on the blacklist
> >> >> (say,
> >> >> Intel 945).
> >> >
> >> >
> >> > Right, but if so they should have already been blacklisted before, this
> >> > CL
> >> > shouldn't have changed anything.
> >
> >
> >> This CL tries to go down a different settings with osmesa.   What I
> >> said was it may not always be what you want if it happens to be a GPU
> >> that was blacklisted.
> >
> >
> >> Before we bypass blacklist through the kSkipGpuDataLoading switch.  In
> >> this CL, you removed the switch on windows, so blacklist was turned
> >> on, which could cause a different behavior.
> >
> >
> > Not quite, in this CL I removed the kSkipGpuDataLoading switch from
> > browser_tests.. it was already unused in all other contexts.
> >
> > I doubt the perf bots are running browser_tests, right? Therefore this
> > should
> > not have had an impact on what is blacklisted.
> >
> 
> BrowserPerfTest inherits from InProcessBrowserTest, which inherits
> from content::BrowserTestBase, which is affected by this CL.

Aah! I didn't know that... than the perf tests would definitely be affected by
this! i.e., it would have turned on software compositing mode on the XP bots!



> 
> Again, I am not saying this is the cause, but it's a possibility.
> 
> >
> >
> >
> >> Just a possibility.  I am not sure if this CL is causing the perf change
> >> or
> >> not.
> >
> >
> >> >
> >> >
> >> >
> >> >
> >> >
> >
> >
> >
>
https://codereview.chromium.org/23703017/diff/294012/content/browser/gpu/gpu_...
> >>
> >> >>
> >> >> File content/browser/gpu/gpu_data_manager_impl_private.cc (right):
> >> >
> >> >
> >> >
> >> >
> >
> >
> >
>
https://codereview.chromium.org/23703017/diff/294012/content/browser/gpu/gpu_...
> >>
> >> >>
> >> >> content/browser/gpu/gpu_data_manager_impl_private.cc:545: if
> >> >> (command_line->HasSwitch(switches::kSkipGpuDataLoading))
> >> >> On 2013/10/16 15:12:09, gab wrote:
> >> >> > Otherwise I just realized that this lost the previous condition of
> >> >> >
> >> >> > && !command_line->HasSwitch(switches::kUseGpuInTests)
> >> >> >
> >> >> > do you think that should stay here? Could this have caused the perf
> >> >> regression?
> >> >
> >> >
> >> >> I don't think perf bots run with kUseGpuInTests switch.
> >> >
> >
> >
> > https://codereview.chromium.org/23703017/
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to
https://mail.google.com/mail/?view=cm&fs=1&tf=1&to=chromium-reviews+unsubscri....

[tonyg, 2013-10-17 01:06:20]

Sounds like you guys got this figured out. But I should mention the perf bots
now spit out their GPU device and feature status (stuff from about:gpu) in their
stdio logs. If you have trouble finding any of the information, just lmk and
I'll look it up.