Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(197)

Issue 688363002: Convert tests to use GetURL(host, path) instead of redirector. (Closed)

Created:
6 years, 1 month ago by nasko
Modified:
6 years, 1 month ago
CC:
chromium-reviews, darin-cc_chromium.org, jam, site-isolation-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

Convert tests to use GetURL(host, path) instead of redirector. As discovered by japhet@, if we navigate cross-site in a browser test using the '/cross-site/' redirector syntax, the initial URL is relative to the embedded_test_server, which happens to also be the SiteInstance of the top-level page. This causes an extra process swap, which is unnecessary and can create unexpected effects in the test. I've added GetURL(hostname, path) method to EmbeddedTestServer to give us the proper URL directly without following a redirect path and this CL updates the tests to use this new method. BUG=418236 Committed: https://crrev.com/30374c75c1736fe2a476382edcadd68fc5ddc0d0 Cr-Commit-Position: refs/heads/master@{#302126}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+17 lines, -37 lines) Patch
M content/browser/site_per_process_browsertest.cc View 7 chunks +17 lines, -37 lines 0 comments Download

Messages

Total messages: 7 (2 generated)
nasko
Hey Charlie, Can you review this CL for me? I've added Nate as FYI. Thanks! ...
6 years, 1 month ago (2014-10-30 17:35:51 UTC) #2
Charlie Reis
I like it-- it's a bit cleaner as well. LGTM.
6 years, 1 month ago (2014-10-30 17:39:33 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/688363002/1
6 years, 1 month ago (2014-10-30 17:42:07 UTC) #5
commit-bot: I haz the power
Committed patchset #1 (id:1)
6 years, 1 month ago (2014-10-30 19:18:44 UTC) #6
commit-bot: I haz the power
6 years, 1 month ago (2014-10-30 19:19:24 UTC) #7
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/30374c75c1736fe2a476382edcadd68fc5ddc0d0
Cr-Commit-Position: refs/heads/master@{#302126}

Powered by Google App Engine
This is Rietveld 408576698