Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2525433003:
Site Isolation: test srcdoc navigation after a cross-site navigation. (Closed)
|
Created:
4 years, 1 month ago by arthursonzogni Modified:
4 years ago Reviewers:
Charlie Reis CC:
chromium-reviews, jam, darin-cc_chromium.org, nasko, clamy, site-isolation-reviews_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionSite Isolation: test srcdoc navigation after a cross-site navigation.
After a cross-site navigation, ensure that an iframe that navigates to
its srcdoc doesn't use its previous process or a new one but reuses its
parent's process instead.
BUG=None
Committed: https://crrev.com/85ef7718b10db125d582ce1556b3dc94189deeb0
Cr-Commit-Position: refs/heads/master@{#434140}
Patch Set 1 #
Total comments: 6
Patch Set 2 : Nit. #Patch Set 3 : Rebase. #Messages
Total messages: 31 (23 generated)
The CQ bit was checked by arthursonzogni@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 ========== Site Isolation: test srcdoc navigation after a cross-site navigation. After a cross-site navigation, ensure that an iframe that navigates to its srcdoc doesn't use its previous process or a new one but reuses its parent's process instead. BUG=None ========== to ========== Site Isolation: test srcdoc navigation after a cross-site navigation. After a cross-site navigation, ensure that an iframe that navigates to its srcdoc doesn't use its previous process or a new one but reuses its parent's process instead. BUG=None ==========
arthursonzogni@chromium.org changed reviewers: + creis@chromium.org
Hi Charlie, This is the test you spoke about in: https://codereview.chromium.org/2494633004 Is it what you though? In that case, please can you take a look at it? Thanks!
The CQ bit was checked by arthursonzogni@chromium.org to run a CQ dry run
Patchset #1 (id:1) has been deleted
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: Try jobs failed on following builders: mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...) mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by arthursonzogni@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.
[+site-isolation-reviews] Thanks-- that's perfect. LGTM. https://codereview.chromium.org/2525433003/diff/20001/content/browser/site_pe... File content/browser/site_per_process_browsertest.cc (right): https://codereview.chromium.org/2525433003/diff/20001/content/browser/site_pe... content/browser/site_per_process_browsertest.cc:8662: // as its parent. Then when it navigates to its "srcdoc" attribute, it must nit: Then when its parent navigates it via the "srcdoc" attribute, it must https://codereview.chromium.org/2525433003/diff/20001/content/browser/site_pe... content/browser/site_per_process_browsertest.cc:8670: GURL blank_url(url::kAboutBlankURL); Sanity check: This will change to about:srcdoc after your other CL lands, right? https://codereview.chromium.org/2525433003/diff/20001/content/browser/site_pe... content/browser/site_per_process_browsertest.cc:8690: R"(document.getElementById("child-0").srcdoc = "srcdoc content";)")); Style nit: We don't use R (raw string literals) much here. I think it might be more consistent and easier to read as a normal string with single quotes inside.
Thank you for your review and suggestions. https://codereview.chromium.org/2525433003/diff/20001/content/browser/site_pe... File content/browser/site_per_process_browsertest.cc (right): https://codereview.chromium.org/2525433003/diff/20001/content/browser/site_pe... content/browser/site_per_process_browsertest.cc:8662: // as its parent. Then when it navigates to its "srcdoc" attribute, it must On 2016/11/23 08:02:25, Charlie Reis (slow) wrote: > nit: Then when its parent navigates it via the "srcdoc" attribute, it must Done. https://codereview.chromium.org/2525433003/diff/20001/content/browser/site_pe... content/browser/site_per_process_browsertest.cc:8670: GURL blank_url(url::kAboutBlankURL); On 2016/11/23 08:02:25, Charlie Reis (slow) wrote: > Sanity check: This will change to about:srcdoc after your other CL lands, right? Yes that's right. I confirm. https://codereview.chromium.org/2525433003/diff/20001/content/browser/site_pe... content/browser/site_per_process_browsertest.cc:8690: R"(document.getElementById("child-0").srcdoc = "srcdoc content";)")); On 2016/11/23 08:02:25, Charlie Reis (slow) wrote: > Style nit: We don't use R (raw string literals) much here. I think it might be > more consistent and easier to read as a normal string with single quotes inside. Done.
The CQ bit was checked by arthursonzogni@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from creis@chromium.org Link to the patchset: https://codereview.chromium.org/2525433003/#ps40001 (title: "Nit.")
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
Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by arthursonzogni@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from creis@chromium.org Link to the patchset: https://codereview.chromium.org/2525433003/#ps60001 (title: "Rebase.")
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": 60001, "attempt_start_ts": 1479893414862340,
"parent_rev": "7006d5d512afef6cf4a975925463476d9377cf56", "commit_rev":
"810269317dc923fb28701a29ca206d7c6ccb0aad"}
Message was sent while issue was closed.
Description was changed from ========== Site Isolation: test srcdoc navigation after a cross-site navigation. After a cross-site navigation, ensure that an iframe that navigates to its srcdoc doesn't use its previous process or a new one but reuses its parent's process instead. BUG=None ========== to ========== Site Isolation: test srcdoc navigation after a cross-site navigation. After a cross-site navigation, ensure that an iframe that navigates to its srcdoc doesn't use its previous process or a new one but reuses its parent's process instead. BUG=None ==========
Message was sent while issue was closed.
Committed patchset #3 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== Site Isolation: test srcdoc navigation after a cross-site navigation. After a cross-site navigation, ensure that an iframe that navigates to its srcdoc doesn't use its previous process or a new one but reuses its parent's process instead. BUG=None ========== to ========== Site Isolation: test srcdoc navigation after a cross-site navigation. After a cross-site navigation, ensure that an iframe that navigates to its srcdoc doesn't use its previous process or a new one but reuses its parent's process instead. BUG=None Committed: https://crrev.com/85ef7718b10db125d582ce1556b3dc94189deeb0 Cr-Commit-Position: refs/heads/master@{#434140} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/85ef7718b10db125d582ce1556b3dc94189deeb0 Cr-Commit-Position: refs/heads/master@{#434140} |
