Fix broken mixed script Rappor metric and add browser test

chrome/browser/content_settings/tab_specific_content_settings.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/content_settings/tab_specific_content_settings.h"
 
 #include <list>
 
 #include "base/command_line.h"
 #include "base/lazy_instance.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
+#include "chrome/browser/browser_process.h"
 #include "chrome/browser/browsing_data/browsing_data_appcache_helper.h"
 #include "chrome/browser/browsing_data/browsing_data_cookie_helper.h"
 #include "chrome/browser/browsing_data/browsing_data_database_helper.h"
 #include "chrome/browser/browsing_data/browsing_data_file_system_helper.h"
 #include "chrome/browser/browsing_data/browsing_data_indexed_db_helper.h"
 #include "chrome/browser/browsing_data/browsing_data_local_storage_helper.h"
 #include "chrome/browser/browsing_data/cookies_tree_model.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/content_settings/chrome_content_settings_utils.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/media/media_capture_devices_dispatcher.h"
 #include "chrome/browser/media/media_stream_capture_indicator.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/render_messages.h"
 #include "components/content_settings/content/common/content_settings_messages.h"
 #include "components/content_settings/core/browser/content_settings_details.h"
 #include "components/content_settings/core/browser/content_settings_info.h"
 #include "components/content_settings/core/browser/content_settings_registry.h"
 #include "components/content_settings/core/browser/content_settings_utils.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "components/prefs/pref_service.h"
+#include "components/rappor/rappor_service.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/navigation_details.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/notification_registrar.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_delegate.h"
@@ skipping 49 matching lines @@
           CONTENT_SETTINGS_TYPE_GEOLOCATION),
       midi_usages_state_(
           HostContentSettingsMapFactory::GetForProfile(
               Profile::FromBrowserContext(tab->GetBrowserContext())),
           CONTENT_SETTINGS_TYPE_MIDI_SYSEX),
       pending_protocol_handler_(ProtocolHandler::EmptyProtocolHandler()),
       previous_protocol_handler_(ProtocolHandler::EmptyProtocolHandler()),
       pending_protocol_handler_setting_(CONTENT_SETTING_DEFAULT),
       load_plugins_link_enabled_(true),
       microphone_camera_state_(MICROPHONE_CAMERA_NOT_ACCESSED),
-      observer_(this) {
+      observer_(this),
+      rappor_service_(g_browser_process->rappor_service()) {
   ClearBlockedContentSettingsExceptForCookies();
   ClearCookieSpecificContentSettings();
 
   observer_.Add(HostContentSettingsMapFactory::GetForProfile(
       Profile::FromBrowserContext(tab->GetBrowserContext())));
 }
 
 TabSpecificContentSettings::~TabSpecificContentSettings() {
   FOR_EACH_OBSERVER(
       SiteDataObserver, observer_list_, ContentSettingsDestroyed());
@@ skipping 219 matching lines @@
     status.blocked = true;
     // TODO: it would be nice to have a way of mocking this in tests.
     content::NotificationService::current()->Notify(
         chrome::NOTIFICATION_WEB_CONTENT_SETTINGS_CHANGED,
         content::Source<WebContents>(web_contents()),
         content::NotificationService::NoDetails());
 
     if (type == CONTENT_SETTINGS_TYPE_MIXEDSCRIPT) {
       content_settings::RecordMixedScriptAction(
           content_settings::MIXED_SCRIPT_ACTION_DISPLAYED_SHIELD);
+      GURL url(details);
+      DCHECK(url.is_valid());
       content_settings::RecordMixedScriptActionWithRAPPOR(
-          content_settings::MIXED_SCRIPT_ACTION_DISPLAYED_SHIELD,
-          GURL(base::UTF16ToUTF8(details)));
+          rappor_service_,
+          content_settings::MIXED_SCRIPT_ACTION_DISPLAYED_SHIELD, url);
     }
   }
 }
 
 void TabSpecificContentSettings::OnContentAllowed(ContentSettingsType type) {
   DCHECK(type != CONTENT_SETTINGS_TYPE_GEOLOCATION)
       << "Geolocation settings handled by OnGeolocationPermissionSet";
   DCHECK(type != CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC &&
          type != CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA)
       << "Media stream settings handled by OnMediaStreamPermissionSet";
@@ skipping 472 matching lines @@
 void TabSpecificContentSettings::MidiDidNavigate(
     const content::LoadCommittedDetails& details) {
   midi_usages_state_.DidNavigate(GetCommittedDetails(details));
 }
 
 void TabSpecificContentSettings::BlockAllContentForTesting() {
   content_settings::ContentSettingsRegistry* registry =
       content_settings::ContentSettingsRegistry::GetInstance();
   for (const content_settings::ContentSettingsInfo* info : *registry) {
     ContentSettingsType type = info->website_settings_info()->type();
-    if (type != CONTENT_SETTINGS_TYPE_GEOLOCATION &&
-        type != CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC &&
-        type != CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA) {
+    if (type == CONTENT_SETTINGS_TYPE_MIXEDSCRIPT) {
+      // For mixed script, OnContentBlockedWithDetail() expects a valid
+      // URL in the |details| argument.
+      OnContentBlockedWithDetail(CONTENT_SETTINGS_TYPE_MIXEDSCRIPT,

[estark, 2016/06/12 17:12:16]

note: an alternative could be to make OnContentBlockedWithDetail() accept an
empty |details| for mixed script, but enforce that if non-empty it must be a
valid GURL. That would not, however, catch a hypothetical bug in which the
renderer sends an empty URL.

+                                 base::ASCIIToUTF16("http://example.test"));
+    } else if (type != CONTENT_SETTINGS_TYPE_GEOLOCATION &&
+               type != CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC &&
+               type != CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA) {
       OnContentBlocked(type);
     }
   }
 
   // Geolocation and media must be blocked separately, as the generic
   // TabSpecificContentSettings::OnContentBlocked does not apply to them.
   OnGeolocationPermissionSet(web_contents()->GetLastCommittedURL(), false);
   MicrophoneCameraStateFlags media_blocked =
       static_cast<MicrophoneCameraStateFlags>(
           TabSpecificContentSettings::MICROPHONE_ACCESSED |
           TabSpecificContentSettings::MICROPHONE_BLOCKED |
           TabSpecificContentSettings::CAMERA_ACCESSED |
           TabSpecificContentSettings::CAMERA_BLOCKED);
   OnMediaStreamPermissionSet(
       web_contents()->GetLastCommittedURL(),
       media_blocked,
       std::string(), std::string(), std::string(), std::string());
 }
+
+void TabSpecificContentSettings::SetRapporServiceForTesting(
+    rappor::RapporService* rappor_service) {
+  rappor_service_ = rappor_service;
+}