Attach WebContents to a hidden Widget during password change [chromium/src : main]
Viktor Semeniuk (Gerrit)
Viktor Semeniuk voted and added 6 comments![]( "Open in Gerrit")
Votes added by Viktor Semeniuk
| Commit-Queue | +1 |
6 comments
Moving Robert and Keren to CC since I managed to get this working with a hidden Widget alone which is way simpler and appears to be working without any issues.
Vasilii PTAL
like rAF aren't propsagated. This results in APC failures. To resolveViktor Semeniuknit: typo
Done
class DetachedWindowTreeHost : public aura::WindowTreeHost {Keren ZhuAdd a quick comment above to indicate this is a special WindowTreeHost that will not actually show the underlying native_view.
Viktor Semeniuk+1. Also explains that this is needed to drive `requestAnimationFrame()` in a detached WebContents.
Done
CreateCompositor(false, false);Viktor SemeniukThese are default arguments to CreateCompositor. Are these values critical for this to work?
Acknowledged
void DetachedWindowTreeHost::ShowImpl() {}Viktor Semeniuknit: add a blank line between each function.
Acknowledged
// to which WebContents is attached. This helps to resolve some a problem with
// animations.Viktor Semeniuk```suggestion
// to which WebContents is attached. This helps to resolve the problem
// that requestAnimationFrame() is not fired on a detached WebContents.
```
Done
Related details
- Keren Zhu
- Robert Liao
- Vasilii Sukhanov
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. |
Vasilii Sukhanov (Gerrit)
Vasilii Sukhanov voted and added 3 comments![]( "Open in Gerrit")
Votes added by Vasilii Sukhanov
| Code-Review | +1 |
3 comments
{kWebContentsWidth, kWebContentsHeight}, false, true, true);Add comments for those paramaeters
/* is_renderer_initiated= */ false),`is_renderer_initiated=`
Related details
- Keren Zhu
- Robert Liao
- Viktor Semeniuk
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. |
Viktor Semeniuk (Gerrit)
Viktor Semeniuk added 4 comments![]( "Open in Gerrit")
Viktor Semeniuk438702587
Done
{kWebContentsWidth, kWebContentsHeight}, false, true, true);Add comments for those paramaeters
Done
/* is_renderer_initiated= */ false),Viktor Semeniuk`is_renderer_initiated=`
Done
Related details
- Keren Zhu
- Robert Liao
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. |
Viktor Semeniuk (Gerrit)
Viktor Semeniuk added 3 comments![]( "Open in Gerrit")
Erik ChenHi Erik and Peter,
I would like to get your opinion on the following implementation. The CL is not ready to be submitted, I'm just looking for an early feedback.
Viktor Semeniukconceptually, the idea of a making a custom window tree host to pump beginframe events makes sense. I'm not a graphics expert so I don't want to comment on the correct of the details of what you've written. Feel free to ping blundell to find an appropriate owner.
What is the plan for macOS support?
Colin BlundellAdding @blun...@chromium.org as a reviewer.
What is the plan for macOS support?
There are no problems with rAF on MacOS. This caused plenty of confusion during the early development since testing the same site can lead to a different result depending on the OS.
Viktor SemeniukThanks! Viktor and I chatted offline, and he's going to send an email to chrome-compositor@ with the context and link to solicit advice from the most knowledgeable folks.
Done
Viktor SemeniukThis approach seems simpler than the "hidden window" approach discussed over email. Can someone provide context about why we're moving in the hidden window approach vs this approach?
Peter KvitekOh, hidden window approach just came up because @kvi...@chromium.org is working on it to solve b/479056794. Originally it has nothing to do with APC or rAF problem we are tying to solve here. I just contacted Peter since he works on a headless mode.
Viktor SemeniukThis minimalist WTH implementation looks fine to me, however, there is no guarantee that it will be enough to run all the existing and future password change pages. As you found out, some happen to need rAF, others may need something else.
The 'hidden browser window' approach runs pages in exactly the same way they would have run normally, now and in the future, so it seems like a more reliable in the long run.
Done
Related details
- Erik Chen
- Keren Zhu
- Peter Kvitek
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Erik Chen (Gerrit)
Erik Chen voted and added 1 comment![]( "Open in Gerrit")
Votes added by Erik Chen
| Code-Review | +1 |
1 comment
I did a quick skim, overall lg, nice work
Related details
- Keren Zhu
- Peter Kvitek
- Viktor Semeniuk
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |