Clean up telemetry_test and fix.

[aee-google]

b/330355045

The fix includes fixing the starboard implementation of MessagePump: MessagePumpUIStarboard and MessagePumpIOStarboard.

[jellefoks]

I tried this on a Sabrina device and it loaded the app but did not respond to remote keypresses. remote devtools worked, but the Quit button on the Cobalt app also got no response beyond a Request to stop log message (conceal worked, but revealing afterward by selecting the app again on the device results in a black screen and an ANR.

[aee-google]

Updated base/message_loop/message_pump_io_starboard.cc and base/message_loop/message_pump_ui_starboard.cc using

cobalt/base/message_loop/message_pump_libevent.cc

Line 273 in c0dfb97

void MessagePumpLibevent::Run(Delegate* delegate) {

as a reference.

[aee-google]

The failed checks are due to infra issues.

C:\BuildTools\VC\Tools\MSVC\14.34.31933\include\xtree(44): fatal error C1088:
    Cannot flush compiler intermediate file:
        'C:\Users\ContainerAdministrator\AppData\Local\Temp2\_CL_3c56ab4bex': No space left on device

[sherryzy]

Can you explain a little bit what's the original issue with the MessagePump and how do you fix it? It will be very helpful if you can provide more context here.

[jellefoks]

This is awesome. There is a lot of long overdue and complex cleanup in here. Thanks!

[aee-google]

The MessagePump::Delegate interface changed.

New:

Delegate::NextWorkInfo next_work_info = delegate->DoWork();

Old:

bool did_work = delegate->DoWork();
TimeDelta next_delayed_work_time;
did_work |= delegate->DoDelayedWork(&next_delayed_work_time);

The updated //base implementation simplifies the running of the next task into one call. If there is a delayed task that is ready to be done and and immediate task available, either task might be run in one iteration of the run loop. The DoWork() call returns the info about the next task to be run (unless preempted). This info is mainly held in next_work_info.delayed_run_time. NextWorkInfo also has next_work_info.recent_now which is used to determine the remaining delay in TimeDelta to wait until the next task is ready to be executed.

next_work_info.is_immediate() checks is delayed_run_time.is_null() which actually checks if the interval value of the TimeDelta is 0. A next_work_info.delayed_run_time with an internal value of 0 is an immediate task which should be executed immediately (no sleep/wait).

next_work_info.delayed_run_time.is_max() is a little clunky, but it means that there is no available next task, and we should idle until woken up.

next_work_info.remaining_delay() is the amount of time we should sleep/wait until running the next task. Similar to idle, we can still wake up if an immediate task is posted or if a new delayed task is posted with a more recent desired run time.

The old //base is different.

bool did_work = delegate->DoWork();
TimeDelta next_delayed_work_time;
did_work |= delegate->DoDelayedWork(&next_delayed_work_time);

If we did work, we should try to pull more work to do now. Another iteration should always be attempted.

We did update the MessagePump code to use the new Delegate::NextWorkInfo returned from DoWork(), but we using it subtly incorrectly.

I tried to fix just the issue. But the code as written was difficult to fix without breaking something else. This is what led me to modifying MessagePumpIOStarboard::Run() and MessagePumpUIStarboard::RunUntildle() to look more like MessagePumpLibevent::Run().

In addition to this, I changed SbEventIdSet outstanding_events_ to be absl::optional<SbEventId> outstanding_event_ since only one element is stored at a time. Same for outstanding_delayed_event_.

[aee-google]

Do I need to keep retrying the on_device checks that failed? Are they more flaky than the normal checks (which are also quite flaky)?

[sherryzy]

Thanks for the detailed explanation. For the failing the tests, maybe better do some retries once or twice to see if it's just flaky. And you can compare the test result with trunk as well. If it continue fails, maybe take a look if it fails for the same reason. Now, it seems like many Android tests fail.