Skip to content

Commit

Permalink
Add a checkbox to the malware interstitial page, for user to opt-in t…
Browse files Browse the repository at this point in the history
…o send malware report. The user's choice will be stored as preference in user's profile.

Landing original change by kewang: http://codereview.chromium.org/5102001/.

BUG=60831
TEST=Relevant unit_tests,browser_tests

Review URL: http://codereview.chromium.org/6066011

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@70755 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
panayiotis@google.com committed Jan 7, 2011
1 parent 39900b4 commit 674731c
Show file tree
Hide file tree
Showing 6 changed files with 119 additions and 13 deletions.
5 changes: 3 additions & 2 deletions chrome/app/generated_resources.grd
Original file line number Diff line number Diff line change
Expand Up @@ -7774,8 +7774,9 @@ Keep your key file in a safe place. You will need it to create new versions of y
<message name="IDS_SAFE_BROWSING_MALWARE_DIAGNOSTIC_PAGE" desc="SafeBrowsing Malware diagnostic page">
Safe Browsing diagnostic page
</message>


<message name="IDS_SAFE_BROWSING_MALWARE_REPORTING_AGREE" desc="SafeBrowsing Malware Details label next to checkbox">
Help improve detection of malware for all users by sending additional data to Google about sites on which you see this warning.
</message>
<message name="IDS_SAFE_BROWSING_PHISHING_TITLE" desc="SafeBrowsing Phishing HTML title">
Phishing Detected!
</message>
Expand Down
39 changes: 36 additions & 3 deletions chrome/browser/resources/safe_browsing_malware_block.html
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,18 @@
.main {
margin:0px 90px 10px;
}
.footer {
margin-top: 40px;
padding-top: 10px;
border-top: 1px solid #ddd;
}
.submission {
margin:15px 5px 15px 0px;
padding:0px;
}
input {
margin:0px;
}
.proceedbutton {
}
.helpbutton {
float:right;
}
Expand All @@ -69,7 +72,6 @@
margin-right:5px;
}


