Issue 12276010: Factor out uses of the WebContentsModalDialog interface from platform-independent code

Issue 12276010: Factor out uses of the WebContentsModalDialog interface from platform-independent code (Closed)

Created:
7 years, 10 months ago by Mike Wittman

Modified:
7 years, 10 months ago

Reviewers:
sail, Ben Goodger (Google)

CC:
chromium-reviews, tfarina, sail+watch_chromium.org, dvh

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

Visibility:
Public.

More Reviews

Description

Factor out uses of the WebContentsModalDialog interface from platform-independent code This CL is part 2 of 6 in the work to remove the WebContentsModalDialog interface. It removes platform-independent uses of functions on the WebContentsModalDialog interface in the UI code and replaces them with the platform-specific equivalents. Patch set 1 removes CloseWebContentsModalDialog call from LoginHandler. This CL builds on part 1 in https://codereview.chromium.org/12281012. BUG=157161 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=183995

Patch Set 1 #

Patch Set 2 : remove CloseWebContentsModalDialog call from TabModalConfirmDialogDelegate #

Patch Set 3 : remove CloseWebContentsModalDialog call from ConstrainedWebDialogDelegateBase #

Total comments: 2

Patch Set 4 : address review comment #

Patch Set 5 : rebase + remove CloseWebContentsModalDialog call from ProfileSigninConfirmationDialog #

Patch Set 6 : fix android compile error #

Patch Set 7 : apply changes to TabModalConfirmDialogWebUI #

Patch Set 8 : fix PrintPreviewUI unit test failure #

Patch Set 9 : fix ConstrainedWebDialogBrowserTest.BasicTest on linux #

Patch Set 10 : fix ConstrainedWebDialogBrowserTest.BasicTest on mac #

Patch Set 11 : remove incorrect override #

