Finch experiments on SSL, malware, and phishing interstitials

chrome/browser/safe_browsing/safe_browsing_blocking_page.cc

Index: chrome/browser/safe_browsing/safe_browsing_blocking_page.cc
diff --git a/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc b/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc
index 032d5eceab5296580fbec821803a52e81f08e190..cda4a293a4846ce49f3e2a0e4cc8f7723807a3af 100644
--- a/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc
@@ -113,6 +113,22 @@ static const char* const kExpandedSeeMore = "expandedSeeMore";
 // RecordUserReactionTime.
 static const char* const kNavigatedAwayMetaCommand = "closed";
 
+// These are the conditions for the summer 2013 Finch experiment.
+static const std::string kCond1MalwareControl = "Cond1MalwareControl";

[agl, 2013/05/01 18:54:12]

static const char kCond1Foo[] = "...";

avoid static initialisers!

[felt, 2013/05/01 19:39:45]

Done.

+static const std::string kCond2MalwareNoBrand = "Cond2MalwareNoBrand";
+static const std::string kCond3PhishingControl = "Cond3PhishingControl";
+static const std::string kCond4PhishingNoBrand = "Cond4PhishingNoBrand";
+static const std::string kCond5MalwareOneStep = "Cond5MalwareOneStep";
+static const std::string kCond6PhishingOneStep = "Cond6PhishingOneStep";
+static const std::string kCond7MalwareFearMsg = "Cond7MalwareFearMsg";
+static const std::string kCond8PhishingFearMsg = "Cond8PhishingFearMsg";
+static const std::string kCond9MalwareCollabMsg = "Cond9MalwareCollabMsg";
+static const std::string kCond10PhishingCollabMsg = "Cond10PhishingCollabMsg";
+static const std::string kCond11MalwareQuestion = "Cond11MalwareQuestion";
+static const std::string kCond12PhishingQuestion = "Cond12PhishingQuestion";
+static const std::string kCond13MalwareGoBack = "Cond13MalwareGoBack";
+static const std::string kCond14PhishingGoBack = "Cond14PhishingGoBack";
+
 // static
 SafeBrowsingBlockingPageFactory* SafeBrowsingBlockingPage::factory_ = NULL;
 
@@ -129,7 +145,7 @@ class SafeBrowsingBlockingPageFactoryImpl
       WebContents* web_contents,
       const SafeBrowsingBlockingPage::UnsafeResourceList& unsafe_resources)
       OVERRIDE {
-    // Only do the trial if the interstitial is for a single malware or
+    // Only use the V2 page if the interstitial is for a single malware or
     // phishing resource, the multi-threat interstitial has not been updated to
     // V2 yet.
     if (unsafe_resources.size() == 1 &&
@@ -378,6 +394,8 @@ void SafeBrowsingBlockingPage::CommandReceived(const std::string& cmd) {
   }
 
   if (command == kExpandedSeeMore) {
+    if (!has_expanded_see_more_section_)  // Toggled in RecordUserReactionTime.
+      RecordUserAction(ADVANCED);
     // User expanded the "see more info" section of the page.  We don't actually
     // do any action based on this, it's just so that RecordUserReactionTime can
     // track it.
@@ -483,6 +501,9 @@ void SafeBrowsingBlockingPage::RecordUserAction(BlockingPageEvent event) {
     PHISHING_DONT_PROCEED,
     PHISHING_FORCED_DONT_PROCEED,
     PHISHING_PROCEED,
+    MALWARE_ADVANCED,

[agl, 2013/05/01 18:54:12]

likewise, I'm not sure that these enum elements have the same style as the
others.

[felt, 2013/05/01 19:39:45]

Done.

+    MULTIPLE_ADVANCED,
+    PHISHING_ADVANCED,
     MAX_ACTION
   } histogram_action = MAX_ACTION;
 
@@ -540,6 +561,19 @@ void SafeBrowsingBlockingPage::RecordUserAction(BlockingPageEvent event) {
         }
       }
       break;
+    case ADVANCED:
+      switch (interstitial_type_) {
+        case TYPE_MALWARE_AND_PHISHING:
+          histogram_action = MULTIPLE_ADVANCED;
+          break;
+        case TYPE_MALWARE:
+          histogram_action = MALWARE_ADVANCED;
+          break;
+        case TYPE_PHISHING:
+          histogram_action = PHISHING_ADVANCED;
+          break;
+      }
+      break;
     default:
       NOTREACHED() << "Unexpected event: " << event;
   }
@@ -891,6 +925,13 @@ SafeBrowsingBlockingPageV2::SafeBrowsingBlockingPageV2(
     WebContents* web_contents,
     const UnsafeResourceList& unsafe_resources)
   : SafeBrowsingBlockingPage(ui_manager, web_contents, unsafe_resources) {
+    trial_ = "";

[agl, 2013/05/01 18:54:12]

std::strings do not need to be explicitly cleared.

[felt, 2013/05/01 19:39:45]

Done.

+    if (unsafe_resources_[0].threat_type == SB_THREAT_TYPE_URL_MALWARE)

[agl, 2013/05/01 18:54:12]

I feel that once a conditional grows past an if and a single line body then
braces are needed.

[felt, 2013/05/01 19:39:45]

Done.

+      trial_ = base::FieldTrialList::FindFullName("InterstitialMalware");
+    else if (unsafe_resources_[0].threat_type == SB_THREAT_TYPE_URL_PHISHING ||
+             unsafe_resources_[0].threat_type ==
+                 SB_THREAT_TYPE_CLIENT_SIDE_PHISHING_URL)
+      trial_ = base::FieldTrialList::FindFullName("InterstitialPhishing");
 }
 
 std::string SafeBrowsingBlockingPageV2::GetHTMLContents() {
@@ -947,6 +988,7 @@ void SafeBrowsingBlockingPageV2::PopulateStringDictionary(
       IDS_SAFE_BROWSING_MALWARE_V2_SEE_MORE));
   strings->SetString("proceed",
       l10n_util::GetStringUTF16(IDS_SAFE_BROWSING_MALWARE_V2_PROCEED_LINK));
+  strings->SetString("trialType", trial_);  // Finch trial
 
   webui::SetFontAndTextDirection(strings);
 }