Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2891633002:
Add a test for subframes redirecting to hosted app URLs. (Closed)
DescriptionAdd a test for subframes redirecting to hosted app URLs.
We recently had a regression that reached stable channel because we
had no tests for loading subframes that redirect to hosted app URLs.
This CL adds test coverage for that case.
BUG=721949, 715756
Review-Url: https://codereview.chromium.org/2891633002
Cr-Commit-Position: refs/heads/master@{#472986}
Committed: https://chromium.googlesource.com/chromium/src/+/c1a9cc5af57748545858620aab0e9cd8e24dcc93
Patch Set 1 #
Messages
Total messages: 18 (12 generated)
The CQ bit was checked by alexmos@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== Add a test for subframes redirecting to hosted apps. BUG=721949 ========== to ========== Add a test for subframes redirecting to hosted app URLs. We recently had a regression that reached stable channel because we had no tests for loading subframes that redirect to hosted app URLs. This CL adds test coverage for that case. BUG=721949 ==========
Description was changed from ========== Add a test for subframes redirecting to hosted app URLs. We recently had a regression that reached stable channel because we had no tests for loading subframes that redirect to hosted app URLs. This CL adds test coverage for that case. BUG=721949 ========== to ========== Add a test for subframes redirecting to hosted app URLs. We recently had a regression that reached stable channel because we had no tests for loading subframes that redirect to hosted app URLs. This CL adds test coverage for that case. BUG=721949 ==========
alexmos@chromium.org changed reviewers: + creis@chromium.org
Charlie, can you please take a look? When verifying that the test would fail without the fix in issue 697513, I discovered that with r455821 reverted, the test still passed due to my r465055 for fixing issue 711006, which placed the hosted app URL in the parent SiteInstance because it doesn't require a dedicated process. But with both r455821 and r465055 reverted, the test failed as it should.
Description was changed from ========== Add a test for subframes redirecting to hosted app URLs. We recently had a regression that reached stable channel because we had no tests for loading subframes that redirect to hosted app URLs. This CL adds test coverage for that case. BUG=721949 ========== to ========== Add a test for subframes redirecting to hosted app URLs. We recently had a regression that reached stable channel because we had no tests for loading subframes that redirect to hosted app URLs. This CL adds test coverage for that case. BUG=721949, 715756 ==========
On 2017/05/17 17:03:24, alexmos wrote: > Charlie, can you please take a look? > > When verifying that the test would fail without the fix in issue 697513, I > discovered that with r455821 reverted, the test still passed due to my r465055 > for fixing issue 711006, which placed the hosted app URL in the parent > SiteInstance because it doesn't require a dedicated process. But with both > r455821 and r465055 reverted, the test failed as it should. LGTM! Thanks for confirming that the test works without both of those fixes. Note: I added issue 715756 to the bug line as well.
creis@chromium.org changed reviewers: + rdevlin.cronin@chromium.org
Ah, looks like we need an extension owner. Devlin, can you take a look for owners approval?
lgtm
The CQ bit was checked by creis@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 1, "attempt_start_ts": 1495143330320740, "parent_rev":
"f8ff4f535ca35569ff840d9ffb71bb3e9e5d3ee1", "commit_rev":
"c1a9cc5af57748545858620aab0e9cd8e24dcc93"}
Message was sent while issue was closed.
Description was changed from ========== Add a test for subframes redirecting to hosted app URLs. We recently had a regression that reached stable channel because we had no tests for loading subframes that redirect to hosted app URLs. This CL adds test coverage for that case. BUG=721949, 715756 ========== to ========== Add a test for subframes redirecting to hosted app URLs. We recently had a regression that reached stable channel because we had no tests for loading subframes that redirect to hosted app URLs. This CL adds test coverage for that case. BUG=721949, 715756 Review-Url: https://codereview.chromium.org/2891633002 Cr-Commit-Position: refs/heads/master@{#472986} Committed: https://chromium.googlesource.com/chromium/src/+/c1a9cc5af57748545858620aab0e... ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1) as https://chromium.googlesource.com/chromium/src/+/c1a9cc5af57748545858620aab0e... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
