saved_desks: Minor cleanups in SavedDeskItemView.
* Make `desk_template` return by const reference instead of pointer
since the value is always going to be there.
* Have `uuid` return by const ref instead of by value. The previous
implementation was likely returning by value because of the
underlying accessor in `DeskTemplate` which was also returning by
value.
* Have `DeskItemGridView` use the existing `uuid` function instead
of `item->desk_template().uuid()`.
* Make the constructor take a `unique_ptr<DeskTemplate>` to make it
obvious that it is taking over ownership.
* `SavedDeskIconContainer` doesn't need non-const access to the
template.
Change-Id: I333807e1304eea915b686e5cdea2210da0023bbf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3646807
Commit-Queue: Daniel Andersson <dandersson@chromium.org>
Reviewed-by: Yongshun Liu <yongshun@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1003897}
diff --git a/ash/public/cpp/desk_template.h b/ash/public/cpp/desk_template.h
index fa85aac..a3ce1ef 100644
--- a/ash/public/cpp/desk_template.h
+++ b/ash/public/cpp/desk_template.h
@@ -60,7 +60,7 @@
// A special value to use as an icon identifier for an incognito window.
static constexpr char kIncognitoWindowIdentifier[] = "incognito_window";
- base::GUID uuid() const { return uuid_; }
+ const base::GUID& uuid() const { return uuid_; }
DeskTemplateSource source() const { return source_; }
base::Time created_time() const { return created_time_; }
diff --git a/ash/wm/desks/templates/saved_desk_grid_view.cc b/ash/wm/desks/templates/saved_desk_grid_view.cc
index 6b4eaad..7cb9cc3 100644
--- a/ash/wm/desks/templates/saved_desk_grid_view.cc
+++ b/ash/wm/desks/templates/saved_desk_grid_view.cc
@@ -165,7 +165,7 @@
(*iter)->UpdateTemplate(*entry);
} else if (grid_items_.size() < kMaxTemplateCount) {
SavedDeskItemView* grid_item =
- AddChildView(std::make_unique<SavedDeskItemView>(entry));
+ AddChildView(std::make_unique<SavedDeskItemView>(entry->Clone()));
grid_items_.push_back(grid_item);
if (!initializing_grid_view)
new_grid_items.push_back(grid_item);
@@ -286,7 +286,7 @@
auto it = std::find_if(grid_items_.begin(), grid_items_.end(),
[&uuid](SavedDeskItemView* item_view) {
- return uuid == item_view->desk_template()->uuid();
+ return uuid == item_view->uuid();
});
return it == grid_items_.end() ? nullptr : *it;
}
diff --git a/ash/wm/desks/templates/saved_desk_icon_container.cc b/ash/wm/desks/templates/saved_desk_icon_container.cc
index ee9c4e0..fea1d0f0 100644
--- a/ash/wm/desks/templates/saved_desk_icon_container.cc
+++ b/ash/wm/desks/templates/saved_desk_icon_container.cc
@@ -161,7 +161,7 @@
SavedDeskIconContainer::~SavedDeskIconContainer() = default;
void SavedDeskIconContainer::PopulateIconContainerFromTemplate(
- DeskTemplate* desk_template) {
+ const DeskTemplate* desk_template) {
const app_restore::RestoreData* restore_data =
desk_template->desk_restore_data();
if (!restore_data)
diff --git a/ash/wm/desks/templates/saved_desk_icon_container.h b/ash/wm/desks/templates/saved_desk_icon_container.h
index 3696945..1e38b7a 100644
--- a/ash/wm/desks/templates/saved_desk_icon_container.h
+++ b/ash/wm/desks/templates/saved_desk_icon_container.h
@@ -65,7 +65,7 @@
// Given a saved desk, determine which icons to show in this and create
// the according SavedDeskIconView's.
- void PopulateIconContainerFromTemplate(DeskTemplate* desk_template);
+ void PopulateIconContainerFromTemplate(const DeskTemplate* desk_template);
// Given `windows`, determine which icons to show in this and create the
// according SavedDeskIconView's.
@@ -90,7 +90,7 @@
BEGIN_VIEW_BUILDER(/* no export */,
SavedDeskIconContainer,
views::BoxLayoutView)
-VIEW_BUILDER_METHOD(PopulateIconContainerFromTemplate, DeskTemplate*)
+VIEW_BUILDER_METHOD(PopulateIconContainerFromTemplate, const DeskTemplate*)
VIEW_BUILDER_METHOD(PopulateIconContainerFromWindows,
const std::vector<aura::Window*>&)
END_VIEW_BUILDER
diff --git a/ash/wm/desks/templates/saved_desk_item_view.cc b/ash/wm/desks/templates/saved_desk_item_view.cc
index f53d3ca..981bf96 100644
--- a/ash/wm/desks/templates/saved_desk_item_view.cc
+++ b/ash/wm/desks/templates/saved_desk_item_view.cc
@@ -113,8 +113,9 @@
} // namespace
-SavedDeskItemView::SavedDeskItemView(const DeskTemplate* desk_template)
- : desk_template_(desk_template->Clone()) {
+SavedDeskItemView::SavedDeskItemView(
+ std::unique_ptr<DeskTemplate> desk_template)
+ : desk_template_(std::move(desk_template)) {
auto launch_template_callback = base::BindRepeating(
&SavedDeskItemView::OnGridItemPressed, weak_ptr_factory_.GetWeakPtr());
@@ -617,8 +618,8 @@
[this, name](const SavedDeskItemView* d) {
// Name duplication is allowed if one of the templates is an admin
// template.
- return (d != this && d->desk_template()->template_name() == name &&
- d->desk_template()->source() != DeskTemplateSource::kPolicy);
+ return (d != this && d->desk_template_->template_name() == name &&
+ d->desk_template_->source() != DeskTemplateSource::kPolicy);
});
return iter == templates_grid_view_items.end() ? nullptr : *iter;
}
diff --git a/ash/wm/desks/templates/saved_desk_item_view.h b/ash/wm/desks/templates/saved_desk_item_view.h
index 7210a451..7526dbe 100644
--- a/ash/wm/desks/templates/saved_desk_item_view.h
+++ b/ash/wm/desks/templates/saved_desk_item_view.h
@@ -68,7 +68,7 @@
public:
METADATA_HEADER(SavedDeskItemView);
- explicit SavedDeskItemView(const DeskTemplate* desk_template);
+ explicit SavedDeskItemView(std::unique_ptr<DeskTemplate> desk_template);
SavedDeskItemView(const SavedDeskItemView&) = delete;
SavedDeskItemView& operator=(const SavedDeskItemView&) = delete;
~SavedDeskItemView() override;
@@ -76,9 +76,9 @@
// The preferred size of the whole SavedDeskItemView.
static constexpr gfx::Size kPreferredSize = {220, 120};
- DeskTemplate* desk_template() const { return desk_template_.get(); }
+ const DeskTemplate& desk_template() const { return *desk_template_; }
SavedDeskNameView* name_view() const { return name_view_; }
- const base::GUID uuid() const { return desk_template_->uuid(); }
+ const base::GUID& uuid() const { return desk_template_->uuid(); }
// Updates the visibility state of the delete and launch buttons depending on
// the current mouse or touch event location, or if switch access is enabled.
diff --git a/ash/wm/desks/templates/saved_desk_library_view.cc b/ash/wm/desks/templates/saved_desk_library_view.cc
index ef8e665..e6863089 100644
--- a/ash/wm/desks/templates/saved_desk_library_view.cc
+++ b/ash/wm/desks/templates/saved_desk_library_view.cc
@@ -324,7 +324,7 @@
std::string extra_diagnostics;
for (auto* grid : grid_views()) {
for (auto* item : grid->grid_items())
- extra_diagnostics += (item->desk_template()->ToString() + "\n");
+ extra_diagnostics += (item->desk_template().ToString() + "\n");
}
// Note that this will activate the dialog which will exit overview and delete
diff --git a/ash/wm/desks/templates/saved_desk_unittest.cc b/ash/wm/desks/templates/saved_desk_unittest.cc
index bfe3e60..b94678cc 100644
--- a/ash/wm/desks/templates/saved_desk_unittest.cc
+++ b/ash/wm/desks/templates/saved_desk_unittest.cc
@@ -3498,13 +3498,13 @@
// The actual template name will still have "(1)" appended to maintain its
// uniqueness.
EXPECT_EQ(u"Desk 1 (1)",
- GetItemViewFromTemplatesGrid(0)->desk_template()->template_name());
+ GetItemViewFromTemplatesGrid(0)->desk_template().template_name());
// Set the second template to have a new unique name by updating the model
// directly. This mimics updating the name on a different device and is the
// only way to change the name without prompting the replace dialog.
SavedDeskItemView* second_item = GetItemViewFromTemplatesGrid(1);
- auto new_desk_template = second_item->desk_template()->Clone();
+ auto new_desk_template = second_item->desk_template().Clone();
new_desk_template->set_template_name(u"Desk 2");
const base::GUID uuid = new_desk_template->uuid();
@@ -3533,7 +3533,7 @@
SavedDeskPresenter::Get()->EntriesAddedOrUpdatedRemotely(
{entry.get()});
ASSERT_EQ(u"Desk 2", second_item->name_view()->GetText());
- ASSERT_EQ(u"Desk 2", second_item->desk_template()->template_name());
+ ASSERT_EQ(u"Desk 2", second_item->desk_template().template_name());
loop1.Quit();
}));
loop1.Run();
