Add the SocEng as a type for checking in CheckUrlForSubresourceFilter.

components/safe_browsing_db/v4_local_database_manager.cc

Index: components/safe_browsing_db/v4_local_database_manager.cc
diff --git a/components/safe_browsing_db/v4_local_database_manager.cc b/components/safe_browsing_db/v4_local_database_manager.cc
index 1916d0b830fcb2df32ad4cf2fb5f54d6954c0557..ab59a87ea3e8fbb1ec2ec314bb80260a9956993a 100644
--- a/components/safe_browsing_db/v4_local_database_manager.cc
+++ b/components/safe_browsing_db/v4_local_database_manager.cc
@@ -74,7 +74,7 @@ ListInfos GetListInfos() {
                GetChromeUrlClientIncidentId(),
                SB_THREAT_TYPE_BLACKLISTED_RESOURCE),
       ListInfo(kSyncNever, "", GetChromeUrlApiId(), SB_THREAT_TYPE_API_ABUSE),
-      ListInfo(kSyncOnlyOnChromeBuilds, "UrlSubresourceFilter.store",
+      ListInfo(kSyncAlways, "UrlSubresourceFilter.store",

[melandory, 2017/04/19 14:47:11]

+nparker@

Before start debugging I want to check if I'm hitting something known.

The main change in this CL -- CheckUrlForSubresourceFilter will check not only
against GetUrlSubresourceFilterId(), but also against GetUrlSocEngId(). These
two lists have different fetch_updates strategies, SocEng is kSyncAlways and
SubresourceFilter is kSyncOnlyOnChromeBuilds.

The symptom I observe is for SubresourceFilterBrowserTest:
1.fetch_update strategy of SB_THREAT_TYPE_SUBRESOURCE_FILTER is
kSyncOnlyOnChromeBuilds.
2. Configure the url to be a SocEng via test wrappers from
chrome/browser/safe_browsing/v4_test_utils.cc (MarkPrefixAsBad from
safe_browsing::TestV4DatabaseFactory and AddToFullHashCache from
safe_browsing::TestV4GetHashProtocolManagerFactory)
3. Navigate to this URL.

The call to CheckUrlForSubresourceFilter identifies the URL as
SB_THREAT_TYPE_SAFE.

But if I change the fetch_update strategy as in this patch (to kSyncAlways)
everything works.

I'll start debugging if the symptoms doesn't ring a bell. Thanks!

                GetUrlSubresourceFilterId(), SB_THREAT_TYPE_SUBRESOURCE_FILTER),
   });
   // NOTE(vakh): IMPORTANT: Please make sure that the server already supports
@@ -276,7 +276,8 @@ bool V4LocalDatabaseManager::CheckUrlForSubresourceFilter(const GURL& url,
                                                           Client* client) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
-  StoresToCheck stores_to_check({GetUrlSubresourceFilterId()});
+  StoresToCheck stores_to_check(
+      {GetUrlSocEngId(), GetUrlSubresourceFilterId()});

[engedy, 2017/04/20 11:16:10]

Just to double check: does this cover both SOCIAL_ENG_ADS_INTERSTITIAL as well
as PHISHING_INTERSTITIAL?

[melandory, 2017/04/25 13:48:13]

Yes

   if (!AreStoresAvailableNow(stores_to_check) || !CanCheckUrl(url)) {
     return true;
   }