Rewrite YouTube Flash embeds

chrome/renderer/chrome_content_renderer_client.cc

 // Copyright (c) 2012 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.
 
 #include "chrome/renderer/chrome_content_renderer_client.h"
 
 #include <memory>
 #include <utility>
 
 #include "base/command_line.h"
@@ skipping 1372 matching lines @@
 // chrome.system.network.getNetworkInterfaces provides the same
 // information. Also, the enforcement of sending and binding UDP is already done
 // by chrome extension permission model.
 bool ChromeContentRendererClient::ShouldEnforceWebRTCRoutingPreferences() {
 #if defined(ENABLE_EXTENSIONS)
   return !IsStandaloneExtensionProcess();
 #else
   return true;
 #endif
 }
+
+// TODO(kdsilva)
+bool ChromeContentRendererClient::OverrideEmbedInfo(std::string* url) {
+  GURL gurl = GURL(*url);
+
+  if (url->find("enablejsapi=1") == (size_t) -1 &&
+      gurl.DomainIs("www.youtube.com") && gurl.path().find("/v/") == 0) {

[mlamouri (slow - plz ping), 2016/07/20 13:22:02]

Could this be "youtube.com" instead of "www.youtube.com"? In other words, would
it work? Can YouTube embed not have "www"?

[kdsilva, 2016/07/21 13:59:45]

Done.

+
+    // If the website is using an invalid YouTube URL, we'll try and
+    // fix the url by ensuring that IF there are multiple parameters,
+    // the parameter string begins with a "?" and then follows with "&"
+    // for each subsequent parameter.
+    size_t indexAmp = url->find("&");
+    bool invalidUrl = false;

[mlamouri (slow - plz ping), 2016/07/20 13:22:02]

nit: maybe have the `bool` first.

[kdsilva, 2016/07/21 13:59:45]

Done.

+    size_t indexQm = url->find("?");
+
+    if (indexQm == std::string::npos || indexQm > indexAmp) {

[mlamouri (slow - plz ping), 2016/07/20 13:22:02]

Does this mean that "www.youtube.com/v/VIDEO_ID" is an invalid URL?

How does string::npos compare to other values (ie. is it per spec equal to 0 or
INT_MAX?)?

[kdsilva, 2016/07/21 13:59:45]

You're right that youtube.com/v/ID should't be considered an invalid URL. I've
updated the code to reflect that! 

string:npos is the is the largest possible value. That simplifies the if
statement a bit.

PTAL.

+        invalidUrl = true;
+    }

[mlamouri (slow - plz ping), 2016/07/20 13:22:02]

style: we [unfortunately] don't put { } for one-liners.

[kdsilva, 2016/07/21 13:59:45]

Done.

+
+    if (invalidUrl) {
+        // Replace all instances of ? with &
+        size_t start_pos = 0;
+        while((start_pos = url->find("?", start_pos)) != std::string::npos) {
+            url->replace(start_pos, 1, "&");
+            start_pos += 1;
+        }
+
+        // ? should appear first before all parameters
+        if (indexAmp != std::string::npos) {
+          url->replace(indexAmp, 1, "?");
+        }
+    }
+    url->replace(url->find("/v/"), 3, "/embed/");
+    return true;
+  }

[mlamouri (slow - plz ping), 2016/07/20 13:22:02]

nit: you should favour early returns. In this case, you could do:
```
if (!gurl.DomainIs(...))
  return false;

if (no "/v/" or has "enablejsapi=1")
  return false;

// the real code
```

[kdsilva, 2016/07/21 13:59:45]

Done.

+
+  return false;
+}