Commit 157403a9 authored by Peter Kasting's avatar Peter Kasting Committed by Commit Bot

Misc. cleanup for c/b/ui/views/apps/.

Mostly initializing variables in declarations and shortening code.

Also changed AppInfoDialogViewsTest.ViewInStore to call the link
callback instead of pressing the link directly, since in a future CL
I'll be removing the link member variable, so the test wouldn't work.

Bug: none
Change-Id: I90c689be86785a9b297b3161dafb01a0a8bb3568
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1983510
Auto-Submit: Peter Kasting <pkasting@chromium.org>
Commit-Queue: Trent Apted <tapted@chromium.org>
Reviewed-by: default avatarTrent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#728452}
parent b1a051af
...@@ -59,12 +59,10 @@ class AppInfoDialogTestApi { ...@@ -59,12 +59,10 @@ class AppInfoDialogTestApi {
public: public:
explicit AppInfoDialogTestApi(AppInfoDialog* dialog) : dialog_(dialog) {} explicit AppInfoDialogTestApi(AppInfoDialog* dialog) : dialog_(dialog) {}
AppInfoHeaderPanel* header_panel() { void ShowAppInWebStore() {
return static_cast<AppInfoHeaderPanel*>(dialog_->children().front()); auto* header_panel =
} static_cast<AppInfoHeaderPanel*>(dialog_->children().front());
return header_panel->ShowAppInWebStore();
views::Link* view_in_store_link() {
return header_panel()->view_in_store_link_;
} }
private: private:
...@@ -264,16 +262,14 @@ TEST_F(AppInfoDialogViewsTest, DestroyedOtherProfileDoesNotCloseDialog) { ...@@ -264,16 +262,14 @@ TEST_F(AppInfoDialogViewsTest, DestroyedOtherProfileDoesNotCloseDialog) {
// dialog cleanly. // dialog cleanly.
TEST_F(AppInfoDialogViewsTest, ViewInStore) { TEST_F(AppInfoDialogViewsTest, ViewInStore) {
ShowAppInfo(kTestExtensionId); ShowAppInfo(kTestExtensionId);
EXPECT_TRUE(extension_->from_webstore()); // Otherwise there is no link. ASSERT_TRUE(extension_->from_webstore());
views::Link* link = test::AppInfoDialogTestApi(dialog_).view_in_store_link();
EXPECT_TRUE(link);
TabStripModel* tabs = browser()->tab_strip_model(); TabStripModel* tabs = browser()->tab_strip_model();
EXPECT_EQ(0, tabs->count()); EXPECT_EQ(0, tabs->count());
ASSERT_TRUE(widget_); ASSERT_TRUE(widget_);
EXPECT_FALSE(widget_->IsClosed()); EXPECT_FALSE(widget_->IsClosed());
link->OnKeyPressed(ui::KeyEvent(ui::ET_KEY_PRESSED, ui::VKEY_SPACE, 0)); test::AppInfoDialogTestApi(dialog_).ShowAppInWebStore();
ASSERT_TRUE(widget_); ASSERT_TRUE(widget_);
EXPECT_TRUE(widget_->IsClosed()); EXPECT_TRUE(widget_->IsClosed());
......
...@@ -63,6 +63,10 @@ AppInfoHeaderPanel::AppInfoHeaderPanel(Profile* profile, ...@@ -63,6 +63,10 @@ AppInfoHeaderPanel::AppInfoHeaderPanel(Profile* profile,
AppInfoHeaderPanel::~AppInfoHeaderPanel() { AppInfoHeaderPanel::~AppInfoHeaderPanel() {
} }
void AppInfoHeaderPanel::OnIconUpdated(extensions::ChromeAppIcon* icon) {
app_icon_view_->SetImage(icon->image_skia());
}
void AppInfoHeaderPanel::CreateControls() { void AppInfoHeaderPanel::CreateControls() {
auto app_icon_view = std::make_unique<views::ImageView>(); auto app_icon_view = std::make_unique<views::ImageView>();
app_icon_view->SetImageSize(gfx::Size(kAppIconSize, kAppIconSize)); app_icon_view->SetImageSize(gfx::Size(kAppIconSize, kAppIconSize));
...@@ -88,13 +92,12 @@ void AppInfoHeaderPanel::CreateControls() { ...@@ -88,13 +92,12 @@ void AppInfoHeaderPanel::CreateControls() {
vertical_info_container_ptr->AddChildView(std::move(app_name_label)); vertical_info_container_ptr->AddChildView(std::move(app_name_label));
if (CanShowAppInWebStore()) { if (CanShowAppInWebStore()) {
auto view_in_store_link = std::make_unique<views::Link>( auto* view_in_store_link =
l10n_util::GetStringUTF16(IDS_APPLICATION_INFO_WEB_STORE_LINK)); vertical_info_container_ptr->AddChildView(std::make_unique<views::Link>(
l10n_util::GetStringUTF16(IDS_APPLICATION_INFO_WEB_STORE_LINK)));
view_in_store_link->SetHorizontalAlignment(gfx::ALIGN_LEFT); view_in_store_link->SetHorizontalAlignment(gfx::ALIGN_LEFT);
view_in_store_link->set_listener(this); view_in_store_link->set_listener(this);
view_in_store_link->SetFocusBehavior(views::View::FocusBehavior::ALWAYS); view_in_store_link->SetFocusBehavior(views::View::FocusBehavior::ALWAYS);
view_in_store_link_ = vertical_info_container_ptr->AddChildView(
std::move(view_in_store_link));
} else { } else {
// If there's no link, allow the app's name to take up multiple lines. // If there's no link, allow the app's name to take up multiple lines.
// TODO(sashab): Limit the number of lines to 2. // TODO(sashab): Limit the number of lines to 2.
...@@ -103,14 +106,9 @@ void AppInfoHeaderPanel::CreateControls() { ...@@ -103,14 +106,9 @@ void AppInfoHeaderPanel::CreateControls() {
} }
void AppInfoHeaderPanel::LinkClicked(views::Link* source, int event_flags) { void AppInfoHeaderPanel::LinkClicked(views::Link* source, int event_flags) {
DCHECK_EQ(source, view_in_store_link_);
ShowAppInWebStore(); ShowAppInWebStore();
} }
void AppInfoHeaderPanel::OnIconUpdated(extensions::ChromeAppIcon* icon) {
app_icon_view_->SetImage(icon->image_skia());
}
void AppInfoHeaderPanel::ShowAppInWebStore() { void AppInfoHeaderPanel::ShowAppInWebStore() {
DCHECK(CanShowAppInWebStore()); DCHECK(CanShowAppInWebStore());
Close(); Close();
......
...@@ -39,14 +39,14 @@ class AppInfoHeaderPanel : public AppInfoPanel, ...@@ -39,14 +39,14 @@ class AppInfoHeaderPanel : public AppInfoPanel,
private: private:
friend class test::AppInfoDialogTestApi; friend class test::AppInfoDialogTestApi;
void CreateControls();
// Overridden from views::LinkListener: // Overridden from views::LinkListener:
void LinkClicked(views::Link* source, int event_flags) override; void LinkClicked(views::Link* source, int event_flags) override;
// extensions::ChromeAppIconDelegate: // extensions::ChromeAppIconDelegate:
void OnIconUpdated(extensions::ChromeAppIcon* icon) override; void OnIconUpdated(extensions::ChromeAppIcon* icon) override;
void CreateControls();
// Opens the app in the web store. Must only be called if // Opens the app in the web store. Must only be called if
// CanShowAppInWebStore() returns true. // CanShowAppInWebStore() returns true.
void ShowAppInWebStore(); void ShowAppInWebStore();
...@@ -54,7 +54,6 @@ class AppInfoHeaderPanel : public AppInfoPanel, ...@@ -54,7 +54,6 @@ class AppInfoHeaderPanel : public AppInfoPanel,
// UI elements on the dialog. Elements are nullptr if they are not displayed. // UI elements on the dialog. Elements are nullptr if they are not displayed.
views::ImageView* app_icon_view_ = nullptr; views::ImageView* app_icon_view_ = nullptr;
views::Link* view_in_store_link_ = nullptr;
std::unique_ptr<extensions::ChromeAppIcon> app_icon_; std::unique_ptr<extensions::ChromeAppIcon> app_icon_;
......
...@@ -98,11 +98,7 @@ base::string16 LaunchOptionsComboboxModel::GetItemAt(int index) { ...@@ -98,11 +98,7 @@ base::string16 LaunchOptionsComboboxModel::GetItemAt(int index) {
AppInfoSummaryPanel::AppInfoSummaryPanel(Profile* profile, AppInfoSummaryPanel::AppInfoSummaryPanel(Profile* profile,
const extensions::Extension* app) const extensions::Extension* app)
: AppInfoPanel(profile, app), : AppInfoPanel(profile, app) {
size_value_(nullptr),
homepage_link_(nullptr),
licenses_link_(nullptr),
launch_options_combobox_(nullptr) {
SetLayoutManager(std::make_unique<views::BoxLayout>( SetLayoutManager(std::make_unique<views::BoxLayout>(
views::BoxLayout::Orientation::kVertical, gfx::Insets(), views::BoxLayout::Orientation::kVertical, gfx::Insets(),
ChromeLayoutProvider::Get()->GetDistanceMetric( ChromeLayoutProvider::Get()->GetDistanceMetric(
...@@ -126,10 +122,10 @@ void AppInfoSummaryPanel::AddDescriptionAndLinksControl( ...@@ -126,10 +122,10 @@ void AppInfoSummaryPanel::AddDescriptionAndLinksControl(
DISTANCE_RELATED_CONTROL_VERTICAL_SMALL))); DISTANCE_RELATED_CONTROL_VERTICAL_SMALL)));
if (!app_->description().empty()) { if (!app_->description().empty()) {
const size_t max_length = 400; constexpr size_t kMaxLength = 400;
base::string16 text = base::UTF8ToUTF16(app_->description()); base::string16 text = base::UTF8ToUTF16(app_->description());
if (text.length() > max_length) { if (text.length() > kMaxLength) {
text = text.substr(0, max_length); text = text.substr(0, kMaxLength - 5);
text += base::ASCIIToUTF16(" ... "); text += base::ASCIIToUTF16(" ... ");
} }
...@@ -138,25 +134,18 @@ void AppInfoSummaryPanel::AddDescriptionAndLinksControl( ...@@ -138,25 +134,18 @@ void AppInfoSummaryPanel::AddDescriptionAndLinksControl(
description_and_labels_stack->AddChildView(std::move(description_label)); description_and_labels_stack->AddChildView(std::move(description_label));
} }
if (CanShowAppHomePage()) { const auto add_link = [&](int message_id) {
auto homepage_link = std::make_unique<views::Link>( auto* link = description_and_labels_stack->AddChildView(
l10n_util::GetStringUTF16(IDS_APPLICATION_INFO_HOMEPAGE_LINK)); std::make_unique<views::Link>(l10n_util::GetStringUTF16(message_id)));
homepage_link->SetHorizontalAlignment(gfx::ALIGN_LEFT); link->SetHorizontalAlignment(gfx::ALIGN_LEFT);
homepage_link->set_listener(this); link->set_listener(this);
homepage_link->SetFocusBehavior(views::View::FocusBehavior::ALWAYS); link->SetFocusBehavior(views::View::FocusBehavior::ALWAYS);
homepage_link_ = return link;
description_and_labels_stack->AddChildView(std::move(homepage_link)); };
} if (CanShowAppHomePage())
homepage_link_ = add_link(IDS_APPLICATION_INFO_HOMEPAGE_LINK);
if (CanDisplayLicenses()) { if (CanDisplayLicenses())
auto licenses_link = std::make_unique<views::Link>( licenses_link_ = add_link(IDS_APPLICATION_INFO_LICENSES_BUTTON_TEXT);
l10n_util::GetStringUTF16(IDS_APPLICATION_INFO_LICENSES_BUTTON_TEXT));
licenses_link->SetHorizontalAlignment(gfx::ALIGN_LEFT);
licenses_link->set_listener(this);
licenses_link->SetFocusBehavior(views::View::FocusBehavior::ALWAYS);
licenses_link_ =
description_and_labels_stack->AddChildView(std::move(licenses_link));
}
vertical_stack->AddChildView(std::move(description_and_labels_stack)); vertical_stack->AddChildView(std::move(description_and_labels_stack));
} }
...@@ -281,6 +270,7 @@ bool AppInfoSummaryPanel::CanSetLaunchType() const { ...@@ -281,6 +270,7 @@ bool AppInfoSummaryPanel::CanSetLaunchType() const {
return !app_->is_platform_app() && !app_->is_extension() && return !app_->is_platform_app() && !app_->is_extension() &&
app_->id() != extension_misc::kChromeAppId; app_->id() != extension_misc::kChromeAppId;
} }
void AppInfoSummaryPanel::ShowAppHomePage() { void AppInfoSummaryPanel::ShowAppHomePage() {
DCHECK(CanShowAppHomePage()); DCHECK(CanShowAppHomePage());
OpenLink(extensions::ManifestURL::GetHomepageURL(app_)); OpenLink(extensions::ManifestURL::GetHomepageURL(app_));
......
...@@ -26,7 +26,6 @@ class Extension; ...@@ -26,7 +26,6 @@ class Extension;
namespace views { namespace views {
class Combobox; class Combobox;
class Label; class Label;
class Link;
class View; class View;
} }
...@@ -79,12 +78,12 @@ class AppInfoSummaryPanel : public AppInfoPanel, ...@@ -79,12 +78,12 @@ class AppInfoSummaryPanel : public AppInfoPanel,
const std::vector<GURL> GetLicenseUrls() const; const std::vector<GURL> GetLicenseUrls() const;
// UI elements on the dialog. // UI elements on the dialog.
views::Label* size_value_; views::Label* size_value_ = nullptr;
views::Link* homepage_link_; views::View* homepage_link_ = nullptr;
views::Link* licenses_link_; views::View* licenses_link_ = nullptr;
std::unique_ptr<LaunchOptionsComboboxModel> launch_options_combobox_model_; std::unique_ptr<LaunchOptionsComboboxModel> launch_options_combobox_model_;
views::Combobox* launch_options_combobox_; views::Combobox* launch_options_combobox_ = nullptr;
base::WeakPtrFactory<AppInfoSummaryPanel> weak_ptr_factory_{this}; base::WeakPtrFactory<AppInfoSummaryPanel> weak_ptr_factory_{this};
......
...@@ -23,9 +23,7 @@ ...@@ -23,9 +23,7 @@
ArcAppInfoLinksPanel::ArcAppInfoLinksPanel(Profile* profile, ArcAppInfoLinksPanel::ArcAppInfoLinksPanel(Profile* profile,
const extensions::Extension* app) const extensions::Extension* app)
: AppInfoPanel(profile, app), : AppInfoPanel(profile, app) {
app_list_observer_(this),
manage_link_(nullptr) {
SetLayoutManager(std::make_unique<views::BoxLayout>( SetLayoutManager(std::make_unique<views::BoxLayout>(
views::BoxLayout::Orientation::kVertical, gfx::Insets(), views::BoxLayout::Orientation::kVertical, gfx::Insets(),
ChromeLayoutProvider::Get()->GetDistanceMetric( ChromeLayoutProvider::Get()->GetDistanceMetric(
...@@ -50,11 +48,9 @@ ArcAppInfoLinksPanel::ArcAppInfoLinksPanel(Profile* profile, ...@@ -50,11 +48,9 @@ ArcAppInfoLinksPanel::ArcAppInfoLinksPanel(Profile* profile,
ArcAppInfoLinksPanel::~ArcAppInfoLinksPanel() {} ArcAppInfoLinksPanel::~ArcAppInfoLinksPanel() {}
void ArcAppInfoLinksPanel::LinkClicked(views::Link* source, int event_flags) { void ArcAppInfoLinksPanel::LinkClicked(views::Link* source, int event_flags) {
DCHECK_EQ(manage_link_, source); gfx::NativeView native_view = GetWidget()->GetNativeView();
const int64_t display_id = const int64_t display_id =
display::Screen::GetScreen() display::Screen::GetScreen()->GetDisplayNearestView(native_view).id();
->GetDisplayNearestView(source->GetWidget()->GetNativeView())
.id();
if (arc::ShowPackageInfo( if (arc::ShowPackageInfo(
arc::ArcIntentHelperBridge::kArcIntentHelperPackageName, arc::ArcIntentHelperBridge::kArcIntentHelperPackageName,
arc::mojom::ShowPackageInfoPage::MANAGE_LINKS, display_id)) { arc::mojom::ShowPackageInfoPage::MANAGE_LINKS, display_id)) {
......
...@@ -46,8 +46,9 @@ class ArcAppInfoLinksPanel : public AppInfoPanel, ...@@ -46,8 +46,9 @@ class ArcAppInfoLinksPanel : public AppInfoPanel,
void UpdateLink(bool enabled); void UpdateLink(bool enabled);
ScopedObserver<ArcAppListPrefs, ArcAppListPrefs::Observer> app_list_observer_; ScopedObserver<ArcAppListPrefs, ArcAppListPrefs::Observer> app_list_observer_{
views::Link* manage_link_; this};
views::Link* manage_link_ = nullptr;
DISALLOW_COPY_AND_ASSIGN(ArcAppInfoLinksPanel); DISALLOW_COPY_AND_ASSIGN(ArcAppInfoLinksPanel);
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment