Move GetSiteURL to SecurityPrincipal interface [chromium/src : main]
Viktoriya Bryhider (Gerrit)
Viktoriya Bryhider added 17 comments![]( "Open in Gerrit")
Publishing removing GetSiteURL from SiteInstance.
I have moved all calls as is to `GetSecurityPrincipal().GetDeprecatedSiteURL()`. It should be green now. Checked all callsites from outside of /content and left unresolved comments where we can get rid of GetSecurityPrincipal().GetDeprecatedSiteURL() - can create separate Cls if you could confirm that those are acceptable replacements.
Please let me know how to proceed or if there a way to make it easier...
Thanks!
->GetSecurityPrincipal()`net::SchemefulSite(frame_host->GetLastCommittedURL()).GetURL()` should work here.
owner_site_instance->GetSecurityPrincipal().GetDeprecatedSiteURL());Can pass `owner_rfh` instead and use `owner_rfh->GetLastCommittedOrigin().GetURL()` in place of `owner_site_instance->GetSecurityPrincipal().GetDeprecatedSiteURL()`. I have tried this locally - and tests seems to pass... Can create CL
const GURL& site_url = web_contents->GetSiteInstance()Intentionally using SiteInstance::GetSiteURL - https://chromium-review.googlesource.com/c/chromium/src/+/7426181
const GURL& site_url =In theory can try to replace with `net::SchemefulSite(render_frame_host->GetLastCommittedURL())` to keep eTLD+1, but it will not work for hosted apps(?).
Another option is to keep it and wait for crbug.com/40775860 mentioned above to replace URL with title.
site_instance_->GetSecurityPrincipal().GetDeprecatedSiteURL().spec());I have tried to replace this with `net::SchemefulSite(rfh_->GetLastCommittedURL());` and run tests `browser_tests --gtest_filter="*FencedFrameTaskBrowserTest*" ` - the only difference is in last slash: `https://b.test/` vs `https://b.test` - looks like safe to replace to me.
site_instance_->GetSecurityPrincipal().GetDeprecatedSiteURL();Need effective URL for hosted apps - otherwise can use SchemefulSite(GetLastCommittedURL())
GURL site_url = contents->GetPrimaryMainFrame()Can potentially use `contents->GetLastCommittedURL()` here. BrowserInstantControllerTest.DefaultSearchProviderChanged passes with that change - not sure if there any corner cases.
webui::LogWebUIShown(web_contents_ret->GetSiteInstance()Test `LogWebUIUrlTest.ShownWebUIForPreloadedPage` is hitting this codepath: I have checked:
1. web_contents_ret->GetVisibleURL();
2. webui_url;
3. web_contents_ret->GetURL(); all of them are the same - can try to substitute `GetDeprecatedSiteURL()` with one of them.
const GURL& GuestViewBase::GetOwnerSiteURL() const {Tried to use `owner_rfh()->GetLastCommittedOrigin().GetURL()` here - tests which are hitting that codepath (*WebViewContentScript*) are passing, not sure about corner cases if any - can put CL.
owner_rfh()owner_rfh should have committed navigation from what I'm reading. Can try to use `owner_rfh()->GetLastCommittedOrigin().host()`. Have not tested.
render_frame_host()Have WIP to replace SiteURL with SecurityPrincipal in Services - https://chromium-review.googlesource.com/c/chromium/src/+/7712794
->GetSecurityPrincipal()Considering replacing it with `embedder_rfh->GetMainFrame()->GetLastCommittedOrigin()` similar to line 119 in this file. Have not found cases when it will not work.
->GetSiteInstance()Considering replacing it with `render_frame_host()->GetLastCommittedURL().withEmptyPath()`
.GetDeprecatedSiteURL()Seems safe to use frame.GetLastCommittedURL() or frame.GetLastCommittedOrigin() here.
const GURL& owner_site_url =Same as https://chromium-review.googlesource.com/c/chromium/src/+/7614138/2/chrome/browser/extensions/chrome_extensions_browser_client_non_android.cc - can use `owner_rfh->GetLastCommittedOrigin().GetURL()`
Related details
- Alex Moshchuk
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |