Fix Resource Priorities and Scheduling (Chrome Side)

chrome/browser/chrome_content_browser_client.cc

Index: chrome/browser/chrome_content_browser_client.cc
diff --git a/chrome/browser/chrome_content_browser_client.cc b/chrome/browser/chrome_content_browser_client.cc
index 1e4392e3f943535bc79a97015283acdd12ce6494..666bdc7a978f2de93f3d8f35b6f23c6282236f4f 100644
--- a/chrome/browser/chrome_content_browser_client.cc
+++ b/chrome/browser/chrome_content_browser_client.cc
@@ -1218,6 +1218,39 @@ void MaybeAppendBlinkSettingsSwitchForFieldTrial(
       }
     }
   }
+
+  // Flags for the ResourcePriorities field trial. The settings are
+  // encoded in the field trial group name instead of as variations
+  // because the variations code is not accessible from the loader.
+  //
+  // The group name encoding looks like this:
+  //  <descriptiveName>_ABCDE_E2_F_G
+  //    A - fetchDeferLateScripts (1 for true, 0 for false)
+  //    B - fetchIncreaseFontPriority (1 for true, 0 for false)
+  //    C - fetchIncreaseAsyncScriptPriority (1 for true, 0 for false)
+  //    D - fetchIncreasePriorities (1 for true, 0 for false)
+  //    E - fetchEnableLayoutBlockingThreshold (1 for true, 0 for false)
+  //    E2 - fetchLayoutBlockingThreshold (Numeric)
+  //    F - fetchMaxNumDelayableWhileLayoutBlocking (Numeric)
+  //    G - fetchMaxNumDelayableRequests (Numeric)
+  //
+  // Only A-D are relevant to blink and exposed as settings
+  std::string resource_priorities_trial_group =
+      base::FieldTrialList::FindFullName("ResourcePriorities");
+  std::vector<std::string> split_group(
+      base::SplitString(resource_priorities_trial_group, "_",
+                        base::KEEP_WHITESPACE, base::SPLIT_WANT_ALL));
+  if (split_group.size() == 5 && split_group[1].length() == 5) {

[Bryan McQuade, 2015/08/07 18:02:26]

you'll likely have Control,Default,and BlinkSettingsFlag groups, which won't
match this pattern. Just worth adding a comment about this so future readers of
the code know that you expect to get some group names that don't match the
pattern, and it's correct to ignore them.

[Pat Meenan, 2015/08/07 20:48:09]

Done.

+    if (split_group[1].at(0) == '1')
+      blink_settings.push_back("fetchDeferLateScripts=true");
+    if (split_group[1].at(1) == '1')
+      blink_settings.push_back("fetchIncreaseFontPriority=true");
+    if (split_group[1].at(2) == '1')
+      blink_settings.push_back("fetchIncreaseAsyncScriptPriority=true");
+    if (split_group[1].at(3) == '1')
+      blink_settings.push_back("fetchIncreasePriorities=true");
+  }

[mmenke, 2015/08/07 16:55:10]

Also, do we have any tests of this method?  I'm paranoid.

[mmenke, 2015/08/07 16:55:10]

I assume blink settings are generally tested at the blink layer, and not with
integrationy-tests?

[mmenke, 2015/08/07 16:55:10]

What about the others?

[Bryan McQuade, 2015/08/07 17:35:53]

There are some basic tests at
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/chr...
- I think we could benefit from adding some that test this new logic as well.

[Pat Meenan, 2015/08/07 20:48:09]

Done.

[Pat Meenan, 2015/08/07 20:48:09]

Done.

+
   if (blink_settings.empty()) {
     return;
   }