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

Issue 10695066: Move SelectFileDialog implementation to ui/base/dialogs/ (Closed)

Created:
8 years, 5 months ago by Elliot Glaysher
Modified:
8 years, 5 months ago
CC:
chromium-reviews
Visibility:
Public.

Description

Part 3 of Move SelectFileDialog implementation to ui/base/dialogs/ -Actually move the SelectFileDialog class to ui::, leaving a compatibility header in the interim. - Because of linking reasons, create a factory interface for the SelectFileDialogExtension to use to inject itself from chrome/browser/views/ui/ - wstring code moved to win only files. BUG=134529 TEST=none Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=146382

Patch Set 1 #

Patch Set 2 : try again #

Patch Set 3 : try using a factory model for the select file dialog extension #

Patch Set 4 : compile fixes #

Patch Set 5 : more compile fix #

Patch Set 6 : More fix #

Patch Set 7 : More fix #

Patch Set 8 : Reupload because weirdness #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+190 lines, -338 lines) Patch
M chrome/browser/file_select_helper.h View 1 2 3 4 5 6 7 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/ui/certificate_dialogs.h View 1 2 3 4 5 6 7 1 chunk +4 lines, -0 lines 0 comments Download
M chrome/browser/ui/select_file_dialog.h View 1 2 3 4 5 6 7 1 chunk +3 lines, -186 lines 0 comments Download
D chrome/browser/ui/select_file_dialog.cc View 1 chunk +0 lines, -88 lines 0 comments Download
M chrome/browser/ui/views/ash/chrome_browser_main_extra_parts_ash.cc View 1 2 3 2 chunks +9 lines, -0 lines 0 comments Download
D chrome/browser/ui/views/select_file_dialog_aura.cc View 1 2 1 chunk +0 lines, -22 lines 0 comments Download
M chrome/browser/ui/views/select_file_dialog_extension.cc View 1 2 3 4 5 6 7 2 chunks +2 lines, -0 lines 1 comment Download
A chrome/browser/ui/views/select_file_dialog_extension_factory.h View 1 2 3 4 5 6 7 1 chunk +25 lines, -0 lines 1 comment Download
A chrome/browser/ui/views/select_file_dialog_extension_factory.cc View 1 2 3 4 5 6 1 chunk +19 lines, -0 lines 0 comments Download
M chrome/browser/ui/views/select_file_dialog_win.cc View 1 chunk +4 lines, -3 lines 0 comments Download
M chrome/browser/ui/views/select_file_dialog_win_unittest.cc View 1 chunk +6 lines, -0 lines 0 comments Download
M chrome/browser/ui/webui/print_preview/print_preview_handler.h View 1 2 3 4 5 6 7 1 chunk +4 lines, -0 lines 0 comments Download
M chrome/chrome_browser.gypi View 1 2 3 4 5 6 7 5 chunks +6 lines, -4 lines 0 comments Download
A + ui/base/dialogs/select_file_dialog.h View 1 2 3 4 5 6 7 6 chunks +18 lines, -20 lines 2 comments Download
A + ui/base/dialogs/select_file_dialog.cc View 1 2 3 chunks +45 lines, -15 lines 0 comments Download
A ui/base/dialogs/select_file_dialog_factory.h View 1 2 3 4 5 6 7 1 chunk +29 lines, -0 lines 0 comments Download
A ui/base/dialogs/select_file_dialog_factory.cc View 1 2 1 chunk +11 lines, -0 lines 0 comments Download
M ui/ui.gyp View 1 2 3 4 5 6 7 1 chunk +4 lines, -0 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
Elliot Glaysher
8 years, 5 months ago (2012-07-11 21:57:20 UTC) #1
James Cook
Most files say "error: old chunk mismatch", maybe it needs a re-upload? The factory approach ...
8 years, 5 months ago (2012-07-11 22:15:56 UTC) #2
Elliot Glaysher
ptal. I don't know what happened with rietveld.
8 years, 5 months ago (2012-07-11 22:24:09 UTC) #3
James Cook
LGTM with two optional comments. I like the overall factory approach. http://codereview.chromium.org/10695066/diff/5006/chrome/browser/ui/views/select_file_dialog_extension.cc File chrome/browser/ui/views/select_file_dialog_extension.cc (right): ...
8 years, 5 months ago (2012-07-11 22:34:14 UTC) #4
Elliot Glaysher
http://codereview.chromium.org/10695066/diff/5006/ui/base/dialogs/select_file_dialog.h File ui/base/dialogs/select_file_dialog.h (right): http://codereview.chromium.org/10695066/diff/5006/ui/base/dialogs/select_file_dialog.h#newcode42 ui/base/dialogs/select_file_dialog.h:42: class UI_EXPORT Listener { On 2012/07/11 22:34:14, James Cook ...
8 years, 5 months ago (2012-07-11 22:40:16 UTC) #5
Ben Goodger (Google)
8 years, 5 months ago (2012-07-12 02:56:23 UTC) #6
lgtm

http://codereview.chromium.org/10695066/diff/5006/chrome/browser/ui/views/sel...
File chrome/browser/ui/views/select_file_dialog_extension_factory.h (right):

http://codereview.chromium.org/10695066/diff/5006/chrome/browser/ui/views/sel...
chrome/browser/ui/views/select_file_dialog_extension_factory.h:16: // Override
from ui::SelectFileDialogFactory:
private?

Powered by Google App Engine
This is Rietveld 408576698