[android] Refactor media_session_client

[zhongqiliang]

1. removed mutex lock in OnMediaSessionStateChanged() in android_media_session_client.cc since it does nothing in the mutex block.
2. removed if (ext_image.src == nullptr) check in OnMediaSessionStateChanged() in media_session_client.cc as media_image.src().c_str() should never return nullptr.
3. earlier return when if (extension_ && extension_->version >= 1) check failed
4. created ext_artwork with vector instead of array.

b/329330347

[datadog-cobalt-youtube]

Datadog Report

Branch report: b-329330347-media_session_client
Commit report: 7985305
Test service: cobalt

✅ 0 Failed, 30191 Passed, 1 Skipped, 14m 25.36s Wall Time

[codecov]

Codecov Report

Attention: Patch coverage is 5.12821% with 37 lines in your changes are missing coverage. Please review.

Project coverage is 58.88%. Comparing base (c65b33a) to head (7985305).
Report is 109 commits behind head on main.

Files	Patch %	Lines
cobalt/media_session/media_session_client.cc	5.12%	37 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2599      +/-   ##
==========================================
+ Coverage   58.17%   58.88%   +0.70%     
==========================================
  Files        1788     1904     +116     
  Lines       84294    93310    +9016     
==========================================
+ Hits        49038    54941    +5903     
- Misses      35256    38369    +3113     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[sideb0ard]

I'm always a bit reluctant to add a mutex if it can be avoided.
If the issue here is that the reference to MediaMetadataInit goes out of scope, at https://source.corp.google.com/h/lbshell-internal/cobalt_src/+/COBALT:cobalt/media_session/media_session_client.cc;l=325;drc=dff48f461fe8b269ff79460332dbd5d258262b99;bpv=1;bpt=0 - could we just take a copy of MediaMetadataInit?

[zhongqiliang]

From here, https://source.corp.google.com/h/lbshell-internal/cobalt_src/+/COBALT:cobalt/media_session/media_session.cc;l=99-114?q=p:cobalt%20UpdateMediaSessionState, do we know if UpdateMediaSessionState() can be called on multiple threads? If this is the case, I feel there could be a chance that 1 thread is reading session_state_ fields inside OnMediaSessionStateChanged https://source.corp.google.com/h/lbshell-internal/cobalt_src/+/COBALT:cobalt/media_session/media_session_client.cc;l=300-350, while the other thread can rewrite session_state_ here https://source.corp.google.com/h/lbshell-internal/cobalt_src/+/COBALT:cobalt/media_session/media_session_client.cc;l=275-278. Thoughts?

[sideb0ard]

do we know if UpdateMediaSessionState() can be called on multiple threads?

It doesn't look like it.
I see it being called twice, once in media_session_client which DCHECKs its on the media_session_->task_runner_, and once in media_session which also checks its on the task_runner_->BelongsToCurrentThread()

[zhongqiliang]

I see, thank you. One thing to confirm, "If the issue here is that the reference to MediaMetadataInit goes out of scope", do you meant the ext_image object here, https://source.corp.google.com/h/lbshell-internal/cobalt_src/+/COBALT:cobalt/media_session/media_session_client.cc;l=333, is it?