Fix Rappor stats reporting for MixedContent.

Fix Rappor stats for MixedContent.

The problem was GURL(string url) need take a complete URL (with scheme and host ), otherwise it will return an empty GURL. 

BUG=536975

[Jialiu Lin, 2015-11-06 23:15:59]

Description was changed from

==========
Fix rappor

BUG=536975
==========

to

==========
Fix Rappor stats for MixedContent.

The problem was GURL(string url) need take a complete URL (with scheme and host
), otherwise it will return an empty GURL. 

BUG=536975
==========

[Jialiu Lin, 2015-11-06 23:20:53]

jialiul@chromium.org changed reviewers:
+ lgarron@chromium.org, thakis@chromium.org

[Jialiu Lin, 2015-11-06 23:20:53]

lgarron@ for bug review
thakis@ for OWNER review.

[lgarron, 2015-11-06 23:25:08]

https://codereview.chromium.org/1424933014/diff/1/chrome/renderer/content_set...
File chrome/renderer/content_settings_observer.cc (right):

https://codereview.chromium.org/1424933014/diff/1/chrome/renderer/content_set...
chrome/renderer/content_settings_observer.cc:589:
DidBlockContentType(CONTENT_SETTINGS_TYPE_MIXEDSCRIPT, origin.toString());
This seems like a good semantic fix.

Were you able to to tell that there are no other places that depend on this
being the host?

LGTM otherwise.

[Jialiu Lin, 2015-11-07 00:17:26]

Patchset #2 (id:20001) has been deleted

[Jialiu Lin, 2015-11-07 00:23:43]

Thanks, lgarron!

https://codereview.chromium.org/1424933014/diff/1/chrome/renderer/content_set...
File chrome/renderer/content_settings_observer.cc (right):

https://codereview.chromium.org/1424933014/diff/1/chrome/renderer/content_set...
chrome/renderer/content_settings_observer.cc:589:
DidBlockContentType(CONTENT_SETTINGS_TYPE_MIXEDSCRIPT, origin.toString());
On 2015/11/06 23:25:08, lgarron wrote:
> This seems like a good semantic fix.
> 
> Were you able to to tell that there are no other places that depend on this
> being the host?
> 
> LGTM otherwise.

I traced this details field, it basically can contains 2 types of information,
plugin name or host/url of a mixed content page.  The later one only used for
report RAPPOR stats. 

So nothing depends on details being the host

[Jialiu Lin, 2015-11-09 23:24:08]

ping Nico for OWNER review. Thanks!

[Nico, 2015-11-09 23:28:38]

is it possible to test this?

[Jialiu Lin, 2015-11-09 23:31:41]

Tested manually. 
Since Rappor stats are sampled probabilistically, it is not easy to test using 
unit tests or browser tests.

[Nico, 2015-11-09 23:32:43]

On 2015/11/09 23:31:41, JialiuLin wrote:
> Tested manually. 
> Since Rappor stats are sampled probabilistically, it is not easy to test using

> unit tests or browser tests.

I think the usual recommendation for this is to make the source of randomness
overridable and set it to the needed value in the testing code

[lgarron, 2015-11-09 23:36:09]

On 2015/11/09 at 23:32:43, thakis wrote:
> On 2015/11/09 23:31:41, JialiuLin wrote:
> > Tested manually. 
> > Since Rappor stats are sampled probabilistically, it is not easy to test
using 
> > unit tests or browser tests.
> 
> I think the usual recommendation for this is to make the source of randomness
overridable and set it to the needed value in the testing code

There is content_settings::RecordMixedScriptActionWithRAPPOR().

I was hoping to fix this along with a test (since that would have caught this
regression), but I just asked Jialiu if she could fix this by branch since I
didn't have time.

[Nico, 2015-11-09 23:38:49]

probably better to fix with a test and then merge the fix to the branch rather
than rush something in for the branch without test coverage?

[lgarron, 2015-11-09 23:44:02]

Yeah, totes. That wasn't meant to be a justification, just an explanation.

Jialiu: Do you think you'd have time to add a test?
If not, I could probably look at it next week.

[Jialiu Lin, 2015-11-09 23:47:36]

On 2015/11/09 23:44:02, lgarron wrote:
> Yeah, totes. That wasn't meant to be a justification, just an explanation.
> 
> Jialiu: Do you think you'd have time to add a test?
> If not, I could probably look at it next week.

Sorry, I am on Chrome Security Sheriffing rotation this week... I will try to
make time next week.

[lgarron, 2015-11-17 00:27:01]

On 2015/11/09 at 23:47:36, jialiul wrote:
> On 2015/11/09 23:44:02, lgarron wrote:
> > Yeah, totes. That wasn't meant to be a justification, just an explanation.
> > 
> > Jialiu: Do you think you'd have time to add a test?
> > If not, I could probably look at it next week.
> 
> Sorry, I am on Chrome Security Sheriffing rotation this week... I will try to
make time next week.

Correction, In #11 I meant GetRecordedSampleForMetric():

https://code.google.com/p/chromium/codesearch#chromium/src/components/rappor/...

[lgarron, 2016-03-17 20:12:23]

thakis@: Despite Jialiu spending tome trying, and us pinging the RAPPOR team a
few times about how to write browser tests for RAPPOR, we still don't really
know how, and the metric has been completely broken in the meantime. Would you
be okay with landing this?

[Nico, 2016-03-18 22:33:19]

(we were able to get some help from rappor folks off-thread)