Web Share Target (desktop): Fail gracefully if manifest URL is blank.

Previously it would DCHECK if any share target in the Preferences file
had a blank or invalid manifest URL, which could be caused if the share
target was previously installed using an app banner. Since this is
corrupted user data, we should recover, not DCHECK, in these cases.

Also adds a DCHECK to catch cases that would write this corrupted data
into the preferences. This doesn't yet fix the issue, but it does report
it to developers. The DCHECK only fires if the manifest has a
share_target and the --enable-experimental-web-platform-features flag is
enabled (so it won't be too annoying).

Bug: 762388
Change-Id: Ibd800dc529510d4755068f7bb0c55f8ca083bc7e
Reviewed-on: https://chromium-review.googlesource.com/652011
Commit-Queue: Matt Giuca <mgiuca@chromium.org>
Reviewed-by: Sam McNally <sammc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504325}

chrome/browser/webshare/share_service_impl.cc

@@ -140,14 +140,16 @@ ShareServiceImpl::GetTargetsWithSufficientEngagement() {
 
   PrefService* pref_service = GetPrefService();
 
-  std::unique_ptr<base::DictionaryValue> share_targets_dict =
-      pref_service->GetDictionary(prefs::kWebShareVisitedTargets)
-          ->CreateDeepCopy();
+  const base::DictionaryValue* share_targets_dict =
+      pref_service->GetDictionary(prefs::kWebShareVisitedTargets);
 
   std::vector<WebShareTarget> sufficiently_engaged_targets;
   for (const auto& it : *share_targets_dict) {
     GURL manifest_url(it.first);
-    DCHECK(manifest_url.is_valid());
+    // This should not happen, but if the prefs file is corrupted, it might, so
+    // don't (D)CHECK, just continue gracefully.
+    if (!manifest_url.is_valid())
+      continue;
 
     if (GetEngagementLevel(manifest_url) < kMinimumEngagementLevel)
       continue;

chrome/browser/webshare/share_service_impl_unittest.cc

@@ -186,6 +186,9 @@ TEST_F(ShareServiceImplUnittest, ShareCallbackParams) {
                                                 kUrlTemplate);
   share_service_helper()->AddShareTargetToPrefs(kManifestUrlHigh, kTargetName,
                                                 kUrlTemplate);
+  // Expect this invalid URL to be ignored (not crash);
+  // https://crbug.com/762388.
+  share_service_helper()->AddShareTargetToPrefs("", kTargetName, kUrlTemplate);
 
   base::Callback<void(blink::mojom::ShareError)> callback =
       base::Bind(&DidShare, blink::mojom::ShareError::OK);

chrome/browser/webshare/share_target_pref_helper.cc

@@ -24,6 +24,11 @@ void UpdateShareTargetInPrefs(const GURL& manifest_url,
     return;
   }
 
+  // TODO(mgiuca): This DCHECK is known to fail due to https://crbug.com/762388.
+  // Currently, this can only happen if flags are turned on. These cases should
+  // be fixed before this feature is rolled out.
+  DCHECK(manifest_url.is_valid());
+
   std::string url_template =
       base::UTF16ToUTF8(manifest.share_target.value().url_template.string());