Skip to content

Commit

Permalink
[M115 merge][lacros desk restore] Add logging for lacros windows desk…
Browse files Browse the repository at this point in the history
… restore

A user experienced missing lacros windows when opening saved desk.
This is the only datapoint we have so far. This CL plants loggings
in prod to debug save-restore flow if same issue ever reported again.

This CL adds loggings at three locations where:
1, restore data is written into file
2, Ash tries to launch lacros windows from read data
3, lacros tries to create windows with data from Ash.

(cherry picked from commit 703f261)

Change-Id: Ib45945278598a437666c4992d19c6b278734a483
Bug: 1442076
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4604976
Reviewed-by: Daniel Andersson <dandersson@chromium.org>
Commit-Queue: pengchao Cai <pengchaocai@chromium.org>
Reviewed-by: Erik Chen <erikchen@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1157753}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4616050
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/5790@{#769}
Cr-Branched-From: 1d71a33-refs/heads/main@{#1148114}
  • Loading branch information
Pengchao Cai authored and Chromium LUCI CQ committed Jun 14, 2023
1 parent 36ac0e3 commit 4d57271
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 0 deletions.
4 changes: 4 additions & 0 deletions ash/wm/desks/templates/saved_desk_presenter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,10 @@ void SavedDeskPresenter::SaveOrUpdateSavedDesk(
AppendDuplicateNumberToDuplicateName(saved_desk->template_name()));
}

// TODO(crbug.com/1442076): Remove after issue is root caused.
LOG(ERROR) << "Windows written to file by Ash: \n"
<< saved_desk->ToDebugString();

// Save or update `desk_template` as an entry in DeskModel.
GetDeskModel()->AddOrUpdateEntry(
std::move(saved_desk),
Expand Down
5 changes: 5 additions & 0 deletions chrome/browser/lacros/desk_template_client_lacros.cc
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,11 @@ void DeskTemplateClientLacros::CreateBrowserWithRestoredData(
create_params.creation_source = Browser::CreationSource::kDeskTemplate;
Browser* browser = Browser::Create(create_params);

// TODO(crbug.com/1442076): Remove after issue is root caused.
LOG(ERROR) << "window " << additional_state->restore_window_id
<< " created by lacros with " << additional_state->urls.size()
<< " tabs";

for (size_t i = 0; i < additional_state->urls.size(); i++) {
chrome::AddTabAt(
browser, additional_state->urls.at(i), /*index=*/-1,
Expand Down
14 changes: 14 additions & 0 deletions chrome/browser/ui/ash/desks/desks_templates_app_launch_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -299,12 +299,23 @@ void DesksTemplatesAppLaunchHandler::MaybeLaunchLacrosBrowsers() {
if (app_id != app_constants::kLacrosAppId)
continue;

// Count the number of lacros windows ash intends to launch. Will be
// checked at lacros side to see if anything is missing between ash and
// lacros when restoring saved desk.
// TODO(crbug.com/1442076): Remove after issue is root caused.
int windows_count = 0;

for (const auto& [restore_window_id, app_restore_data] : iter.second) {
if (!app_restore_data->active_tab_index.has_value() ||
app_restore_data->urls.empty()) {
continue;
}

// TODO(crbug.com/1442076): Remove after issue is root caused.
windows_count++;
LOG(ERROR) << "window " << restore_window_id << " launched by Ash with "
<< app_restore_data->urls.size() << " tabs";

crosapi::BrowserManager::Get()->CreateBrowserWithRestoredData(
app_restore_data->urls,
app_restore_data->current_bounds.value_or(gfx::Rect()),
Expand All @@ -318,6 +329,9 @@ void DesksTemplatesAppLaunchHandler::MaybeLaunchLacrosBrowsers() {
app_restore_data->first_non_pinned_tab_index.value_or(0),
GetBrowserAppName(app_restore_data, app_id), restore_window_id);
}
// TODO(crbug.com/1442076): Remove after issue is root caused.
LOG(ERROR) << windows_count
<< " windows launched by Ash in total for this desk";
}
restore_data()->RemoveApp(app_constants::kLacrosAppId);
}
Expand Down

0 comments on commit 4d57271

Please sign in to comment.