[Blimp] Refactor Blimp test engine with embedded test server and URL rewriting

blimp/engine/testing/app/blimp_url_rewriter.cc

Index: blimp/engine/testing/app/blimp_url_rewriter.cc
diff --git a/blimp/engine/testing/app/blimp_url_rewriter.cc b/blimp/engine/testing/app/blimp_url_rewriter.cc
new file mode 100644
index 0000000000000000000000000000000000000000..9a95035838c3f5cf9d191ae3748e3fd3fbb17c8b
--- /dev/null
+++ b/blimp/engine/testing/app/blimp_url_rewriter.cc
@@ -0,0 +1,54 @@
+// Copyright (c) 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.
+
+#include <string>
+
+#include "blimp/engine/testing/app/blimp_url_rewriter.h"
+#include "content/public/test/test_browser_context.h"
+#include "net/test/embedded_test_server/embedded_test_server.h"
+#include "url/gurl.h"
+
+namespace content {
+const char kBlimpTestScheme[] = "blimp-test";
+}  // namespace content
+
+namespace blimp {
+namespace engine {
+
+namespace test {
+
+// Embedded Test Server instance. This should be available globally and should
+// be long lived.
+std::unique_ptr<net::EmbeddedTestServer> g_ets_instance;
+
+bool HandleBlimpTestURL(GURL* url, content::BrowserContext* browser_context) {

[Kevin M, 2017/01/09 20:17:18]

browser_context is never referenced, so can we remove BrowserContext includes
too?

[shenghuazhang, 2017/01/10 23:20:50]

I think it needs the content::BrowserContext included?

This would be called by BrowserURLHandler->AddHandlerPair (line 55 in
https://codereview.chromium.org/2572563006/diff/60001/blimp/engine/testing/ap...).
Here is an example in content/browser:
https://codesearch.chromium.org/chromium/src/content/browser/browser_url_hand...

So the method HandleBlimpTestURL should have the same type of function with
URLHandler:
https://codesearch.chromium.org/chromium/src/content/public/browser/browser_u...

+  // Handles rewriting request URLs to be what the engine will actually load
+  // URL with scheme 'blimp-test', i.e.,
+  // 'blimp-test://chrome/test/data/simple.html'. It works together with
+  // EmbeddedTestServer to handle getting full url, which strips the url
+  // content first and calls the GetURL function. The result URL would be like
+  // 'http://127.0.0.1:12345/chrome/test/data/simple.html'
+  if (url->is_valid() && url->SchemeIsHTTPOrHTTPS())

[Kevin M, 2017/01/09 20:17:18]

What about non-HTTP urls? Can you fall back to a default handler for
non-"blimp-test" URLs instead of hardcoding a whitelist?

[shenghuazhang, 2017/01/10 23:20:50]

Got it.
Return false for all conditions except for 'blimp-test' urls, BrowserUrlHandler
would find a handler that can run it by default.

+    return true;
+
+  if (url->SchemeIs(content::kBlimpTestScheme)) {
+    // Load the URL content which is everything after the scheme.
+    // URL = "blimp-test://chrome/test/data/page.html"
+    // URL->GetContent() = "//chrome/test/data/page.html"
+    std::string url_content = url->GetContent();
+    url_content.erase(url_content.begin());
+    GURL rewritten_url = g_ets_instance->GetURL(url_content);
+
+    if (rewritten_url.is_valid()) {

[Kevin M, 2017/01/09 20:17:18]

Assuming our code isn't broken, should this ever fail? Can we just use
DCHECK(rewritten_url.is_valid())  instead?

[shenghuazhang, 2017/01/10 23:20:50]

That makes sense. Would use DCHECK thing instead.

+      *url = rewritten_url;
+      return true;
+    }
+  }
+  return false;
+}
+
+}  // namespace test
+
+}  // namespace engine
+}  // namespace blimp