Media Galleries API: Fix MediaGalleriesPreferences initialization for fresh profiles.

Media Galleries API: Fix MediaGalleriesPreferences initialization for fresh profiles.

BUG=151701, 295880
R=vandebo@chromium.org

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

[tommycli, 2013-10-01 17:07:48]

vandebo: Small change to separate profile freshness determination step from
actual writing of default media galleries.

This would actually benefit from Scheduler, as you could put the
AddDefaultGalleriesIfFreshProfile step in a stage between the StorageMonitor and
the finders.

[tommycli, 2013-10-01 17:52:49]

vandebo: removed field. added method.

[vandebo (ex-Chrome), 2013-10-01 22:19:01]

LGTM

https://codereview.chromium.org/25539003/diff/15001/chrome/browser/media_gall...
File chrome/browser/media_galleries/media_galleries_preferences.cc (right):

https://codereview.chromium.org/25539003/diff/15001/chrome/browser/media_gall...
chrome/browser/media_galleries/media_galleries_preferences.cc:385:
weak_factory_.GetWeakPtr(), need_to_add_default_galleries));
nit: inline call to APIHasBeenUsed()

https://codereview.chromium.org/25539003/diff/15001/chrome/browser/media_gall...
File chrome/browser/media_galleries/media_galleries_preferences.h (right):

https://codereview.chromium.org/25539003/diff/15001/chrome/browser/media_gall...
chrome/browser/media_galleries/media_galleries_preferences.h:257: void
OnStorageMonitorInit(bool need_to_add_default_galleries);
nit: add_default_galleries

[commit-bot: I haz the power, 2013-10-01 22:28:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tommycli@chromium.org/25539003/23001

[tommycli, 2013-10-01 23:20:37]

https://codereview.chromium.org/25539003/diff/15001/chrome/browser/media_gall...
File chrome/browser/media_galleries/media_galleries_preferences.cc (right):

https://codereview.chromium.org/25539003/diff/15001/chrome/browser/media_gall...
chrome/browser/media_galleries/media_galleries_preferences.cc:385:
weak_factory_.GetWeakPtr(), need_to_add_default_galleries));
On 2013/10/01 22:19:01, vandebo wrote:
> nit: inline call to APIHasBeenUsed()

Done.

https://codereview.chromium.org/25539003/diff/15001/chrome/browser/media_gall...
File chrome/browser/media_galleries/media_galleries_preferences.h (right):

https://codereview.chromium.org/25539003/diff/15001/chrome/browser/media_gall...
chrome/browser/media_galleries/media_galleries_preferences.h:257: void
OnStorageMonitorInit(bool need_to_add_default_galleries);
On 2013/10/01 22:19:01, vandebo wrote:
> nit: add_default_galleries

Done.

[commit-bot: I haz the power, 2013-10-01 23:28:39]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win_x64_rel.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_x64_re...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-10-01 23:31:05]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tommycli@chromium.org/25539003/23001

[commit-bot: I haz the power, 2013-10-02 00:10:37]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win_x64_rel.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_x64_re...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-10-02 00:18:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tommycli@chromium.org/25539003/23001

[commit-bot: I haz the power, 2013-10-02 00:48:59]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win_x64_rel.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_x64_re...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-10-02 03:07:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tommycli@chromium.org/25539003/23001

[commit-bot: I haz the power, 2013-10-02 03:56:36]

Retried try job too often on linux_rel for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2013-10-02 16:37:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tommycli@chromium.org/25539003/23001

[commit-bot: I haz the power, 2013-10-02 17:23:25]

Retried try job too often on linux_rel for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[tommycli, 2013-10-02 18:33:06]

Committed patchset #5 manually as r226509 (presubmit successful).

[Sergey Ulanov, 2013-10-02 19:06:58]

On 2013/10/02 18:33:06, tommycli wrote:
> Committed patchset #5 manually as r226509 (presubmit successful).

Reverted because this change broke unit_tests. Please pay attention to test
results before committing manually. commit-bot rejected that change for a
reason.

[tommycli, 2013-10-02 19:16:46]

On 2013/10/02 19:06:58, Sergey Ulanov wrote:
> On 2013/10/02 18:33:06, tommycli wrote:
> > Committed patchset #5 manually as r226509 (presubmit successful).
> 
> Reverted because this change broke unit_tests. Please pay attention to test
> results before committing manually. commit-bot rejected that change for a
> reason.

Sorry about that. I thought CQ was failing due to http://crbug.com/297972.

[Lei Zhang, 2013-10-08 01:18:34]

The second try has this issue as well:

https://codereview.chromium.org/25539003/diff/23001/chrome/browser/media_gall...
File chrome/browser/media_galleries/media_galleries_preferences.cc (right):

https://codereview.chromium.org/25539003/diff/23001/chrome/browser/media_gall...
chrome/browser/media_galleries/media_galleries_preferences.cc:518: bool
need_to_add_default_galleries) {
nit: the parameter name here does not match the .h file and the comment on line
385.