[Contextual Tasks] Implement Android Panel Host [chromium/src : main]
Sho Biswas (Gerrit)
Message from Sho Biswas![]( "Open in Gerrit")
Set Ready For Review
Related details
- Sho Biswas
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Shakti Sahu (Gerrit)
Shakti Sahu voted and added 3 comments![]( "Open in Gerrit")
Votes added by Shakti Sahu
| Code-Review | +1 |
3 comments
return !!GetTabAndroid();This feels wrong because if there isn't an active tab (somehow), it will go from uninitialized -> initialized -> uninitialized.
I dont know what `IsPanelInitialized` is useful for. On desktop I see that it's for tracking `web_view_` creation which is an async operation?
Should it be just `true` in android?
observers_.Notify(&ContextualTasksPanelHost::Observer::OnSurfaceStateChanged,This and line 42 invoke the OnSurfaceStateChanged which triggers this logic in the controller
https://source.chromium.org/chromium/chromium/src/+/main:chrome/browser/contextual_tasks/contextual_tasks_side_panel_coordinator.cc;drc=3f7112dccdaf8ad032ee03247030eb43cfc8d706;l=977
Based on SystemAction / Useraction we do different thigns there. Not super clear. Something to watch out and revisit
Related details
- Sho Biswas
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. |
Sho Biswas (Gerrit)
Sho Biswas added 2 comments![]( "Open in Gerrit")
This feels wrong because if there isn't an active tab (somehow), it will go from uninitialized -> initialized -> uninitialized.
I dont know what `IsPanelInitialized` is useful for. On desktop I see that it's for tracking `web_view_` creation which is an async operation?
Should it be just `true` in android?
It's just because the side panel framework on desktop is async, so there's a call to UpdateWebContentsForActiveTab when the side panel is initialized. I don't remember exactly what problems this caused, but maybe we can deprecate it.
For now, let's just go with true.
observers_.Notify(&ContextualTasksPanelHost::Observer::OnSurfaceStateChanged,This and line 42 invoke the OnSurfaceStateChanged which triggers this logic in the controller
https://source.chromium.org/chromium/chromium/src/+/main:chrome/browser/contextual_tasks/contextual_tasks_side_panel_coordinator.cc;drc=3f7112dccdaf8ad032ee03247030eb43cfc8d706;l=977Based on SystemAction / Useraction we do different thigns there. Not super clear. Something to watch out and revisit
Agreed -- the systemaction case was to address the UpdateWebContentsForActiveTab case I mentioned in the other comment. So I'll leave these as useraction since that's the more typical use case.
Related details
- Shakti Sahu
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Shakti Sahu (Gerrit)
Shakti Sahu voted Code-Review+1![]( "Open in Gerrit")
- Sho Biswas
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Khanh Nguyen (Gerrit)
Khanh Nguyen voted![]( "Open in Gerrit")
| Code-Review | +1 |
| Commit-Queue | +2 |
Related details
- Sho Biswas
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Shakti Sahu (Gerrit)
Shakti Sahu voted![]( "Open in Gerrit")
| Code-Review | +1 |
| Commit-Queue | +1 |
Related details
- Sho Biswas
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Shakti Sahu (Gerrit)
Shakti Sahu voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Unreviewed changes
9 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
Change information
[Contextual Tasks] Implement Android Panel Host
Provides the concrete Android implementation of ContextualTasksPanelHost
using the TabBottomSheetConnector.
This enables the Contextual Tasks feature to display its UI within
the Android bottom sheet container, with full support for lifecycle
events and WebContents management.
Also includes unit tests for the new host implementation.
- M chrome/browser/contextual_tasks/BUILD.gn
- M chrome/browser/contextual_tasks/android/contextual_tasks_panel_host_android.cc
- M chrome/browser/contextual_tasks/android/contextual_tasks_panel_host_android.h
Code-Review: +1 by Shakti Sahu, +1 by Khanh Nguyen
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |