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 2f4ebe61c9e84f915ea5ff91c494ed572651d63a..5da3ecaa6c1310a73263c8073d9da2ad19964478 100644
--- a/chrome/renderer/chrome_content_renderer_client.cc
+++ b/chrome/renderer/chrome_content_renderer_client.cc
@@ -21,6 +21,7 @@
 #include "base/values.h"
 #include "build/build_config.h"
 #include "chrome/common/channel_info.h"
+#include "chrome/common/chrome_features.h"
 #include "chrome/common/chrome_isolated_world_ids.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
@@ -1379,3 +1380,45 @@ bool ChromeContentRendererClient::ShouldEnforceWebRTCRoutingPreferences() {
   return true;
 #endif
 }
+
+std::string ChromeContentRendererClient::OverrideFlashEmbedWithHTML(
+    const std::string& str_url) {
+  if (!base::FeatureList::IsEnabled(features::kOverrideYouTubeFlashEmbed))
+    return "";
+
+  GURL url = GURL(str_url);
+  if (!url.is_valid())
+    return "";
+
+  // We'll only modify YouTube Flash embeds. The URLs can be recognized since
+  // they're in the following form: youtube.com/v/VIDEO_ID. So, we check to see
+  // if the given URL does follow that format. We don't modify
+  // any URLs that contain the enablejsapi=1 parameter since the page may be

[mlamouri (slow - plz ping), 2016/08/01 13:43:59]

Can you open a follow-up bug to actually do the rewrite when enablejsapi=1 on
Android?

[kdsilva, 2016/08/02 15:34:46]

Done.

+  // interacting with the YouTube Flash player in Javascript and we don't
+  // want to break working content.
+  if (!url.DomainIs("youtube.com") || url.path().find("/v/") != 0
+      || str_url.find("enablejsapi=1") != std::string::npos)
+    return "";

[dcheng, 2016/08/01 14:50:34]

This should just use GURLs, but note that converting "" to std::string() like
this is surprisingly expensive in general: it's better to just use the
std::string default constructor. A similar comment applies throughout the rest
of this CL for WebString/WebURL

[kdsilva, 2016/08/02 15:34:46]

Done.

+
+  std::string ret_url = str_url;
+  ret_url.replace(ret_url.find("/v/"), 3, "/embed/");

[dcheng, 2016/08/01 14:50:34]

Can we just use GURL and avoid cracking URLs manually? At least we can make it
just match the patch component that way and simplify the matching logic below.

[kdsilva, 2016/08/02 15:34:46]

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 a "&"
+  // for each subsequent parameter.

[dcheng, 2016/08/01 14:50:34]

Is it important to fix invalid URLs? Wouldn't they be broken anyway?

[kdsilva, 2016/08/02 15:34:46]

The Flash video player has some URL correction capabilities, so we didn't want
the move to HTML5 to break pages that were already working. I've updated the
comment to mention this, let me know if you have any more questions! 

+  size_t index = ret_url.find_first_of("&?");
+  bool invalid_url = index != std::string::npos && ret_url.at(index) == '&';
+
+  if (!invalid_url)
+    return ret_url;
+
+  // ? should appear first before all parameters
+  ret_url.replace(index, 1, "?");
+
+  // Replace all instances of ? (after the first) with &
+  for (size_t pos = index + 1;
+        (pos= ret_url.find("?",pos )) != std::string::npos; pos += 1)
+      ret_url.replace(pos, 1, "&");
+  return ret_url;
+}