Clean up the interface to DeserializeSecurityInfo, fix uninitialized memory use.

Clean up the interface to DeserializeSecurityInfo, fix uninitialized memory use.

BUG=507018
TEST=clusterfuzz is happy

Committed: https://crrev.com/e0d90188576ca7206a7a411e1bfe45c898440d7d
Cr-Commit-Position: refs/heads/master@{#337883}

[Avi (use Gerrit), 2015-07-08 15:16:22]

avi@chromium.org changed reviewers:
+ cpalmer@chromium.org, estark@chromium.org

[Avi (use Gerrit), 2015-07-08 15:17:21]

This is more of a "stem the bleeding" patch; Emily will be handling things for
reals in http://crbug.com/445234.

Can you both take a look? This is pretty simple.

[Ryan Sleevi, 2015-07-08 15:23:38]

rsleevi@chromium.org changed reviewers:
+ palmer@chromium.org
- cpalmer@chromium.org

[Ryan Sleevi, 2015-07-08 15:25:04]

rsleevi@chromium.org changed reviewers:
+ rsleevi@chromium.org

[Ryan Sleevi, 2015-07-08 15:25:04]

-cpalmer
+palmer

I'm kind of meh on the "stop the bleeding" - I'm not sure an uninitialized
memory use is the actual bleeding, so much as a symptom of it (which is trusting
junk data & not checking results), but I'll defer to palmer and emily and
refrain from being too pessimistic.

[Ryan Sleevi, 2015-07-08 15:25:34]

rsleevi@chromium.org changed reviewers:
- rsleevi@chromium.org

[Ryan Sleevi, 2015-07-08 15:25:34]

Gah, I unchecked the wrong box. Sending another email so it's on Emily & Chris's
radar

[Avi (use Gerrit), 2015-07-08 15:26:57]

On 2015/07/08 15:25:04, Ryan Sleevi (slow through 7-15 wrote:
> -cpalmer
> +palmer
> 
> I'm kind of meh on the "stop the bleeding" - I'm not sure an uninitialized
> memory use is the actual bleeding, so much as a symptom of it (which is
trusting
> junk data & not checking results), but I'll defer to palmer and emily and
> refrain from being too pessimistic.

It's slightly less bleeding; passing around defined values on a failure to
deserialize is IMO strictly better than passing around junk. :)

[Avi (use Gerrit), 2015-07-08 18:14:20]

On 2015/07/08 15:26:57, Avi wrote:
> On 2015/07/08 15:25:04, Ryan Sleevi (slow through 7-15 wrote:
> > -cpalmer
> > +palmer
> > 
> > I'm kind of meh on the "stop the bleeding" - I'm not sure an uninitialized
> > memory use is the actual bleeding, so much as a symptom of it (which is
> trusting
> > junk data & not checking results), but I'll defer to palmer and emily and
> > refrain from being too pessimistic.
> 
> It's slightly less bleeding; passing around defined values on a failure to
> deserialize is IMO strictly better than passing around junk. :)

Also, the problem is that I know *nothing* about this area, and got
ClusterFuzz's bug because I touched navigation, which is how this triggers.

I'm trying to make the minimum change to make ClusterFuzz happy without having
to take ownership of what looks to be a huge problem :O

[estark, 2015-07-08 18:17:43]

LGTM. I'm spinning out a separate bug for more thorough bleeding-stopping.

My only general question is if it's okay to return an SSLStatus by value. Can't
find a rule in the style guide, but it contains a vector so it seems like a
moderately big thing to pass around by value.

https://codereview.chromium.org/1225983003/diff/1/content/common/ssl_status_s...
File content/common/ssl_status_serialization.h (right):

https://codereview.chromium.org/1225983003/diff/1/content/common/ssl_status_s...
content/common/ssl_status_serialization.h:24: // Deerializes the given security
info. Note that this returns the SecurityStyle
Deserializes (missing first s)

[Avi (use Gerrit), 2015-07-08 18:30:00]

Emily already has a bug to fix this better; I don't know if she wants that to
cover all of the things we were talking about on the mailing list, or if she
wants another one.

Emily, thoughts on this CL?

https://codereview.chromium.org/1225983003/diff/1/content/common/ssl_status_s...
File content/common/ssl_status_serialization.h (right):

https://codereview.chromium.org/1225983003/diff/1/content/common/ssl_status_s...
content/common/ssl_status_serialization.h:24: // Deerializes the given security
info. Note that this returns the SecurityStyle
On 2015/07/08 18:17:42, estark wrote:
> Deserializes (missing first s)

Done.

[Avi (use Gerrit), 2015-07-08 18:30:33]

On 2015/07/08 18:30:00, Avi wrote:
> Emily, thoughts on this CL?

Gah, misread who replied.

[Avi (use Gerrit), 2015-07-08 18:31:45]

The CQ bit was checked by avi@chromium.org

[Avi (use Gerrit), 2015-07-08 18:31:46]

The patchset sent to the CQ was uploaded after l-g-t-m from estark@chromium.org
Link to the patchset: https://codereview.chromium.org/1225983003/#ps20001
(title: "typo")

[estark, 2015-07-08 18:31:51]

On 2015/07/08 18:30:33, Avi wrote:
> On 2015/07/08 18:30:00, Avi wrote:
> > Emily, thoughts on this CL?
> 
> Gah, misread who replied.

Haha, I was pretty confused for a second there.

The bug I filed for fixing this better is crbug.com/508232.

[commit-bot: I haz the power, 2015-07-08 18:33:28]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1225983003/20001

[commit-bot: I haz the power, 2015-07-08 19:34:06]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-07-08 19:36:26]

Patchset 2 (id:??) landed as
https://crrev.com/e0d90188576ca7206a7a411e1bfe45c898440d7d
Cr-Commit-Position: refs/heads/master@{#337883}