Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(375)

Issue 23456035: Media Galleries API Picasa: Windows Custom Database Locations (Closed)

Created:
7 years, 3 months ago by tommycli
Modified:
7 years, 2 months ago
Reviewers:
Greg Billock
CC:
chromium-reviews, extensions-reviews_chromium.org, vandebo (ex-Chrome), Lei Zhang, tzik+watch_chromium.org, Greg Billock, chromium-apps-reviews_chromium.org, kinuko+watch
Visibility:
Public.

Description

Media Galleries API Picasa: Windows Custom Database Locations This code enables Picasa to find custom Windows database locations as defined in the registry. It also provides for overridden database locations for testing. Depends on https://codereview.chromium.org/23513059/. BUG=151701 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=227656

Patch Set 1 #

Patch Set 2 : reformat and stuff #

Patch Set 3 : merge #

Patch Set 4 : #

Total comments: 6

Patch Set 5 : #

Total comments: 10

Patch Set 6 : #

Patch Set 7 : #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+132 lines, -34 lines) Patch
M chrome/browser/extensions/api/media_galleries/media_galleries_apitest.cc View 1 2 3 4 5 6 chunks +37 lines, -19 lines 2 comments Download
M chrome/browser/media_galleries/fileapi/picasa_finder.h View 1 2 3 1 chunk +5 lines, -0 lines 0 comments Download
M chrome/browser/media_galleries/fileapi/picasa_finder.cc View 1 2 3 4 5 1 chunk +59 lines, -14 lines 2 comments Download
M chrome/browser/media_galleries/media_galleries_test_util.h View 1 2 3 4 5 6 3 chunks +11 lines, -0 lines 0 comments Download
M chrome/browser/media_galleries/media_galleries_test_util.cc View 1 2 3 4 5 4 chunks +20 lines, -1 line 0 comments Download

Messages

Total messages: 16 (0 generated)
tommycli
gbillock: Need review.
7 years, 3 months ago (2013-09-17 18:49:34 UTC) #1
tommycli
ready for review
7 years, 2 months ago (2013-10-01 19:11:48 UTC) #2
Greg Billock
https://codereview.chromium.org/23456035/diff/9001/chrome/browser/media_galleries/fileapi/picasa_finder.cc File chrome/browser/media_galleries/fileapi/picasa_finder.cc (right): https://codereview.chromium.org/23456035/diff/9001/chrome/browser/media_galleries/fileapi/picasa_finder.cc#newcode27 chrome/browser/media_galleries/fileapi/picasa_finder.cc:27: const wchar_t kPicasaRegistryPath[] = Can you just move this ...
7 years, 2 months ago (2013-10-01 20:50:35 UTC) #3
tommycli
https://codereview.chromium.org/23456035/diff/9001/chrome/browser/media_galleries/fileapi/picasa_finder.cc File chrome/browser/media_galleries/fileapi/picasa_finder.cc (right): https://codereview.chromium.org/23456035/diff/9001/chrome/browser/media_galleries/fileapi/picasa_finder.cc#newcode27 chrome/browser/media_galleries/fileapi/picasa_finder.cc:27: const wchar_t kPicasaRegistryPath[] = On 2013/10/01 20:50:35, Greg Billock ...
7 years, 2 months ago (2013-10-01 22:07:22 UTC) #4
vandebo (ex-Chrome)
drive by... https://codereview.chromium.org/23456035/diff/15001/chrome/browser/extensions/api/media_galleries/media_galleries_apitest.cc File chrome/browser/extensions/api/media_galleries/media_galleries_apitest.cc (right): https://codereview.chromium.org/23456035/diff/15001/chrome/browser/extensions/api/media_galleries/media_galleries_apitest.cc#newcode264 chrome/browser/extensions/api/media_galleries/media_galleries_apitest.cc:264: .AppendASCII("Google").AppendASCII("Picasa2"); nit: wrapping https://codereview.chromium.org/23456035/diff/15001/chrome/browser/extensions/api/media_galleries/media_galleries_apitest.cc#newcode289 chrome/browser/extensions/api/media_galleries/media_galleries_apitest.cc:289: .AppendASCII("Google").AppendASCII("Picasa3"); nit: ...
7 years, 2 months ago (2013-10-02 15:32:45 UTC) #5
tommycli
https://codereview.chromium.org/23456035/diff/15001/chrome/browser/extensions/api/media_galleries/media_galleries_apitest.cc File chrome/browser/extensions/api/media_galleries/media_galleries_apitest.cc (right): https://codereview.chromium.org/23456035/diff/15001/chrome/browser/extensions/api/media_galleries/media_galleries_apitest.cc#newcode264 chrome/browser/extensions/api/media_galleries/media_galleries_apitest.cc:264: .AppendASCII("Google").AppendASCII("Picasa2"); On 2013/10/02 15:32:46, vandebo wrote: > nit: wrapping ...
7 years, 2 months ago (2013-10-02 16:44:39 UTC) #6
tommycli
gbillock: need OWNER approval/review
7 years, 2 months ago (2013-10-03 20:36:06 UTC) #7
tommycli
On 2013/10/03 20:36:06, tommycli wrote: > gbillock: need OWNER approval/review gbillock: ping
7 years, 2 months ago (2013-10-04 22:14:17 UTC) #8
Greg Billock
https://codereview.chromium.org/23456035/diff/33001/chrome/browser/extensions/api/media_galleries/media_galleries_apitest.cc File chrome/browser/extensions/api/media_galleries/media_galleries_apitest.cc (right): https://codereview.chromium.org/23456035/diff/33001/chrome/browser/extensions/api/media_galleries/media_galleries_apitest.cc#newcode284 chrome/browser/extensions/api/media_galleries/media_galleries_apitest.cc:284: PopulatePicasaTestData(metadata_root); Should this still set the metadata_root as above ...
7 years, 2 months ago (2013-10-04 23:52:50 UTC) #9
tommycli
https://codereview.chromium.org/23456035/diff/33001/chrome/browser/extensions/api/media_galleries/media_galleries_apitest.cc File chrome/browser/extensions/api/media_galleries/media_galleries_apitest.cc (right): https://codereview.chromium.org/23456035/diff/33001/chrome/browser/extensions/api/media_galleries/media_galleries_apitest.cc#newcode284 chrome/browser/extensions/api/media_galleries/media_galleries_apitest.cc:284: PopulatePicasaTestData(metadata_root); On 2013/10/04 23:52:50, Greg Billock wrote: > Should ...
7 years, 2 months ago (2013-10-05 00:18:44 UTC) #10
tommycli
Hey Greg, I confirmed that this CL replicates the Windows Picasa behavior as is. It ...
7 years, 2 months ago (2013-10-07 21:48:56 UTC) #11
tommycli
On 2013/10/07 21:48:56, tommycli wrote: > Hey Greg, > > I confirmed that this CL ...
7 years, 2 months ago (2013-10-08 17:15:58 UTC) #12
Greg Billock
lgtm
7 years, 2 months ago (2013-10-08 19:22:37 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/tommycli@chromium.org/23456035/33001
7 years, 2 months ago (2013-10-08 19:30:16 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/tommycli@chromium.org/23456035/33001
7 years, 2 months ago (2013-10-09 02:05:59 UTC) #15
commit-bot: I haz the power
7 years, 2 months ago (2013-10-09 03:42:23 UTC) #16
Message was sent while issue was closed.
Change committed as 227656

Powered by Google App Engine
This is Rietveld 408576698