Put all scan results in the media galleries scan result dialog.

chrome/browser/media_galleries/media_galleries_preferences.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/media_galleries/media_galleries_preferences.h"
 
 #include "base/base_paths_posix.h"
 #include "base/callback.h"
 #include "base/i18n/time_formatting.h"
 #include "base/path_service.h"
@@ skipping 234 matching lines @@
     dict->SetString(kMediaGalleriesVolumeLabelKey, gallery.volume_label);
     dict->SetString(kMediaGalleriesVendorNameKey, gallery.vendor_name);
     dict->SetString(kMediaGalleriesModelNameKey, gallery.model_name);
     dict->SetDouble(kMediaGalleriesSizeKey, gallery.total_size_in_bytes);
     dict->SetDouble(kMediaGalleriesLastAttachTimeKey,
                     gallery.last_attach_time.ToInternalValue());
   } else {
     dict->SetString(kMediaGalleriesDisplayNameKey, gallery.display_name);
   }
 
-  dict->SetInteger(kMediaGalleriesScanAudioCountKey, gallery.audio_count);
-  dict->SetInteger(kMediaGalleriesScanImageCountKey, gallery.image_count);
-  dict->SetInteger(kMediaGalleriesScanVideoCountKey, gallery.video_count);
+  if (gallery.IsBlackListedType() || gallery.audio_count ||

[tommycli, 2014/02/10 18:45:53]

Should this be all blacklisted  galleries, or only kRemovedScan galleries?

[Lei Zhang, 2014/02/10 19:55:22]

Can you add a comment to explain why we are checking this? The last 3/4s of this
if statement is equivalent to !IsEmptyScanResult() if you change the 3 fields in
MediaGalleryPrefInfo to a struct MediaGalleryScanResult.

[vandebo (ex-Chrome), 2014/02/11 19:19:31]

Blacklisted galleries in general shouldn't have their file counts stored.

[Lei Zhang, 2014/02/13 22:08:04]

But the code says:

if (gallery.IsBlackListedType() || ...) {
  dict->SetiInteger(...)
}

Isn't that a contradiction? What am I missing? Shouldn't this be:

if (!gallery.IsBlackLsitedType() && (...)) ?

[vandebo (ex-Chrome), 2014/02/13 22:10:02]

This is at a lower level - when we mark a gallery as blacklisted, this lets us
clear the file count out of preferences.

[Lei Zhang, 2014/02/14 07:34:02]

So you are trying to zero them out if the type is blacklisted?

CreateGalleryPrefInfoDictionary() only has 1 caller, which is
MediaGalleriesPreferences::AddGalleryInternal(). At the very top, it has a
DCHECK() that essentially says gallery.IsBlackListedType() cannot ever be true
here.

[vandebo (ex-Chrome), 2014/02/14 19:11:54]

Indeed you are correct - EraseOrBlacklistGalleryById() takes care of zeroing out
the file counts.

+      gallery.image_count || gallery.video_count) {
+    dict->SetInteger(kMediaGalleriesScanAudioCountKey, gallery.audio_count);
+    dict->SetInteger(kMediaGalleriesScanImageCountKey, gallery.image_count);
+    dict->SetInteger(kMediaGalleriesScanVideoCountKey, gallery.video_count);
+  }
 
   // Version 0 of the prefs format was that the display_name was always
   // used to show the user-visible name of the gallery. Version 1 means
   // that there is an optional display_name, and when it is present, it
   // overrides the name that would be built from the volume metadata, path,
   // or whatever other data. So if we see a display_name with version 0, it
   // means it may be overwritten simply by getting new volume metadata.
   // A display_name with version 1 should not be overwritten.
   dict->SetInteger(kMediaGalleriesPrefsVersionKey, gallery.prefs_version);
 
@@ skipping 959 matching lines @@
   if (extension_prefs_for_testing_)
     return extension_prefs_for_testing_;
   return extensions::ExtensionPrefs::Get(profile_);
 }
 
 void MediaGalleriesPreferences::SetExtensionPrefsForTesting(
     extensions::ExtensionPrefs* extension_prefs) {
   DCHECK(IsInitialized());
   extension_prefs_for_testing_ = extension_prefs;
 }