ClientSideDetectionService model fetch should honor kSbDisableAutoUpdate switch.

ClientSideDetectionService model fetch should honor kSbDisableAutoUpdate switch.

BUG=232851
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=194728

[noelutz, 2013-04-17 22:36:45]

Thanks for taking care of this.  Just one question.

https://codereview.chromium.org/14225020/diff/1/chrome/browser/safe_browsing/...
File chrome/browser/safe_browsing/client_side_detection_service.cc (right):

https://codereview.chromium.org/14225020/diff/1/chrome/browser/safe_browsing/...
chrome/browser/safe_browsing/client_side_detection_service.cc:276: return;
There are two other places in this file where we issue HTTP requests.

The first one is if there is a client-side phishing hit.  This only happens if
there is a phishing model present so we should be good there given that we
always fetch it.

The second case happens if we show a malware interstitial.  Do we ever send hash
server requests if that flag is set?  Does it only disable update requests or
all SB requests?  If hash server requests are still sent then there might be a
slight chance of us sending a malware report even if that flag is set.  Is that
acceptable?

[mattm, 2013-04-17 23:12:17]

https://codereview.chromium.org/14225020/diff/1/chrome/browser/safe_browsing/...
File chrome/browser/safe_browsing/client_side_detection_service.cc (right):

https://codereview.chromium.org/14225020/diff/1/chrome/browser/safe_browsing/...
chrome/browser/safe_browsing/client_side_detection_service.cc:276: return;
On 2013/04/17 22:36:45, noelutz wrote:
> There are two other places in this file where we issue HTTP requests.
> 
> The first one is if there is a client-side phishing hit.  This only happens if
> there is a phishing model present so we should be good there given that we
> always fetch it.
> 
> The second case happens if we show a malware interstitial.  Do we ever send
hash
> server requests if that flag is set?  Does it only disable update requests or
> all SB requests?  If hash server requests are still sent then there might be a
> slight chance of us sending a malware report even if that flag is set.  Is
that
> acceptable?

I think you're thinking of the MalwareDetails class, the malware stuff in this
file is the new malware IP blacklisting stuff kewang is working on, which also
depends on the phishing model being present, if I'm remembering correctly.

[noelutz, 2013-04-17 23:36:48]

lgtm

https://codereview.chromium.org/14225020/diff/1/chrome/browser/safe_browsing/...
File chrome/browser/safe_browsing/client_side_detection_service.cc (right):

https://codereview.chromium.org/14225020/diff/1/chrome/browser/safe_browsing/...
chrome/browser/safe_browsing/client_side_detection_service.cc:276: return;
On 2013/04/17 23:12:17, mattm wrote:
> On 2013/04/17 22:36:45, noelutz wrote:
> > There are two other places in this file where we issue HTTP requests.
> > 
> > The first one is if there is a client-side phishing hit.  This only happens
if
> > there is a phishing model present so we should be good there given that we
> > always fetch it.
> > 
> > The second case happens if we show a malware interstitial.  Do we ever send
> hash
> > server requests if that flag is set?  Does it only disable update requests
or
> > all SB requests?  If hash server requests are still sent then there might be
a
> > slight chance of us sending a malware report even if that flag is set.  Is
> that
> > acceptable?
> 
> I think you're thinking of the MalwareDetails class, the malware stuff in this
> file is the new malware IP blacklisting stuff kewang is working on, which also
> depends on the phishing model being present, if I'm remembering correctly.

You're absolutely right.  Thanks for refreshing my memory.

[mattm, 2013-04-17 23:56:29]

Committed patchset #1 manually as r194728 (presubmit successful).