Issue 148093011: Abstract media galleries context menu code and use it in scan result dialog.

Issue 148093011: Abstract media galleries context menu code and use it in scan result dialog. (Closed)

Created:
6 years, 10 months ago by vandebo (ex-Chrome)

Modified:
6 years, 10 months ago

Reviewers:
Lei Zhang, groby-ooo-7-16, sky

CC:
chromium-reviews, extensions-reviews_chromium.org, Lei Zhang, tfarina, tommycli, Greg Billock, chromium-apps-reviews_chromium.org

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

More Reviews

Description

Abstract media galleries context menu code and use it in scan result dialog. BUG=161119 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=247861

Patch Set 6 : nit #

Created: 6 years, 10 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+169 lines, -56 lines)			Patch
M	chrome/browser/media_galleries/media_galleries_dialog_controller.h	View		3 chunks	+3 lines, -4 lines	0 comments	Download
M	chrome/browser/media_galleries/media_galleries_dialog_controller.cc	View		4 chunks	+9 lines, -43 lines	0 comments	Download
M	chrome/browser/media_galleries/media_galleries_scan_result_dialog_controller.h	View		3 chunks	+9 lines, -0 lines	0 comments	Download
M	chrome/browser/media_galleries/media_galleries_scan_result_dialog_controller.cc	View		3 chunks	+12 lines, -0 lines	0 comments	Download
A	chrome/browser/media_galleries/media_gallery_context_menu.h	View		1 chunk	+41 lines, -0 lines	0 comments	Download
A	chrome/browser/media_galleries/media_gallery_context_menu.cc	View	1 2 3 4	1 chunk	+39 lines, -0 lines	0 comments	Download
M	chrome/browser/ui/cocoa/extensions/media_galleries_dialog_cocoa.h	View	1 2 3 4	1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/ui/cocoa/extensions/media_galleries_dialog_cocoa.mm	View	1 2	2 chunks	+3 lines, -3 lines	0 comments	Download
M	chrome/browser/ui/views/extensions/media_galleries_dialog_views.h	View		1 chunk	+0 lines, -4 lines	0 comments	Download
M	chrome/browser/ui/views/extensions/media_galleries_dialog_views.cc	View		1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/ui/views/extensions/media_galleries_scan_result_dialog_views.h	View		6 chunks	+14 lines, -0 lines	0 comments	Download
M	chrome/browser/ui/views/extensions/media_galleries_scan_result_dialog_views.cc	View	1 2 3 4 5	5 chunks	+35 lines, -0 lines	0 comments	Download
M	chrome/chrome_browser.gypi	View	1 2 3 4	1 chunk	+2 lines, -0 lines	0 comments	Download

Messages

Total messages: 11 (0 generated)

Expand Messages | Collapse Messages

vandebo (ex-Chrome)

Let me know if I should add views or cocoa owners.

6 years, 10 months ago (2014-01-28 19:43:39 UTC) #1

Lei Zhang

generally lgtm Deferring to the views/cocoa reviewers for the platform specific code. https://codereview.chromium.org/148093011/diff/60001/chrome/browser/media_galleries/media_gallery_context_menu.cc File chrome/browser/media_galleries/media_gallery_context_menu.cc ...

6 years, 10 months ago (2014-01-29 00:26:24 UTC) #2

vandebo (ex-Chrome)

On 2014/01/29 00:26:24, Lei Zhang wrote: > generally lgtm > > Deferring to the views/cocoa ...

6 years, 10 months ago (2014-01-29 03:29:36 UTC) #3

groby-ooo-7-16

Cocoa LGTM w/ nit https://codereview.chromium.org/148093011/diff/60001/chrome/browser/media_galleries/media_galleries_dialog_controller.h File chrome/browser/media_galleries/media_galleries_dialog_controller.h (right): https://codereview.chromium.org/148093011/diff/60001/chrome/browser/media_galleries/media_galleries_dialog_controller.h#newcode101 chrome/browser/media_galleries/media_galleries_dialog_controller.h:101: virtual void DidForgetGallery(MediaGalleryPrefId pref_id); Out ...

6 years, 10 months ago (2014-01-29 05:01:36 UTC) #4

vandebo (ex-Chrome)

On 2014/01/29 17:00:19, sky wrote: > What files do you need me to review here? ...

6 years, 10 months ago (2014-01-29 17:04:19 UTC) #6

vandebo (ex-Chrome)

https://codereview.chromium.org/148093011/diff/60001/chrome/browser/media_galleries/media_galleries_dialog_controller.h File chrome/browser/media_galleries/media_galleries_dialog_controller.h (right): https://codereview.chromium.org/148093011/diff/60001/chrome/browser/media_galleries/media_galleries_dialog_controller.h#newcode101 chrome/browser/media_galleries/media_galleries_dialog_controller.h:101: virtual void DidForgetGallery(MediaGalleryPrefId pref_id); On 2014/01/29 05:01:37, groby wrote: ...

6 years, 10 months ago (2014-01-29 17:16:54 UTC) #7

sky

LGTM https://codereview.chromium.org/148093011/diff/80001/chrome/browser/ui/views/extensions/media_galleries_scan_result_dialog_views.cc File chrome/browser/ui/views/extensions/media_galleries_scan_result_dialog_views.cc (right): https://codereview.chromium.org/148093011/diff/80001/chrome/browser/ui/views/extensions/media_galleries_scan_result_dialog_views.cc#newcode317 chrome/browser/ui/views/extensions/media_galleries_scan_result_dialog_views.cc:317: } NOTREACHED()?

6 years, 10 months ago (2014-01-29 17:32:59 UTC) #8

vandebo (ex-Chrome)

Thanks all https://codereview.chromium.org/148093011/diff/80001/chrome/browser/ui/views/extensions/media_galleries_scan_result_dialog_views.cc File chrome/browser/ui/views/extensions/media_galleries_scan_result_dialog_views.cc (right): https://codereview.chromium.org/148093011/diff/80001/chrome/browser/ui/views/extensions/media_galleries_scan_result_dialog_views.cc#newcode317 chrome/browser/ui/views/extensions/media_galleries_scan_result_dialog_views.cc:317: } On 2014/01/29 17:33:00, sky wrote: > ...

6 years, 10 months ago (2014-01-29 17:42:38 UTC) #9

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/vandebo@chromium.org/148093011/80002

6 years, 10 months ago (2014-01-29 17:43:35 UTC) #10

Message was sent while issue was closed.

Change committed as 247861

Expand Messages | Collapse Messages