DevTools: patch browser's Chrome version into Chrome user agents for emulation

third_party/WebKit/Source/devtools/front_end/common/BrowserVersionInfo.js

Index: third_party/WebKit/Source/devtools/front_end/common/BrowserVersionInfo.js
diff --git a/third_party/WebKit/Source/devtools/front_end/common/BrowserVersionInfo.js b/third_party/WebKit/Source/devtools/front_end/common/BrowserVersionInfo.js
new file mode 100644
index 0000000000000000000000000000000000000000..b3daae0f0d2e83d1d6080ac9bade8dda9ff4c010
--- /dev/null
+++ b/third_party/WebKit/Source/devtools/front_end/common/BrowserVersionInfo.js
@@ -0,0 +1,22 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+WebInspector.BrowserVersionInfo = {

[pfeldman, 2016/09/14 21:47:09]

This is no longer browser version info, also you don't need a new file for a 3
line function and put it into common. This can be a part of "emulation" module.

[luoe, 2016/09/14 22:49:13]

There are 2 places where we could use this patching:
emulated_devices/module.json (device mode)
network/networkConfigView.js (the drawer)

Since network doesn't depend on emulation, isn't this necessary?

[pfeldman, 2016/09/15 21:44:06]

"sdk" would be the common denominator then, you can put it into
WebInspector.MultitargetNetworkManager.

[luoe, 2016/09/15 23:18:08]

Done.

+    DefaultChromeVersion: "54.0.2834.0",

[pfeldman, 2016/09/14 21:47:09]

I'd leave these hardcoded in the config view and only replace them with current
if regex matches.

[luoe, 2016/09/14 22:49:13]

Done.

+
+    /**
+     * @param {string} uaString
+     * @return {string}
+     */
+    patchUserAgentWithChromeVersion: function(uaString)
+    {
+        // Extracts Chrome version from user agent ("1.2.3.4" when user agent is: "Chrome/1.2.3.4").
+        var sectionRegex = new RegExp("(?:^|\\W)Chrome/(\\S+)");

[pfeldman, 2016/09/14 21:47:09]

var regex = /Chrome\/[\d.]+/;
var version = navigator.userAgent.match(regex)[0];
return version ? uaString.replace(regex, version) : uaString;

[luoe, 2016/09/14 22:49:13]

Regex: I think the "(?:^|\\W)" is warranted so that we don't match on "XChrome",
for example.  Also, \S should be more flexible than \d.  We aren't guaranteed
that the version value is only numbers.  Could change in the future, similar to
"Mobile/13B143".

match: If there's no match, ua.match(regex) === null.  A change in UA format
could throw an error in DevTools if this isn't checked.

replace: String.sprintf() gives us an added check, asserting that the string
doesn't have more than one '%s'

With replace, we should now do 2 replacements, one for 'Chrome' and one for
'CriOS'.

[pfeldman, 2016/09/15 21:44:06]

- If XChrome gets into the UA string, this means that you no longer emulate it
properly - there is no XChrome in your template that you paste version into.
Same here, if version gets characters, your pattern will no longer stand and
will likely need an update.
Sure, the null check is necessary.
Which string? There are no longer strings with %s.
Could you elaborate?

[luoe, 2016/09/15 23:18:08]

If there is an XChrome in the staleAgent, we will update it when we shouldn't. 
If there is an XChrome in the frontend UA, we will extract the wrong value.

What we gain by using "(?:^|\\W)" is we avoid these bad cases:
(1)
frontEndAgent: "GoogleChrome/bad"
staleAgent: "Chrome/x", staleAgent.replace(): "Chrome/bad"

(2)
frontEndAgent: "Chrome/bad"
staleAgent: "XChrome/x", staleAgent.replace(): "Chrome/bad"

I don't think we lose anything, and we don't need to update the regex if in the
future either agent gets XChrome.
I think the pattern will still work?  I tested it and it will replace the
staleAgent with the current version even if the current version contains
characters.

The $1 in the .replace() means replace only the part of the string inside the
2nd capturing group.
Acknowledged.
For Chrome on iPhone and iPad, the user agents use 'CriOS' instead of 'Chrome'. 
Therefore, to keep those up to date without explicitly marking them with '%s' we
need to replace stale instances of both keywords.

Also, replacing all user agents with 'Chrome' means that things like 'Opera 37',
an agent that we intentionally hard coded at version 37, will have an
auto-updating 'Chrome/' number.  Isn't this a valid reason to put '%s' back in?

+        var sectionMatch = navigator.userAgent.match(sectionRegex);
+        var version = WebInspector.BrowserVersionInfo.DefaultChromeVersion;
+        if (sectionMatch && sectionMatch.length > 1)
+            version = sectionMatch[1];
+        return String.sprintf(uaString, version);
+    }
+};