Created: 7 years, 10 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+253 lines, -114 lines)			Patch
M	chrome/browser/ui/android/login_prompt_android.cc	View	1 2 3 4 5	1 chunk	+2 lines, -0 lines	0 comments	Download
M	chrome/browser/ui/cocoa/constrained_web_dialog_delegate_mac.mm	View	1 2 3 4 5 6 7 8 9	5 chunks	+44 lines, -14 lines	0 comments	Download
M	chrome/browser/ui/cocoa/login_prompt_cocoa.mm	View	1 2 3	1 chunk	+6 lines, -2 lines	0 comments	Download
M	chrome/browser/ui/cocoa/tab_modal_confirm_dialog_mac.h	View	1	1 chunk	+3 lines, -0 lines	0 comments	Download
M	chrome/browser/ui/cocoa/tab_modal_confirm_dialog_mac.mm	View	1	2 chunks	+5 lines, -1 line	0 comments	Download
M	chrome/browser/ui/gtk/constrained_web_dialog_delegate_gtk.cc	View	1 2 3 4 5 6 7 8 9 10	4 chunks	+53 lines, -18 lines	0 comments	Download
M	chrome/browser/ui/gtk/login_prompt_gtk.cc	View		4 chunks	+12 lines, -3 lines	0 comments	Download
M	chrome/browser/ui/gtk/tab_modal_confirm_dialog_gtk.h	View	1	3 chunks	+6 lines, -0 lines	0 comments	Download
M	chrome/browser/ui/gtk/tab_modal_confirm_dialog_gtk.cc	View	1	3 chunks	+8 lines, -2 lines	0 comments	Download
M	chrome/browser/ui/login/login_prompt.h	View		3 chunks	+3 lines, -7 lines	0 comments	Download
M	chrome/browser/ui/login/login_prompt.cc	View		4 chunks	+1 line, -9 lines	0 comments	Download
M	chrome/browser/ui/tab_modal_confirm_dialog.h	View	1	2 chunks	+6 lines, -2 lines	0 comments	Download
M	chrome/browser/ui/tab_modal_confirm_dialog_delegate.h	View	1	5 chunks	+18 lines, -5 lines	0 comments	Download
M	chrome/browser/ui/tab_modal_confirm_dialog_delegate.cc	View	1	4 chunks	+6 lines, -7 lines	0 comments	Download
M	chrome/browser/ui/views/constrained_web_dialog_delegate_views.cc	View	1 2 3 4 5 6 7 8	7 chunks	+23 lines, -11 lines	0 comments	Download
M	chrome/browser/ui/views/constrained_window_views_browsertest.cc	View	1 2 3 4 5 6 7 8	2 chunks	+3 lines, -1 line	0 comments	Download
M	chrome/browser/ui/views/login_prompt_views.cc	View		5 chunks	+13 lines, -4 lines	0 comments	Download
M	chrome/browser/ui/views/tab_modal_confirm_dialog_views.h	View	1	2 chunks	+7 lines, -0 lines	0 comments	Download
M	chrome/browser/ui/views/tab_modal_confirm_dialog_views.cc	View	1	2 chunks	+8 lines, -2 lines	0 comments	Download
M	chrome/browser/ui/webui/certificate_viewer_webui.cc	View	1 2 3 4 5 6 7 8	2 chunks	+4 lines, -2 lines	0 comments	Download
M	chrome/browser/ui/webui/constrained_web_dialog_delegate_base.h	View	1 2 3 4 5 6 7 8	3 chunks	+1 line, -6 lines	0 comments	Download
M	chrome/browser/ui/webui/constrained_web_dialog_delegate_base.cc	View	1 2 3 4 5 6 7 8	3 chunks	+5 lines, -9 lines	0 comments	Download
M	chrome/browser/ui/webui/constrained_web_dialog_ui.h	View	1 2 3 4 5 6 7 8	2 chunks	+5 lines, -5 lines	0 comments	Download
M	chrome/browser/ui/webui/constrained_web_dialog_ui_browsertest.cc	View	1 2 3 4 5 6 7 8	1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/ui/webui/signin/profile_signin_confirmation_dialog.cc	View	1 2 3 4	1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/ui/webui/tab_modal_confirm_dialog_webui.h	View	1 2 3 4 5 6	1 chunk	+3 lines, -0 lines	0 comments	Download
M	chrome/browser/ui/webui/tab_modal_confirm_dialog_webui.cc	View	1 2 3 4 5 6	3 chunks	+6 lines, -2 lines	0 comments	Download

Messages

Total messages: 10 (0 generated)

Expand Messages | Collapse Messages

Mike Wittman

Ben, please take a look at the non-Cocoa parts of this. Sailesh, please take a ...

7 years, 10 months ago (2013-02-16 00:45:31 UTC) #1

sail

cocoa/* LGTM https://codereview.chromium.org/12276010/diff/5001/chrome/browser/ui/cocoa/login_prompt_cocoa.mm File chrome/browser/ui/cocoa/login_prompt_cocoa.mm (right): https://codereview.chromium.org/12276010/diff/5001/chrome/browser/ui/cocoa/login_prompt_cocoa.mm#newcode86 chrome/browser/ui/cocoa/login_prompt_cocoa.mm:86: if (constrained_window_.get()) don't need .get()

7 years, 10 months ago (2013-02-16 21:45:22 UTC) #3

Mike Wittman

https://codereview.chromium.org/12276010/diff/5001/chrome/browser/ui/cocoa/login_prompt_cocoa.mm File chrome/browser/ui/cocoa/login_prompt_cocoa.mm (right): https://codereview.chromium.org/12276010/diff/5001/chrome/browser/ui/cocoa/login_prompt_cocoa.mm#newcode86 chrome/browser/ui/cocoa/login_prompt_cocoa.mm:86: if (constrained_window_.get()) On 2013/02/16 21:45:22, sail wrote: > don't ...

7 years, 10 months ago (2013-02-19 19:08:36 UTC) #4