Commit 16e5be88 authored by Ryan Sturm's avatar Ryan Sturm Committed by Commit Bot

Capping Pages InfoBar buttons -> InfoBar Links

Per UX decision, this change makes the InfoBar better for users.

Instead of using an InfoBar Button for pausing/resuming subresource
loading, we will use an InfoBar clickable link.

Bug: 853885

Change-Id: Ia967baec922d0c363f48d26e057e121fa6ae969f
Reviewed-on: https://chromium-review.googlesource.com/1104722Reviewed-by: default avatarTarun Bansal <tbansal@chromium.org>
Commit-Queue: Ryan Sturm <ryansturm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568229}
parent 96bff731
......@@ -13,7 +13,6 @@
#include "base/test/scoped_feature_list.h"
#include "base/threading/thread_task_runner_handle.h"
#include "chrome/browser/data_use_measurement/page_load_capping/chrome_page_load_capping_features.h"
#include "chrome/browser/infobars/infobar_responder.h"
#include "chrome/browser/infobars/infobar_service.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/tabs/tab_strip_model.h"
......@@ -132,11 +131,15 @@ IN_PROC_BROWSER_TEST_F(PageLoadCappingBrowserTest, PageLoadCappingBlocksLoads) {
content::WebContents* contents =
browser()->tab_strip_model()->GetActiveWebContents();
InfoBarResponder infobar_responder(InfoBarService::FromWebContents(contents),
InfoBarResponder::ACCEPT);
// Load a mostly empty page.
ui_test_utils::NavigateToURL(
browser(), https_test_server_->GetURL("/page_capping.html"));
// Pause sub-resource loading.
InfoBarService::FromWebContents(contents)
->infobar_at(0)
->delegate()
->AsConfirmInfoBarDelegate()
->LinkClicked(WindowOpenDisposition::CURRENT_TAB);
// Adds images to the page. They should not be allowed to load.
// Running this 20 times makes 20 round trips to the renderer, making it very
......@@ -171,11 +174,15 @@ IN_PROC_BROWSER_TEST_F(PageLoadCappingBrowserTest,
content::WebContents* contents =
browser()->tab_strip_model()->GetActiveWebContents();
InfoBarResponder infobar_responder(InfoBarService::FromWebContents(contents),
InfoBarResponder::ACCEPT);
// Load a mostly empty page.
ui_test_utils::NavigateToURL(
browser(), https_test_server_->GetURL("/page_capping.html"));
// Pause sub-resource loading.
InfoBarService::FromWebContents(contents)
->infobar_at(0)
->delegate()
->AsConfirmInfoBarDelegate()
->LinkClicked(WindowOpenDisposition::CURRENT_TAB);
// Adds an image to the page. It should not be allowed to load at first.
// PageLoadCappingBlocksLoads tests that it is not loaded more robustly
......@@ -192,7 +199,7 @@ IN_PROC_BROWSER_TEST_F(PageLoadCappingBrowserTest,
->infobar_at(0)
->delegate()
->AsConfirmInfoBarDelegate()
->Accept();
->LinkClicked(WindowOpenDisposition::CURRENT_TAB);
// An image should be fetched because subresource loading was paused then
// resumed.
......@@ -212,8 +219,6 @@ IN_PROC_BROWSER_TEST_F(PageLoadCappingBrowserTest, PageLoadCappingAllowLoads) {
content::WebContents* contents =
browser()->tab_strip_model()->GetActiveWebContents();
InfoBarResponder infobar_responder(InfoBarService::FromWebContents(contents),
InfoBarResponder::DISMISS);
// Load a mostly empty page.
ui_test_utils::NavigateToURL(
browser(), https_test_server_->GetURL("/page_capping.html"));
......@@ -245,11 +250,15 @@ IN_PROC_BROWSER_TEST_F(PageLoadCappingBrowserTest,
content::WebContents* contents =
browser()->tab_strip_model()->GetActiveWebContents();
InfoBarResponder infobar_responder(InfoBarService::FromWebContents(contents),
InfoBarResponder::ACCEPT);
// Load a mostly empty page.
ui_test_utils::NavigateToURL(
browser(), https_test_server_->GetURL("/page_capping.html"));
// Pause sub-resource loading.
InfoBarService::FromWebContents(contents)
->infobar_at(0)
->delegate()
->AsConfirmInfoBarDelegate()
->LinkClicked(WindowOpenDisposition::CURRENT_TAB);
content::TestNavigationObserver load_observer(contents);
// Adds an image to the page. It should be allowed to load.
......@@ -298,11 +307,15 @@ IN_PROC_BROWSER_TEST_F(PageLoadCappingBrowserTest,
content::WebContents* contents =
browser()->tab_strip_model()->GetActiveWebContents();
InfoBarResponder infobar_responder(InfoBarService::FromWebContents(contents),
InfoBarResponder::ACCEPT);
// Load a mostly empty page.
ui_test_utils::NavigateToURL(
browser(), https_test_server_->GetURL("/page_capping.html"));
// Pause sub-resource loading.
InfoBarService::FromWebContents(contents)
->infobar_at(0)
->delegate()
->AsConfirmInfoBarDelegate()
->LinkClicked(WindowOpenDisposition::CURRENT_TAB);
content::TestNavigationObserver load_observer(contents);
// Adds an image to the page. It should be allowed to load.
......@@ -339,7 +352,7 @@ IN_PROC_BROWSER_TEST_F(PageLoadCappingBrowserTest,
->infobar_at(0)
->delegate()
->AsConfirmInfoBarDelegate()
->Accept();
->LinkClicked(WindowOpenDisposition::CURRENT_TAB);
// An image should be fetched because subresource loading was resumed.
if (images_attempted() < 1u)
......
......@@ -37,11 +37,10 @@ class ResumeDelegate : public PageLoadCappingInfoBarDelegate {
base::string16 GetMessageText() const override {
return l10n_util::GetStringUTF16(IDS_PAGE_CAPPING_STOPPED_TITLE);
}
base::string16 GetButtonLabel(InfoBarButton button) const override {
DCHECK_EQ(ConfirmInfoBarDelegate::BUTTON_OK, button);
base::string16 GetLinkText() const override {
return l10n_util::GetStringUTF16(IDS_PAGE_CAPPING_CONTINUE_MESSAGE);
}
bool Accept() override {
bool LinkClicked(WindowOpenDisposition disposition) override {
RecordInteractionUMA(InfoBarInteraction::kResumedPage);
if (pause_callback_.is_null())
return true;
......@@ -61,8 +60,8 @@ class ResumeDelegate : public PageLoadCappingInfoBarDelegate {
class PauseDelegate : public PageLoadCappingInfoBarDelegate {
public:
// This object is destroyed wqhen the page is terminated, and methods related
// to functionality of the infobar (E.g., Accept()), are not called from page
// destructors. This object is also destroyed on all non-same page
// to functionality of the infobar (E.g., LinkClicked()), are not called from
// page destructors. This object is also destroyed on all non-same page
// navigations.
// |pause_callback| is a callback that will pause subresource loading on the
// page.
......@@ -78,12 +77,11 @@ class PauseDelegate : public PageLoadCappingInfoBarDelegate {
static_cast<int>(bytes_threshold_ / 1024 / 1024));
}
base::string16 GetButtonLabel(InfoBarButton button) const override {
DCHECK_EQ(ConfirmInfoBarDelegate::BUTTON_OK, button);
base::string16 GetLinkText() const override {
return l10n_util::GetStringUTF16(IDS_PAGE_CAPPING_STOP_MESSAGE);
}
bool Accept() override {
bool LinkClicked(WindowOpenDisposition disposition) override {
RecordInteractionUMA(InfoBarInteraction::kPausedPage);
if (!pause_callback_.is_null()) {
// Pause subresouce loading on the page.
......@@ -149,5 +147,5 @@ bool PageLoadCappingInfoBarDelegate::ShouldExpire(
}
int PageLoadCappingInfoBarDelegate::GetButtons() const {
return ConfirmInfoBarDelegate::BUTTON_OK;
return BUTTON_NONE;
}
......@@ -64,8 +64,8 @@ class PageLoadCappingInfoBarDelegate : public ConfirmInfoBarDelegate {
int GetButtons() const override;
bool ShouldExpire(const NavigationDetails& details) const override;
base::string16 GetMessageText() const override = 0;
base::string16 GetButtonLabel(InfoBarButton button) const override = 0;
bool Accept() override = 0;
bool LinkClicked(WindowOpenDisposition disposition) override = 0;
base::string16 GetLinkText() const override = 0;
DISALLOW_COPY_AND_ASSIGN(PageLoadCappingInfoBarDelegate);
};
......
......@@ -76,9 +76,9 @@ TEST_F(PageLoadCappingInfoBarDelegateTest, ClickingCreatesNewInfobar) {
EXPECT_TRUE(delegate);
// Make sure this is pause delegate.
EXPECT_EQ(l10n_util::GetStringUTF16(IDS_PAGE_CAPPING_STOP_MESSAGE),
delegate->GetButtonLabel(ConfirmInfoBarDelegate::BUTTON_OK));
delegate->GetLinkText());
// |delegate| and |infobar| will be deleted by this call.
EXPECT_FALSE(delegate->Accept());
EXPECT_FALSE(delegate->LinkClicked(WindowOpenDisposition::CURRENT_TAB));
EXPECT_EQ(1u, InfoBarCount());
EXPECT_EQ(1u, pause_subresource_loading_count_);
......@@ -93,15 +93,15 @@ TEST_F(PageLoadCappingInfoBarDelegateTest, ClickingCreatesNewInfobar) {
stopped_delegate = infobar->delegate()->AsConfirmInfoBarDelegate();
// Make sure this is the resume delegate.
EXPECT_EQ(
l10n_util::GetStringUTF16(IDS_PAGE_CAPPING_CONTINUE_MESSAGE),
stopped_delegate->GetButtonLabel(ConfirmInfoBarDelegate::BUTTON_OK));
EXPECT_EQ(l10n_util::GetStringUTF16(IDS_PAGE_CAPPING_CONTINUE_MESSAGE),
stopped_delegate->GetLinkText());
EXPECT_TRUE(stopped_delegate);
// Make sure that they are different infobar instances.
EXPECT_NE(delegate, stopped_delegate);
// If this is true, the infobar will be closed by the infobar manager.
EXPECT_TRUE(stopped_delegate->Accept());
EXPECT_TRUE(
stopped_delegate->LinkClicked(WindowOpenDisposition::CURRENT_TAB));
EXPECT_EQ(2u, pause_subresource_loading_count_);
histogram_tester.ExpectBucketCount(
......
......@@ -379,7 +379,7 @@ TEST_F(PageCappingObserverTest, PageCappingTriggered) {
->GetPauseSubresourceLoadingCalled());
static_cast<ConfirmInfoBarDelegate*>(
infobar_service()->infobar_at(0u)->delegate())
->Accept();
->LinkClicked(WindowOpenDisposition::CURRENT_TAB);
EXPECT_TRUE(content::WebContentsTester::For(web_contents())
->GetPauseSubresourceLoadingCalled());
EXPECT_EQ(1u, InfoBarCount());
......@@ -392,7 +392,7 @@ TEST_F(PageCappingObserverTest, PageCappingTriggered) {
static_cast<ConfirmInfoBarDelegate*>(
infobar_service()->infobar_at(0u)->delegate())
->Accept();
->LinkClicked(WindowOpenDisposition::CURRENT_TAB);
EXPECT_FALSE(content::WebContentsTester::For(web_contents())
->GetPauseSubresourceLoadingCalled());
EXPECT_FALSE(observer_->IsPausedForTesting());
......@@ -432,7 +432,7 @@ TEST_F(PageCappingObserverTest, DataSavingsDefault) {
->GetPauseSubresourceLoadingCalled());
static_cast<ConfirmInfoBarDelegate*>(
infobar_service()->infobar_at(0u)->delegate())
->Accept();
->LinkClicked(WindowOpenDisposition::CURRENT_TAB);
EXPECT_TRUE(content::WebContentsTester::For(web_contents())
->GetPauseSubresourceLoadingCalled());
EXPECT_EQ(1u, InfoBarCount());
......@@ -456,7 +456,7 @@ TEST_F(PageCappingObserverTest, DataSavingsDefault) {
static_cast<ConfirmInfoBarDelegate*>(
infobar_service()->infobar_at(0u)->delegate())
->Accept();
->LinkClicked(WindowOpenDisposition::CURRENT_TAB);
EXPECT_FALSE(content::WebContentsTester::For(web_contents())
->GetPauseSubresourceLoadingCalled());
EXPECT_FALSE(observer_->IsPausedForTesting());
......@@ -502,7 +502,7 @@ TEST_F(PageCappingObserverTest, DataSavingsParam) {
->GetPauseSubresourceLoadingCalled());
static_cast<ConfirmInfoBarDelegate*>(
infobar_service()->infobar_at(0u)->delegate())
->Accept();
->LinkClicked(WindowOpenDisposition::CURRENT_TAB);
EXPECT_TRUE(content::WebContentsTester::For(web_contents())
->GetPauseSubresourceLoadingCalled());
EXPECT_EQ(1u, InfoBarCount());
......@@ -526,7 +526,7 @@ TEST_F(PageCappingObserverTest, DataSavingsParam) {
static_cast<ConfirmInfoBarDelegate*>(
infobar_service()->infobar_at(0u)->delegate())
->Accept();
->LinkClicked(WindowOpenDisposition::CURRENT_TAB);
EXPECT_FALSE(content::WebContentsTester::For(web_contents())
->GetPauseSubresourceLoadingCalled());
EXPECT_FALSE(observer_->IsPausedForTesting());
......@@ -570,7 +570,7 @@ TEST_F(PageCappingObserverTest, DataSavingsHistogram) {
static_cast<ConfirmInfoBarDelegate*>(
infobar_service()->infobar_at(0u)->delegate())
->Accept();
->LinkClicked(WindowOpenDisposition::CURRENT_TAB);
// Savings is only recorded in OnComplete
SimulateAppEnterBackground();
......@@ -611,11 +611,11 @@ TEST_F(PageCappingObserverTest, DataSavingsHistogramWhenResumed) {
static_cast<ConfirmInfoBarDelegate*>(
infobar_service()->infobar_at(0u)->delegate())
->Accept();
->LinkClicked(WindowOpenDisposition::CURRENT_TAB);
static_cast<ConfirmInfoBarDelegate*>(
infobar_service()->infobar_at(0u)->delegate())
->Accept();
->LinkClicked(WindowOpenDisposition::CURRENT_TAB);
NavigateToUntrackedUrl();
histogram_tester.ExpectTotalCount("HeavyPageCapping.RecordedDataSavings", 0);
......@@ -718,7 +718,7 @@ TEST_F(PageCappingObserverTest, UKMRecordedPaused) {
// Pause the page.
static_cast<ConfirmInfoBarDelegate*>(
infobar_service()->infobar_at(0u)->delegate())
->Accept();
->LinkClicked(WindowOpenDisposition::CURRENT_TAB);
NavigateToUntrackedUrl();
using UkmEntry = ukm::builders::PageLoadCapping;
......@@ -769,11 +769,11 @@ TEST_F(PageCappingObserverTest, UKMRecordedResumed) {
// Pause then resume.
static_cast<ConfirmInfoBarDelegate*>(
infobar_service()->infobar_at(0u)->delegate())
->Accept();
->LinkClicked(WindowOpenDisposition::CURRENT_TAB);
static_cast<ConfirmInfoBarDelegate*>(
infobar_service()->infobar_at(0u)->delegate())
->Accept();
->LinkClicked(WindowOpenDisposition::CURRENT_TAB);
NavigateToUntrackedUrl();
using UkmEntry = ukm::builders::PageLoadCapping;
......
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