Skip to content

Commit

Permalink
[cc-side-panel] Send requested section when opening side panel
Browse files Browse the repository at this point in the history 
Sends the requested section from the NTP front-end via the NTP handler
to the customize chrome side panel controller code. We will use this
signal to request the side panel front-end to scroll to that requested
section in a follow-up CL.

Bug: 1402251
Change-Id: Icb5076d1fffc34cda13c566a6e42bdbad8fa14af
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4183104
Commit-Queue: Tibor Goldschwendt <tiborg@chromium.org>
Reviewed-by: Paul Adedeji <pauladedeji@google.com>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1096248}
  • Loading branch information
Tibor Goldschwendt authored and Chromium LUCI CQ committed Jan 24, 2023
1 parent 7490cc8 commit 8e65980
Show file tree
Hide file tree
Showing 13 changed files with 184 additions and 36 deletions.
36 changes: 26 additions & 10 deletions chrome/browser/resources/new_tab_page/app.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import {loadTimeData} from './i18n_setup.js';
import {IframeElement} from './iframe.js';
import {LogoElement} from './logo.js';
import {recordLoadDuration} from './metrics_utils.js';
import {PageCallbackRouter, PageHandlerRemote, Theme} from './new_tab_page.mojom-webui.js';
import {CustomizeChromeSection, PageCallbackRouter, PageHandlerRemote, Theme} from './new_tab_page.mojom-webui.js';
import {NewTabPageProxy} from './new_tab_page_proxy.js';
import {$$} from './utils.js';
import {Action as VoiceAction, recordVoiceAction} from './voice_search_overlay.js';
Expand Down Expand Up @@ -383,7 +383,7 @@ export class AppElement extends PolymerElement {
});
// Open Customize Chrome if there are Customize Chrome URL params.
if (this.showCustomize_) {
this.pageHandler_.setCustomizeChromeSidePanelVisible(this.showCustomize_);
this.setCustomizeChromeSidePanelVisible_(this.showCustomize_);
recordCustomizeChromeOpen(NtpCustomizeChromeEntryPoint.URL);
}
this.eventTracker_.add(window, 'message', (event: MessageEvent) => {
Expand Down Expand Up @@ -523,12 +523,10 @@ export class AppElement extends PolymerElement {
}

private onCustomizeClick_() {
// Let customize dialog or side panel decide what page or section to show.
this.selectedCustomizeDialogPage_ = null;
if (this.customizeChromeEnabled_) {
// TODO(crbug.com/1402251): Scroll to section requested by
// |this.selectedCustomizeDialogPage_|.
// Flip customize chrome's visibility e.g. if it is closed, open it.
this.pageHandler_.setCustomizeChromeSidePanelVisible(
!this.showCustomize_);
this.setCustomizeChromeSidePanelVisible_(!this.showCustomize_);
if (!this.showCustomize_) {
recordCustomizeChromeOpen(
NtpCustomizeChromeEntryPoint.CUSTOMIZE_BUTTON);
Expand Down Expand Up @@ -729,11 +727,29 @@ export class AppElement extends PolymerElement {

private onCustomizeModule_() {
this.showCustomize_ = true;
if (this.customizeChromeEnabled_) {
this.pageHandler_.setCustomizeChromeSidePanelVisible(this.showCustomize_);
}
this.selectedCustomizeDialogPage_ = CustomizeDialogPage.MODULES;
recordCustomizeChromeOpen(NtpCustomizeChromeEntryPoint.MODULE);
this.setCustomizeChromeSidePanelVisible_(this.showCustomize_);
}

private setCustomizeChromeSidePanelVisible_(visible: boolean) {
if (!this.customizeChromeEnabled_) {
return;
}
let section: CustomizeChromeSection = CustomizeChromeSection.kUnspecified;
switch (this.selectedCustomizeDialogPage_) {
case CustomizeDialogPage.BACKGROUNDS:
case CustomizeDialogPage.THEMES:
section = CustomizeChromeSection.kAppearance;
break;
case CustomizeDialogPage.SHORTCUTS:
section = CustomizeChromeSection.kShortcuts;
break;
case CustomizeDialogPage.MODULES:
section = CustomizeChromeSection.kModules;
break;
}
this.pageHandler_.setCustomizeChromeSidePanelVisible(visible, section);
}

private printPerformanceDatum_(
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/ui/BUILD.gn
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5334,6 +5334,7 @@ static_library("ui") {
"webauthn/transport_hover_list_model.h",
"webauthn/webauthn_ui_helpers.cc",
"webauthn/webauthn_ui_helpers.h",
"webui/side_panel/customize_chrome/customize_chrome_section.h",
"window_name_prompt/window_name_prompt.cc",
]

Expand Down
6 changes: 4 additions & 2 deletions chrome/browser/ui/side_panel/customize_chrome/customize_chrome_tab_helper.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "chrome/browser/ui/side_panel/customize_chrome/customize_chrome_tab_helper.h"

#include "chrome/browser/ui/side_panel/customize_chrome/customize_chrome_side_panel_controller_utils.h"
#include "chrome/browser/ui/webui/side_panel/customize_chrome/customize_chrome_section.h"

CustomizeChromeTabHelper::~CustomizeChromeTabHelper() = default;

Expand All @@ -19,9 +20,10 @@ void CustomizeChromeTabHelper::DeregisterEntry() {
}

void CustomizeChromeTabHelper::SetCustomizeChromeSidePanelVisible(
bool visible) {
bool visible,
CustomizeChromeSection section) {
DCHECK(delegate_);
delegate_->SetCustomizeChromeSidePanelVisible(visible);
delegate_->SetCustomizeChromeSidePanelVisible(visible, section);
}

bool CustomizeChromeTabHelper::IsCustomizeChromeEntryShowing() const {
Expand Down
13 changes: 10 additions & 3 deletions chrome/browser/ui/side_panel/customize_chrome/customize_chrome_tab_helper.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#ifndef CHROME_BROWSER_UI_SIDE_PANEL_CUSTOMIZE_CHROME_CUSTOMIZE_CHROME_TAB_HELPER_H_
#define CHROME_BROWSER_UI_SIDE_PANEL_CUSTOMIZE_CHROME_CUSTOMIZE_CHROME_TAB_HELPER_H_

#include "chrome/browser/ui/webui/side_panel/customize_chrome/customize_chrome_section.h"
#include "content/public/browser/web_contents_user_data.h"

namespace content {
Expand All @@ -23,7 +24,9 @@ class CustomizeChromeTabHelper
public:
virtual void CreateAndRegisterEntry() = 0;
virtual void DeregisterEntry() = 0;
virtual void SetCustomizeChromeSidePanelVisible(bool visible) = 0;
virtual void SetCustomizeChromeSidePanelVisible(
bool visible,
CustomizeChromeSection section) = 0;
virtual bool IsCustomizeChromeEntryShowing() const = 0;
virtual bool IsCustomizeChromeEntryAvailable() const = 0;
virtual ~Delegate() = default;
Expand All @@ -43,7 +46,9 @@ class CustomizeChromeTabHelper
void DeregisterEntry();

// Opens and closes Side Panel to the customize chrome entry.
void SetCustomizeChromeSidePanelVisible(bool visible);
virtual void SetCustomizeChromeSidePanelVisible(
bool visible,
CustomizeChromeSection section);

// True if the side panel is open and showing the customize chrome entry.
bool IsCustomizeChromeEntryShowing() const;
Expand All @@ -58,9 +63,11 @@ class CustomizeChromeTabHelper
// Sets callback that is run when side panel entry state is changed
void SetCallback(StateChangedCallBack callback);

protected:
explicit CustomizeChromeTabHelper(content::WebContents* web_contents);

private:
friend class content::WebContentsUserData<CustomizeChromeTabHelper>;
explicit CustomizeChromeTabHelper(content::WebContents* web_contents);

std::unique_ptr<Delegate> delegate_;

Expand Down
4 changes: 3 additions & 1 deletion ...me/browser/ui/views/side_panel/customize_chrome/customize_chrome_side_panel_controller.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@ void CustomizeChromeSidePanelController::DeregisterEntry() {
}

void CustomizeChromeSidePanelController::SetCustomizeChromeSidePanelVisible(
bool visible) {
bool visible,
CustomizeChromeSection section) {
auto* browser_view = GetBrowserView();
if (!browser_view)
return;
Expand All @@ -76,6 +77,7 @@ void CustomizeChromeSidePanelController::SetCustomizeChromeSidePanelVisible(
} else {
browser_view->side_panel_coordinator()->Close();
}
// TODO(crbug.com/1402251): Scroll to requested `section`.
}

bool CustomizeChromeSidePanelController::IsCustomizeChromeEntryShowing() const {
Expand Down
5 changes: 4 additions & 1 deletion chrome/browser/ui/views/side_panel/customize_chrome/customize_chrome_side_panel_controller.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "chrome/browser/ui/side_panel/customize_chrome/customize_chrome_tab_helper.h"
#include "chrome/browser/ui/views/frame/browser_view.h"
#include "chrome/browser/ui/views/side_panel/side_panel_entry_observer.h"
#include "chrome/browser/ui/webui/side_panel/customize_chrome/customize_chrome_section.h"

namespace content {
class WebContents;
Expand All @@ -31,7 +32,9 @@ class CustomizeChromeSidePanelController
// CustomizeChromeTabHelper::Delegate
void CreateAndRegisterEntry() override;
void DeregisterEntry() override;
void SetCustomizeChromeSidePanelVisible(bool visible) override;
void SetCustomizeChromeSidePanelVisible(
bool visible,
CustomizeChromeSection section) override;
bool IsCustomizeChromeEntryShowing() const override;
bool IsCustomizeChromeEntryAvailable() const override;

Expand Down
10 changes: 9 additions & 1 deletion chrome/browser/ui/webui/new_tab_page/new_tab_page.mojom
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,13 @@ enum DoodleShareChannel {
kLinkCopy,
};

enum CustomizeChromeSection {
kUnspecified,
kAppearance,
kShortcuts,
kModules,
};

struct PromoImagePart {
url.mojom.Url image_url;
url.mojom.Url target;
Expand Down Expand Up @@ -287,7 +294,8 @@ interface PageHandler {
// Log user's FRE |optInStatus|.
LogModulesFreOptInStatus(OptInStatus opt_in_status);
// Shows or hides the customize chrome in unified side panel.
SetCustomizeChromeSidePanelVisible(bool visible);
SetCustomizeChromeSidePanelVisible(bool visible,
CustomizeChromeSection section);

// ======= METRICS =======
// Logs that the One Google Bar was added to the DOM / loaded in an iframe at
Expand Down
23 changes: 21 additions & 2 deletions chrome/browser/ui/webui/new_tab_page/new_tab_page_handler.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
#include "chrome/browser/ui/side_panel/customize_chrome/customize_chrome_tab_helper.h"
#include "chrome/browser/ui/side_panel/customize_chrome/customize_chrome_utils.h"
#include "chrome/browser/ui/webui/new_tab_page/ntp_pref_names.h"
#include "chrome/browser/ui/webui/side_panel/customize_chrome/customize_chrome_section.h"
#include "chrome/browser/ui/webui/webui_util.h"
#include "chrome/common/chrome_features.h"
#include "chrome/common/pref_names.h"
Expand Down Expand Up @@ -835,10 +836,28 @@ void NewTabPageHandler::LogModulesFreOptInStatus(
}
}

void NewTabPageHandler::SetCustomizeChromeSidePanelVisible(bool visible) {
void NewTabPageHandler::SetCustomizeChromeSidePanelVisible(
bool visible,
new_tab_page::mojom::CustomizeChromeSection section) {
CustomizeChromeSection section_enum;
switch (section) {
case new_tab_page::mojom::CustomizeChromeSection::kUnspecified:
section_enum = CustomizeChromeSection::kUnspecified;
break;
case new_tab_page::mojom::CustomizeChromeSection::kAppearance:
section_enum = CustomizeChromeSection::kAppearance;
break;
case new_tab_page::mojom::CustomizeChromeSection::kShortcuts:
section_enum = CustomizeChromeSection::kShortcuts;
break;
case new_tab_page::mojom::CustomizeChromeSection::kModules:
section_enum = CustomizeChromeSection::kModules;
break;
}
auto* customize_chrome_tab_helper =
CustomizeChromeTabHelper::FromWebContents(web_contents_);
customize_chrome_tab_helper->SetCustomizeChromeSidePanelVisible(visible);
customize_chrome_tab_helper->SetCustomizeChromeSidePanelVisible(visible,
section_enum);
}

void NewTabPageHandler::OnPromoDataUpdated() {
Expand Down
4 changes: 3 additions & 1 deletion chrome/browser/ui/webui/new_tab_page/new_tab_page_handler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,9 @@ class NewTabPageHandler : public new_tab_page::mojom::PageHandler,
void UpdateModulesFreVisibility() override;
void LogModulesFreOptInStatus(
new_tab_page::mojom::OptInStatus opt_in_status) override;
void SetCustomizeChromeSidePanelVisible(bool visible) override;
void SetCustomizeChromeSidePanelVisible(
bool visible,
new_tab_page::mojom::CustomizeChromeSection section) override;
void OnAppRendered(double time) override;
void OnOneGoogleBarRendered(double time) override;
void OnPromoRendered(double time,
Expand Down
69 changes: 64 additions & 5 deletions chrome/browser/ui/webui/new_tab_page/new_tab_page_handler_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#include "chrome/browser/ui/side_panel/customize_chrome/customize_chrome_tab_helper.h"
#include "chrome/browser/ui/webui/new_tab_page/new_tab_page.mojom.h"
#include "chrome/browser/ui/webui/new_tab_page/new_tab_page_handler.h"
#include "chrome/browser/ui/webui/side_panel/customize_chrome/customize_chrome_section.h"
#include "chrome/browser/ui/webui/webui_util.h"
#include "chrome/common/chrome_features.h"
#include "chrome/common/pref_names.h"
Expand Down Expand Up @@ -168,6 +169,29 @@ class MockPromoService : public PromoService {
MOCK_METHOD(void, Refresh, (), (override));
};

class MockCustomizeChromeTabHelper : public CustomizeChromeTabHelper {
public:
static MockCustomizeChromeTabHelper* CreateForWebContents(
content::WebContents* contents) {
DCHECK(contents);
DCHECK(!contents->GetUserData(UserDataKey()));
contents->SetUserData(
UserDataKey(),
base::WrapUnique(new MockCustomizeChromeTabHelper(contents)));
return static_cast<MockCustomizeChromeTabHelper*>(
contents->GetUserData(UserDataKey()));
}

MOCK_METHOD(void,
SetCustomizeChromeSidePanelVisible,
(bool, CustomizeChromeSection),
(override));

private:
explicit MockCustomizeChromeTabHelper(content::WebContents* web_contents)
: CustomizeChromeTabHelper(web_contents) {}
};

std::unique_ptr<TestingProfile> MakeTestingProfile(
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory) {
TestingProfile::Builder profile_builder;
Expand Down Expand Up @@ -196,7 +220,10 @@ class NewTabPageHandlerTest : public testing::Test {
mock_ntp_custom_background_service_(profile_.get()),
mock_promo_service_(*static_cast<MockPromoService*>(
PromoServiceFactory::GetForProfile(profile_.get()))),
web_contents_(factory_.CreateWebContents(profile_.get())) {
web_contents_(factory_.CreateWebContents(profile_.get())),
mock_customize_chrome_tab_helper_(
MockCustomizeChromeTabHelper::CreateForWebContents(
web_contents_.get())) {
mock_hats_service_ = static_cast<MockHatsService*>(
HatsServiceFactory::GetInstance()->SetTestingFactoryAndUse(
profile_.get(), base::BindRepeating(&BuildMockHatsService)));
Expand Down Expand Up @@ -224,10 +251,8 @@ class NewTabPageHandlerTest : public testing::Test {
web_contents_->SetColorProviderSource(&mock_color_provider_source_);
const std::vector<std::pair<const std::string, int>> module_id_names = {
{"recipe_tasks", IDS_NTP_MODULES_RECIPE_TASKS_SENTENCE}};
CustomizeChromeTabHelper::CreateForWebContents(web_contents_.get());
auto* customize_chrome_tab_helper_ =
CustomizeChromeTabHelper::FromWebContents(web_contents_.get());
EXPECT_FALSE(customize_chrome_tab_helper_->IsCustomizeChromeEntryShowing());
EXPECT_FALSE(
mock_customize_chrome_tab_helper_->IsCustomizeChromeEntryShowing());
handler_ = std::make_unique<NewTabPageHandler>(
mojo::PendingReceiver<new_tab_page::mojom::PageHandler>(),
mock_page_.BindAndGetRemote(), profile_.get(),
Expand Down Expand Up @@ -286,6 +311,7 @@ class NewTabPageHandlerTest : public testing::Test {
const raw_ref<MockPromoService> mock_promo_service_;
content::TestWebContentsFactory factory_;
raw_ptr<content::WebContents> web_contents_; // Weak. Owned by factory_.
raw_ptr<MockCustomizeChromeTabHelper> mock_customize_chrome_tab_helper_;
base::HistogramTester histogram_tester_;
std::unique_ptr<NewTabPageHandler> handler_;
// This field is not a raw_ptr<> because it was filtered by the rewriter for:
Expand Down Expand Up @@ -909,3 +935,36 @@ TEST_F(NewTabPageHandlerTest, ModulesVisiblePrefChangeTriggersPageCall) {
EXPECT_CALL(mock_page_, SetDisabledModules).Times(1);
mock_page_.FlushForTesting();
}

TEST_F(NewTabPageHandlerTest, SetCustomizeChromeSidePanelVisible) {
bool visible;
CustomizeChromeSection section;
EXPECT_CALL(*mock_customize_chrome_tab_helper_,
SetCustomizeChromeSidePanelVisible)
.Times(1)
.WillOnce(testing::DoAll(testing::SaveArg<0>(&visible),
testing::SaveArg<1>(&section)));

handler_->SetCustomizeChromeSidePanelVisible(
/*visible=*/true,
new_tab_page::mojom::CustomizeChromeSection::kAppearance);

EXPECT_TRUE(visible);
EXPECT_EQ(CustomizeChromeSection::kAppearance, section);
}

TEST_F(NewTabPageHandlerTest, SetCustomizeChromeSidePanelInvisible) {
bool visible;
CustomizeChromeSection section;
EXPECT_CALL(*mock_customize_chrome_tab_helper_,
SetCustomizeChromeSidePanelVisible)
.Times(1)
.WillOnce(testing::DoAll(testing::SaveArg<0>(&visible),
testing::SaveArg<1>(&section)));

handler_->SetCustomizeChromeSidePanelVisible(
/*visible=*/false, new_tab_page::mojom::CustomizeChromeSection::kModules);

EXPECT_FALSE(visible);
EXPECT_EQ(CustomizeChromeSection::kModules, section);
}

0 comments on commit 8e65980

Please sign in to comment.