Rewrite YouTube Flash embeds

chrome/renderer/chrome_content_renderer_client.cc

Index: chrome/renderer/chrome_content_renderer_client.cc
diff --git a/chrome/renderer/chrome_content_renderer_client.cc b/chrome/renderer/chrome_content_renderer_client.cc
index 68c5529d55ea8d86eeaf69d6b5891f783f3a670a..2907c9c1c737f6d7aa980fa4fdf33e95e8f2f328 100644
--- a/chrome/renderer/chrome_content_renderer_client.cc
+++ b/chrome/renderer/chrome_content_renderer_client.cc
@@ -1390,3 +1390,42 @@ bool ChromeContentRendererClient::ShouldEnforceWebRTCRoutingPreferences() {
   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;
+}