.green {
background: -webkit-linear-gradient(#83c260, #71b44c 44%, #549d2c);
border: 1px solid #4c7336;
Expand Down Expand Up @@ -107,13 +109,36 @@
.green:focus {
border: 1px solid #000;
}

label.checkbox {
position: relative;
}

label.checkbox > input {
margin-top: 3px;
position: absolute;
}

label.checkbox > span {
-webkit-margin-start: 20px;
display: block;
}
</style>

<script>
function sendCommand(cmd) {
window.domAutomationController.setAutomationId(1);
window.domAutomationController.send(cmd);
}

function savePreference() {
var checkBox = document.getElementById('checkreport');
if (checkBox.checked) {
sendCommand('doReport');
} else {
sendCommand('dontReport');
}
}
</script>

</head>
Expand All @@ -137,6 +162,14 @@

<div class="main" i18n-values=".innerHTML:description3"></div>

<div class="main footer" jsdisplay="displaycheckbox">
<label class="checkbox" for="checkreport">
<input name="checked" id="checkreport" type="checkbox"
jsvalues=".checked:boxchecked" onclick="savePreference()">
<span i18n-content="confirm_text"></span>
</label>
</div>

</div>
</td>
</table>
Expand Down
41 changes: 41 additions & 0 deletions chrome/browser/safe_browsing/safe_browsing_blocking_page.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "base/string_number_conversions.h"
#include "base/utf_string_conversions.h"
#include "base/values.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/browser_thread.h"
#include "chrome/browser/dom_operation_notification_details.h"
#include "chrome/browser/dom_ui/new_tab_ui.h"
Expand Down Expand Up @@ -72,6 +73,10 @@ static const char* const kReportErrorCommand = "reportError";
static const char* const kLearnMoreCommand = "learnMore";
static const char* const kProceedCommand = "proceed";
static const char* const kTakeMeBackCommand = "takeMeBack";
static const char* const kDoReportCommand = "doReport";
static const char* const kDontReportCommand = "dontReport";
static const char* const kDisplayCheckBox = "displaycheckbox";
static const char* const kBoxChecked = "boxchecked";

// static
SafeBrowsingBlockingPageFactory* SafeBrowsingBlockingPage::factory_ = NULL;
Expand Down Expand Up @@ -316,6 +321,27 @@ void SafeBrowsingBlockingPage::PopulateMalwareStringDictionary(
strings->SetString("proceed_link",
l10n_util::GetStringUTF16(IDS_SAFE_BROWSING_MALWARE_PROCEED_LINK));
strings->SetString("textdirection", base::i18n::IsRTL() ? "rtl" : "ltr");

if (!CanShowMalwareDetailsOption()) {
strings->SetBoolean(kDisplayCheckBox, false);
} else {
// Show the checkbox for sending malware details.
strings->SetBoolean(kDisplayCheckBox, true);
strings->SetString(
"confirm_text",
l10n_util::GetStringUTF16(IDS_SAFE_BROWSING_MALWARE_REPORTING_AGREE));

const PrefService::Preference* pref =
tab()->profile()->GetPrefs()->FindPreference(
prefs::kSafeBrowsingReportingEnabled);

bool value;
if (pref && pref->GetValue()->GetAsBoolean(&value) && value) {
strings->SetString(kBoxChecked, "yes");
} else {
strings->SetString(kBoxChecked, "");
}
}
}

void SafeBrowsingBlockingPage::PopulatePhishingStringDictionary(
Expand Down Expand Up @@ -349,6 +375,16 @@ void SafeBrowsingBlockingPage::CommandReceived(const std::string& cmd) {
command = command.substr(1, command.length() - 2);
}

if (command == kDoReportCommand) {
SetReportingPreference(true);
return;
}

if (command == kDontReportCommand) {
SetReportingPreference(false);
return;
}

if (command == kLearnMoreCommand) {
// User pressed "Learn more".
GURL url;
Expand Down Expand Up @@ -424,6 +460,11 @@ void SafeBrowsingBlockingPage::CommandReceived(const std::string& cmd) {
NOTREACHED() << "Unexpected command: " << command;
}

void SafeBrowsingBlockingPage::SetReportingPreference(bool report) {
PrefService* pref = tab()->profile()->GetPrefs();
pref->SetBoolean(prefs::kSafeBrowsingReportingEnabled, report);
}

void SafeBrowsingBlockingPage::Proceed() {
RecordUserAction(PROCEED);
FinishMalwareDetails(); // Send the malware details, if we opted to.
Expand Down
3 changes: 2 additions & 1 deletion chrome/browser/safe_browsing/safe_browsing_blocking_page.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ class SafeBrowsingBlockingPage : public InterstitialPage {

// InterstitialPage method:
virtual std::string GetHTMLContents();
virtual void SetReportingPreference(bool report);
virtual void Proceed();
virtual void DontProceed();

Expand Down Expand Up @@ -106,7 +107,7 @@ class SafeBrowsingBlockingPage : public InterstitialPage {
// SBInterstitial[Phishing|Malware|Multiple][Show|Proceed|DontProceed].
void RecordUserAction(BlockingPageEvent event);

// See if we should even show the malware details option. For example, we
// Checks if we should even show the malware details option. For example, we
// don't show it in incognito mode.
bool CanShowMalwareDetailsOption();

Expand Down
12 changes: 5 additions & 7 deletions chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -297,19 +297,17 @@ IN_PROC_BROWSER_TEST_F(SafeBrowsingBlockingPageTest,

IN_PROC_BROWSER_TEST_F(SafeBrowsingBlockingPageTest,
MalwareIframeReportDetails) {
// Enable reporting of malware details.
browser()->GetProfile()->GetPrefs()->SetBoolean(
prefs::kSafeBrowsingReportingEnabled, true);
EXPECT_TRUE(browser()->GetProfile()->GetPrefs()->GetBoolean(
prefs::kSafeBrowsingReportingEnabled));

GURL url = test_server()->GetURL(kMalwarePage);
GURL iframe_url = test_server()->GetURL(kMalwareIframe);
AddURLResult(iframe_url, SafeBrowsingService::URL_MALWARE);

ui_test_utils::NavigateToURL(browser(), url);

SendCommand("\"proceed\""); // Simulate the user clicking "back"
SendCommand("\"doReport\""); // Simulate the user checking the checkbox.
EXPECT_TRUE(browser()->GetProfile()->GetPrefs()->GetBoolean(
prefs::kSafeBrowsingReportingEnabled));

SendCommand("\"proceed\""); // Simulate the user clicking "back"
AssertNoInterstitial(); // Assert the interstitial is gone

EXPECT_EQ(url, browser()->GetSelectedTabContents()->GetURL());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -548,3 +548,35 @@ TEST_F(SafeBrowsingBlockingPageTest, MalwareReportsDisabled) {
EXPECT_EQ(0u, service_->GetDetails()->size());
service_->GetDetails()->clear();
}

// Test setting the malware report preferance
TEST_F(SafeBrowsingBlockingPageTest, MalwareReports) {
// Disable malware reports.
contents()->profile()->GetPrefs()->SetBoolean(
prefs::kSafeBrowsingReportingEnabled, false);

// Start a load.
controller().LoadURL(GURL(kBadURL), GURL(), PageTransition::TYPED);

// Simulate the load causing a safe browsing interstitial to be shown.
ShowInterstitial(ResourceType::MAIN_FRAME, kBadURL);
SafeBrowsingBlockingPage* sb_interstitial = GetSafeBrowsingBlockingPage();
ASSERT_TRUE(sb_interstitial);

MessageLoop::current()->RunAllPending();

EXPECT_FALSE(contents()->profile()->GetPrefs()->GetBoolean(
prefs::kSafeBrowsingReportingEnabled));

// Simulate the user check the report agreement checkbox.
sb_interstitial->SetReportingPreference(true);

EXPECT_TRUE(contents()->profile()->GetPrefs()->GetBoolean(
prefs::kSafeBrowsingReportingEnabled));

// Simulate the user uncheck the report agreement checkbox.
sb_interstitial->SetReportingPreference(false);

EXPECT_FALSE(contents()->profile()->GetPrefs()->GetBoolean(
prefs::kSafeBrowsingReportingEnabled));
}

0 comments on commit 674731c

Please sign in to comment.