Stop free scrolling on app hide

[andrewsavage1]

b/290655439
b/290846002

[andrewsavage1]

@jellefoks let me know if you think I'm using the wrong lifecycle states here

[kaidokert]

Resetting scroll on Conceal/Freeze makes sense to me

[codecov]

Codecov Report

Merging #901 (b3635ba) into main (0f2376d) will decrease coverage by 0.01%.
Report is 1 commits behind head on main.
The diff coverage is 10.00%.

@@            Coverage Diff             @@
##             main     #901      +/-   ##
==========================================
- Coverage   57.58%   57.58%   -0.01%     
==========================================
  Files        1905     1905              
  Lines       94754    94764      +10     
==========================================
+ Hits        54566    54570       +4     
- Misses      40188    40194       +6     

Files Changed	Coverage Δ
...obalt/ui_navigation/scroll_engine/scroll_engine.cc	0.00% <0.00%> (ø)
cobalt/ui_navigation/scroll_engine/scroll_engine.h	0.00% <0.00%> (ø)
cobalt/dom/html_element.cc	85.24% <50.00%> (-0.07%)	⬇️

... and 10 files with indirect coverage changes

[andrewsavage1]

@kaidokert @sherryzy would y'all be able to take another look after the more recent commits? Thanks!

[kaidokert]

@kaidokert @sherryzy would y'all be able to take another look after the more recent commits? Thanks!

Looks fine to me, the cleanup in destructor looks correct

[andrewsavage1]

Here's the current failure that I'm debugging, happens during the CancelSyncLoadsWhenSuspended black box test:

[cobalt/6330:0720/181732.787827:INFO:application.cc(1196)] Got blur event.
[DebugConsoleWebModule/6364:0720/181732.787986:INFO:application_lifecycle_state.cc(153)] SetApplicationState: kApplicationStateStarted (1) -> kApplicationStateBlurred (2)
[cobalt/6330:0720/181732.788248:FATAL:observer_list_internal.h(73)] Check failed: false. 
        base::internal::CheckedObserverAdapter::IsMarkedForRemoval() [0x56156fe7ba00]
        base::ObserverList<>::Iter::EnsureValidIndex() [0x56156fe7b194]
        base::ObserverList<>::Iter::operator++() [0x56156fe5736a]
        cobalt::browser::BrowserModule::Blur() [0x56156fe36e67]
        cobalt::browser::Application::OnApplicationEvent() [0x56156fe108f5]
        cobalt::browser::Application::HandleStarboardEvent() [0x56156fe1145c]
        SbEventHandle [0x56156cc07d78]
        starboard::shared::starboard::Application::HandleEventAndUpdateState() [0x561571e6c461]
        starboard::shared::starboard::Application::DispatchAndDelete() [0x561571e6ab3b]
        starboard::shared::starboard::Application::RunLoop() [0x561571e67a2e]
        starboard::shared::starboard::Application::Run() [0x561571e681d3]
        main [0x561571c6102e]
        __libc_start_main [0x7f53ab8ca09b]
        _start [0x56156cb3dfea]

Haven't been able to recreate it by manually sending SIGUSR1 to the cobalt process after launching it. Tried replacing the call to RemoveObserver with a PostTask that ensures it is done in the browser modular thread as splash screen destruction does, but that didn't seem to help.

I'm also noticing that Cobalt seem to crash in general during black box tests if it is hidden in some way, i.e. if I click to another window. That seems a little strange, as that is just a Blur event I believe—I wouldn't expect any constructors/destructors to be called or for the scroll engine to try to stop scrolling, so something else must be going on. I wonder if the lifecycle abstract events expect a certain call to be made in their implementation.

[andrewsavage1]

Even after 3623b78 (#901) I'm still occasionally running into the failing IsDesignated DCHECK in TopmostEventTarget. This happened when messing around on the secondary side nav for the settings page.

[andrewsavage1]

Even after 3623b78 (#901) I'm still occasionally running into the failing IsDesignated DCHECK in TopmostEventTarget. This happened when messing around on the secondary side nav for the settings page.

Oh, well of course it didn't, I didn't use the previous_html_element_weak_ pointer in the call to DispatchPointerEventsForScrollStart. Fixed in ed06432 (#901)

[andrewsavage1]

Merging to unblock Sherry in writing black box tests, but we'll keep an eye on if we see regressions in scroll behavior