Componentize SecurityStateModel

Componentize SecurityStateModel

This CL componentizes SecurityStateModel so that some of its logic can
eventually be used on iOS.

SecurityStateModel has a delegate interface to provide information that
it needs to compute the security info for a page or
request. ChromeSecurityStateModelDelegate, in chrome/browser/ssl, is
associated with a WebContents and uses that WebContents to implement the
delegate interface. ChromeSecurityStateModelDelegate also exposes the
SecurityStateModel's main method, GetSecurityInfo(), so that WebContents
consumers can access the security info for a WebContents.

BUG=515071

[estark, 2015-11-14 02:30:56]

Patchset #1 (id:1) has been deleted

[estark, 2015-11-14 02:31:54]

Patchset #1 (id:20001) has been deleted

[estark, 2015-11-14 02:33:35]

Patchset #1 (id:40001) has been deleted

[estark, 2015-11-14 02:33:53]

Patchset #1 (id:60001) has been deleted

[estark, 2015-11-14 02:34:05]

Patchset #1 (id:80001) has been deleted

[estark, 2015-11-20 01:05:16]

Description was changed from

==========
Componentize SecurityStateModel

BUG=
==========

to

==========
Componentize SecurityStateModel

This CL componentizes SecurityStateModel so that some of its logic can
eventually be used on iOS.

SecurityStateModel has a delegate interface to provide information that
it needs to compute the security info for a page or
request. ChromeSecurityStateModelDelegate, in chrome/browser/ssl, is
associated with a WebContents and uses that WebContents to implement the
delegate interface. ChromeSecurityStateModelDelegate also exposes the
SecurityStateModel's main method, GetSecurityInfo(), so that WebContents
consumers can access the security info for a WebContents.

BUG=515071
==========

[estark, 2015-11-20 01:07:55]

estark@chromium.org changed reviewers:
+ blundell@chromium.org

[estark, 2015-11-20 01:07:56]

blundell, could you please take a preliminary look at this?

Here are some additional caveats:

- There's a weird thing where the SecurityStateModel unit tests are failing on
linux_chromium_rel_ng but nowhere else. Still looking into this.

- This CL leaves a SecurityStateModel "shell" in chrome/browser/ssl, which
redefines an enum and typedefs a struct from the corresponding component. This
is to avoid updating a whole bunch of users of these types, which I'll do in a
follow-up CL. (Otherwise this CL would touch a gazillion files and I feel
terrible for anyone who would have to review it.)

Thanks!

[blundell, 2015-11-20 10:58:53]

Hi Emily,

I scanned this CL, and at a high level, it looks great. It's quite large and
doing several things; I'd prefer to see it split up into several CLs to be able
to really drill down into the details effectively and make sure we're not
introducing any regressions during the refactoring. Here's the split that I
would suggest:

(1) Introduction of security_state_model_delegate.h with one or two methods and
chrome_security_state_model_delegate.* with implementations of those methods. In
this CL SecurityStateModel can simply have a
scoped_ptr<SecurityStateModelDelegate> that it internally populates with a
ChromeSecurityStateModelDelegate instance.

(2) Change of the ownership structure so that ChromeSecurityStateModelDelegate
is the WebContentsUserData and SecurityStateModel is owned by it instead of
owning it, eliminating the knowledge of SecurityStateModel about
ChromeSecurityStateModelDelegate.

(3) CL (or CLs, depending on how large an amount of work it is) moving all of
the problematic dependencies out of SecurityStateModel into
ChromeSecurityStateModelDelegate via the SecurityStateModelDelegate interface.

(4) Componentization of SecurityStateModel, which should be trivial once you've
gotten to this point.

How does that sound?

[estark, 2015-11-20 22:01:34]

On 2015/11/20 10:58:53, blundell wrote:
> Hi Emily,
> 
> I scanned this CL, and at a high level, it looks great. It's quite large and
> doing several things; I'd prefer to see it split up into several CLs to be
able
> to really drill down into the details effectively and make sure we're not
> introducing any regressions during the refactoring. Here's the split that I
> would suggest:
> 
> (1) Introduction of security_state_model_delegate.h with one or two methods
and
> chrome_security_state_model_delegate.* with implementations of those methods.
In
> this CL SecurityStateModel can simply have a
> scoped_ptr<SecurityStateModelDelegate> that it internally populates with a
> ChromeSecurityStateModelDelegate instance.
> 
> (2) Change of the ownership structure so that ChromeSecurityStateModelDelegate
> is the WebContentsUserData and SecurityStateModel is owned by it instead of
> owning it, eliminating the knowledge of SecurityStateModel about
> ChromeSecurityStateModelDelegate.
> 
> (3) CL (or CLs, depending on how large an amount of work it is) moving all of
> the problematic dependencies out of SecurityStateModel into
> ChromeSecurityStateModelDelegate via the SecurityStateModelDelegate interface.
> 
> (4) Componentization of SecurityStateModel, which should be trivial once
you've
> gotten to this point.
> 
> How does that sound?

That sounds totally reasonable, thanks!