Skip to content

Commit

Permalink
Implement lacros tab groups save and launch
Browse files Browse the repository at this point in the history
CL implements logic for lacros tab groups and pinned tab save and launch.  Included tests for the DeskTemplateClientLacros which didn't exist before this CL.

TESTED: DeskTemplateLacrosBrowsertest.*

Design Doc: go/desk-template-browser-state
Implementation Plan: go/saved-desk-support-for-lacros-ip

Bug: b/247938864
Change-Id: I20c39b731b616a704c8d88576b6162be813e53d7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4083923
Reviewed-by: Sammie Quon <sammiequon@chromium.org>
Reviewed-by: Yanzhu Du <yzd@google.com>
Reviewed-by: Erik Chen <erikchen@chromium.org>
Commit-Queue: Avynn Donaghe <avynn@google.com>
Cr-Commit-Position: refs/heads/main@{#1096931}
  • Loading branch information
avynn authored and Chromium LUCI CQ committed Jan 25, 2023
1 parent afb53a6 commit 17b41b4
Show file tree
Hide file tree
Showing 13 changed files with 691 additions and 25 deletions.
1 change: 1 addition & 0 deletions chrome/browser/ash/crosapi/BUILD.gn
Expand Up @@ -308,6 +308,7 @@ source_set("crosapi") {
"//chromeos/startup:constants",
"//chromeos/ui/wm:wm",
"//chromeos/version",
"//components/app_restore",
"//components/arc",
"//components/crash/core/app",
"//components/exo",
Expand Down
29 changes: 20 additions & 9 deletions chrome/browser/ash/crosapi/browser_action.cc
Expand Up @@ -259,24 +259,30 @@ ui::mojom::WindowShowState ConvertWindowShowState(ui::WindowShowState state) {

class CreateBrowserWithRestoredDataAction final : public BrowserAction {
public:
CreateBrowserWithRestoredDataAction(const std::vector<GURL>& urls,
const gfx::Rect& bounds,
ui::WindowShowState show_state,
int32_t active_tab_index,
base::StringPiece app_name,
int32_t restore_window_id)
CreateBrowserWithRestoredDataAction(
const std::vector<GURL>& urls,
const gfx::Rect& bounds,
const std::vector<tab_groups::TabGroupInfo>& tab_group_infos,
ui::WindowShowState show_state,
int32_t active_tab_index,
int32_t first_non_pinned_tab_index,
base::StringPiece app_name,
int32_t restore_window_id)
: BrowserAction(true),
urls_(urls),
bounds_(bounds),
tab_group_infos_(tab_group_infos),
show_state_(show_state),
active_tab_index_(active_tab_index),
first_non_pinned_tab_index_(first_non_pinned_tab_index),
app_name_(app_name),
restore_window_id_(restore_window_id) {}

void Perform(const VersionedBrowserService& service) override {
crosapi::mojom::DeskTemplateStatePtr additional_state =
crosapi::mojom::DeskTemplateState::New(urls_, active_tab_index_,
app_name_, restore_window_id_);
crosapi::mojom::DeskTemplateState::New(
urls_, active_tab_index_, app_name_, restore_window_id_,
first_non_pinned_tab_index_, tab_group_infos_);
crosapi::CrosapiManager::Get()
->crosapi_ash()
->desk_template_ash()
Expand All @@ -288,8 +294,10 @@ class CreateBrowserWithRestoredDataAction final : public BrowserAction {
private:
const std::vector<GURL> urls_;
const gfx::Rect bounds_;
const std::vector<tab_groups::TabGroupInfo> tab_group_infos_;
const ui::WindowShowState show_state_;
const int32_t active_tab_index_;
const int32_t first_non_pinned_tab_index_;
const std::string app_name_;
const int32_t restore_window_id_;
};
Expand Down Expand Up @@ -368,12 +376,15 @@ std::unique_ptr<BrowserAction> BrowserAction::HandleTabScrubbing(
std::unique_ptr<BrowserAction> BrowserAction::CreateBrowserWithRestoredData(
const std::vector<GURL>& urls,
const gfx::Rect& bounds,
const std::vector<tab_groups::TabGroupInfo>& tab_groups,
ui::WindowShowState show_state,
int32_t active_tab_index,
int32_t first_non_pinned_tab_index,
base::StringPiece app_name,
int32_t restore_window_id) {
return std::make_unique<CreateBrowserWithRestoredDataAction>(
urls, bounds, show_state, active_tab_index, app_name, restore_window_id);
urls, bounds, tab_groups, show_state, active_tab_index,
first_non_pinned_tab_index, app_name, restore_window_id);
}

// No window will be opened in the following circumstances:
Expand Down
3 changes: 3 additions & 0 deletions chrome/browser/ash/crosapi/browser_action.h
Expand Up @@ -9,6 +9,7 @@
#include "base/strings/string_piece_forward.h"
#include "chrome/browser/ui/browser_navigator_params.h"
#include "chromeos/crosapi/mojom/crosapi.mojom.h"
#include "components/tab_groups/tab_group_info.h"
#include "ui/base/ui_base_types.h"

namespace crosapi {
Expand Down Expand Up @@ -59,8 +60,10 @@ class BrowserAction {
static std::unique_ptr<BrowserAction> CreateBrowserWithRestoredData(
const std::vector<GURL>& urls,
const gfx::Rect& bounds,
const std::vector<tab_groups::TabGroupInfo>& tab_group_infos,
ui::WindowShowState show_state,
int32_t active_tab_index,
int32_t first_non_pinned_tab_index,
base::StringPiece app_name,
int32_t restore_window_id);

Expand Down
5 changes: 4 additions & 1 deletion chrome/browser/ash/crosapi/browser_manager.cc
Expand Up @@ -692,12 +692,15 @@ void BrowserManager::HandleTabScrubbing(float x_offset) {
void BrowserManager::CreateBrowserWithRestoredData(
const std::vector<GURL>& urls,
const gfx::Rect& bounds,
const std::vector<tab_groups::TabGroupInfo>& tab_group_infos,
ui::WindowShowState show_state,
int32_t active_tab_index,
int32_t first_non_pinned_tab_index,
const std::string& app_name,
int32_t restore_window_id) {
PerformOrEnqueue(BrowserAction::CreateBrowserWithRestoredData(
urls, bounds, show_state, active_tab_index, app_name, restore_window_id));
urls, bounds, tab_group_infos, show_state, active_tab_index,
first_non_pinned_tab_index, app_name, restore_window_id));
}

void BrowserManager::InitializeAndStartIfNeeded() {
Expand Down
28 changes: 18 additions & 10 deletions chrome/browser/ash/crosapi/browser_manager.h
Expand Up @@ -31,13 +31,15 @@
#include "chromeos/ash/components/dbus/session_manager/session_manager_client.h"
#include "chromeos/crosapi/mojom/crosapi.mojom.h"
#include "chromeos/crosapi/mojom/desk_template.mojom.h"
#include "components/component_updater/component_updater_service.h"
#include "components/policy/core/common/cloud/cloud_policy_core.h"
#include "components/policy/core/common/cloud/cloud_policy_refresh_scheduler_observer.h"
#include "components/policy/core/common/cloud/cloud_policy_store.h"
#include "components/policy/core/common/cloud/component_cloud_policy_service_observer.h"
#include "components/policy/core/common/policy_namespace.h"
#include "components/policy/core/common/values_util.h"
#include "components/session_manager/core/session_manager_observer.h"
#include "components/tab_groups/tab_group_info.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
#include "ui/base/ui_base_types.h"
Expand Down Expand Up @@ -208,16 +210,22 @@ class BrowserManager : public session_manager::SessionManagerObserver,
// |x_offset| is in DIP coordinates.
void HandleTabScrubbing(float x_offset);

// Create a browser with the restored data containing |urls|,
// |bounds|, |show_state|, |active_tab_index| and |app_name|. Note an
// non-empty |app_name| indicates that the browser window is an app type
// browser window.
void CreateBrowserWithRestoredData(const std::vector<GURL>& urls,
const gfx::Rect& bounds,
const ui::WindowShowState show_state,
int32_t active_tab_index,
const std::string& app_name,
int32_t restore_window_id);
// Create a browser with the restored data containing `urls`,
// `bounds`,`tab_group_infos`, `show_state`, `active_tab_index`,
// `first_non_pinned_tab_index`, and `app_name`. Note an non-empty `app_name`
// indicates that the browser window is an app type browser window. Also
// note that` first_non_pinned_tab_indexes` with negative values are ignored
// type constratins for the `first_non_pinned_tab_index` and are enforced on
// the browser side and are dropped if they don't comply with said restraints.
void CreateBrowserWithRestoredData(
const std::vector<GURL>& urls,
const gfx::Rect& bounds,
const std::vector<tab_groups::TabGroupInfo>& tab_group_infos,
const ui::WindowShowState show_state,
int32_t active_tab_index,
int32_t first_non_pinned_tab_index,
const std::string& app_name,
int32_t restore_window_id);

// Initialize resources and start Lacros. This class provides two approaches
// to fulfill different requirements.
Expand Down
124 changes: 124 additions & 0 deletions chrome/browser/lacros/desk_template_client_lacros.cc
Expand Up @@ -4,6 +4,7 @@

#include "chrome/browser/lacros/desk_template_client_lacros.h"

#include "base/ranges/algorithm.h"
#include "chrome/browser/apps/icon_standardizer.h"
#include "chrome/browser/favicon/favicon_service_factory.h"
#include "chrome/browser/profiles/profile.h"
Expand All @@ -12,14 +13,54 @@
#include "chrome/browser/ui/browser_list.h"
#include "chrome/browser/ui/browser_tabstrip.h"
#include "chrome/browser/ui/browser_window.h"
#include "chrome/browser/ui/tabs/tab_group.h"
#include "chrome/browser/ui/tabs/tab_group_model.h"
#include "chrome/browser/ui/tabs/tab_strip_model.h"
#include "chromeos/lacros/lacros_service.h"
#include "components/favicon/core/favicon_service.h"
#include "components/favicon_base/favicon_util.h"
#include "components/tab_groups/tab_group_id.h"
#include "components/tab_groups/tab_group_info.h"
#include "components/tab_groups/tab_group_visual_data.h"
#include "ui/platform_window/platform_window.h"
#include "ui/views/widget/desktop_aura/desktop_window_tree_host_lacros.h"

namespace {

const std::vector<int> ConvertRangeToTabGroupIndices(const gfx::Range& range) {
std::vector<int> indices;

for (uint32_t index = range.start(); index < range.end(); ++index) {
indices.push_back(static_cast<int>(index));
}

return indices;
}

bool ValidateTabRange(const tab_groups::TabGroupInfo& group_info,
const TabStripModel* tab_strip_model) {
const gfx::Range& range = group_info.tab_range;

if (range.length() == 0) {
LOG(WARNING) << "group_info: range must have a non-zero length!";
return false;
}

if (range.start() > range.end()) {
LOG(WARNING)
<< "group_info: range.start() cannot be larger than range.end()!";
return false;
}

if (range.GetMax() >= static_cast<uint32_t>(tab_strip_model->count())) {
LOG(WARNING)
<< "group_info: range max cannot be larger than count of tabs!";
return false;
}

return true;
}

// Creates a callback for when a favicon image is retrieved which creates a
// standard icon image and then calls `callback` with the standardized image.
base::OnceCallback<void(const favicon_base::FaviconImageResult&)>
Expand All @@ -36,8 +77,69 @@ CreateFaviconResultCallback(
std::move(callback));
}

void AddTabGroupToBrowser(TabStripModel* browser_tab_model,
const tab_groups::TabGroupInfo& group_info) {
if (!ValidateTabRange(group_info, browser_tab_model)) {
return;
}

const std::vector<int> tab_range =
ConvertRangeToTabGroupIndices(group_info.tab_range);
tab_groups::TabGroupId new_group_id =
browser_tab_model->AddToNewGroup(tab_range);
browser_tab_model->group_model()
->GetTabGroup(new_group_id)
->SetVisualData(group_info.visual_data);
}

void PopulateTabGroups(const std::vector<tab_groups::TabGroupInfo>& group_infos,
Browser* out_browser) {
TabStripModel* browser_tab_model = out_browser->tab_strip_model();
DCHECK(browser_tab_model);

for (const auto& group : group_infos) {
AddTabGroupToBrowser(browser_tab_model, group);
}
}

void SetPinnedTabs(const int first_non_pinned_tab_index, Browser* out_browser) {
TabStripModel* browser_tab_model = out_browser->tab_strip_model();
DCHECK(browser_tab_model);

if (first_non_pinned_tab_index < 0 ||
first_non_pinned_tab_index >= out_browser->tab_strip_model()->count()) {
LOG(WARNING) << "Pinned tab outside of tab bounds!";
return;
}

for (int index = 0; index < first_non_pinned_tab_index; ++index) {
browser_tab_model->SetTabPinned(index, /*pinned=*/true);
}
}

void ConvertTabGroupsToTabGroupInfos(
const TabGroupModel* group_model,
crosapi::mojom::DeskTemplateState* out_state) {
DCHECK(group_model);
const std::vector<tab_groups::TabGroupId>& listed_group_ids =
group_model->ListTabGroups();

if (listed_group_ids.size() == 0) {
return;
}

out_state->groups = std::vector<tab_groups::TabGroupInfo>();
for (const tab_groups::TabGroupId& group_id : listed_group_ids) {
const TabGroup* tab_group = group_model->GetTabGroup(group_id);
out_state->groups->emplace_back(
gfx::Range(tab_group->ListTabs()),
tab_groups::TabGroupVisualData(*(tab_group->visual_data())));
}
}

} // namespace

// DeskTemplateClientLacros
DeskTemplateClientLacros::DeskTemplateClientLacros() {
auto* const lacros_service = chromeos::LacrosService::Get();
if (lacros_service->IsAvailable<crosapi::mojom::DeskTemplate>()) {
Expand All @@ -57,6 +159,7 @@ void DeskTemplateClientLacros::CreateBrowserWithRestoredData(

const absl::optional<std::string>& browser_app_name =
additional_state->browser_app_name;

Browser::CreateParams create_params =
browser_app_name.has_value() && !browser_app_name.value().empty()
? Browser::CreateParams::CreateForApp(browser_app_name.value(),
Expand All @@ -71,12 +174,20 @@ void DeskTemplateClientLacros::CreateBrowserWithRestoredData(
create_params.initial_bounds = bounds;
create_params.restore_id = additional_state->restore_window_id;
Browser* browser = Browser::Create(create_params);

for (size_t i = 0; i < additional_state->urls.size(); i++) {
chrome::AddTabAt(
browser, additional_state->urls.at(i), /*index=*/-1,
/*foreground=*/
(i == static_cast<size_t>(additional_state->active_index)));
}

if (additional_state->groups.has_value()) {
PopulateTabGroups(additional_state->groups.value(), browser);
}

SetPinnedTabs(additional_state->first_non_pinned_index, browser);

if (show_state == ui::mojom::WindowShowState::SHOW_STATE_MINIMIZED) {
browser->window()->Minimize();
} else {
Expand All @@ -89,6 +200,7 @@ void DeskTemplateClientLacros::GetBrowserInformation(
const std::string& window_unique_id,
GetBrowserInformationCallback callback) {
Browser* browser = nullptr;

for (auto* b : *BrowserList::GetInstance()) {
if (views::DesktopWindowTreeHostLacros::From(
b->window()->GetNativeWindow()->GetHost())
Expand All @@ -108,10 +220,22 @@ void DeskTemplateClientLacros::GetBrowserInformation(
crosapi::mojom::DeskTemplateState::New();
TabStripModel* tab_strip_model = browser->tab_strip_model();
state->active_index = tab_strip_model->active_index();
state->first_non_pinned_index = tab_strip_model->IndexOfFirstNonPinnedTab();

if (browser->type() == Browser::Type::TYPE_APP) {
state->browser_app_name = browser->app_name();
}

for (int i = 0; i < tab_strip_model->count(); ++i) {
state->urls.push_back(
tab_strip_model->GetWebContentsAt(i)->GetLastCommittedURL());
}

if (tab_strip_model->group_model() != nullptr) {
ConvertTabGroupsToTabGroupInfos(tab_strip_model->group_model(),
state.get());
}

std::move(callback).Run(serial, window_unique_id, std::move(state));
}

Expand Down
6 changes: 4 additions & 2 deletions chrome/browser/lacros/desk_template_client_lacros.h
Expand Up @@ -5,6 +5,8 @@
#ifndef CHROME_BROWSER_LACROS_DESK_TEMPLATE_CLIENT_LACROS_H_
#define CHROME_BROWSER_LACROS_DESK_TEMPLATE_CLIENT_LACROS_H_

#include "base/observer_list.h"
#include "base/observer_list_types.h"
#include "base/task/cancelable_task_tracker.h"
#include "chromeos/crosapi/mojom/desk_template.mojom.h"
#include "mojo/public/cpp/bindings/receiver.h"
Expand All @@ -19,7 +21,6 @@ class DeskTemplateClientLacros : public crosapi::mojom::DeskTemplateClient {
DeskTemplateClientLacros& operator=(const DeskTemplateClientLacros&) = delete;
~DeskTemplateClientLacros() override;

private:
// DeskTemplateClient:
void CreateBrowserWithRestoredData(
const gfx::Rect& bounds,
Expand All @@ -31,7 +32,8 @@ class DeskTemplateClientLacros : public crosapi::mojom::DeskTemplateClient {
void GetFaviconImage(const GURL& url,
GetFaviconImageCallback callback) override;

// The cancelable task tracker used for retreiving icons from the favicon
private:
// The cancelable task tracker used for retrieving icons from the favicon
// service.
base::CancelableTaskTracker task_tracker_;

Expand Down

0 comments on commit 17b41b4

Please sign in to comment.