Make UrlLoadingBrowserAgent support navigating a background tab. [chromium/src : main]
Kirubel Aklilu (Gerrit)
Kirubel Aklilu voted and added 1 comment![]( "Open in Gerrit")
Votes added by Kirubel Aklilu
| Commit-Queue | +1 |
1 comment
@marq,
I found some more instances in the LoadUrlInCurrentTab impl that assumes the WebState is the currently active/foregrounded tab. I considered a few ways around this:
1. Revert the changes to LoadChangesInCurrentTab. Introduce a separate LoadChangesInTab which has an independent and simpler impl. (Although it seems not-great to have the behavior diverge).
1. Update the UrlLoadingNotifierBrowserAgent and underlying observers to take in the WebState instead of assuming the url was loaded in the active tab. There was only a handful of observers that needed to be updated (3 non-tests, [codesearch](https://source.chromium.org/search?q=(TabWillLoadUrl%7CTabDidLoadUrl%7CTabFailedToLoadUrl%7CTabDidPrerenderUrl%7CTabDidReloadUrl%7CTabDidLoadUrl%7CNewTabWillLoadUrl%7CNewTabDidLoadUrl%7CWillSwitchToTabWithUrl%7CDidSwitchToTabWithUrl)%20-f:url_loading%20%20&sq=)).
1. Update LoadUrlInTab throughout to check if `target_web_state` is the active tab. I chose against this to avoid unneeded nesting.
I ended up going with option 2. WDYT?
Related details
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. |