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

Side by Side Diff: components/app_modal/javascript_app_modal_dialog.cc

Issue 2639893002: Clean up unneeded JavaScript dialog parameter. (Closed)
Patch Set: rebase Created 3 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
OLDNEW
1 // Copyright (c) 2012 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "components/app_modal/javascript_app_modal_dialog.h" 5 #include "components/app_modal/javascript_app_modal_dialog.h"
6 6
7 #include "base/metrics/histogram_macros.h" 7 #include "base/metrics/histogram_macros.h"
8 #include "base/time/time.h" 8 #include "base/time/time.h"
9 #include "build/build_config.h" 9 #include "build/build_config.h"
10 #include "components/app_modal/javascript_dialog_manager.h" 10 #include "components/app_modal/javascript_dialog_manager.h"
(...skipping 74 matching lines...) Expand 10 before | Expand all | Expand 10 after
85 NativeAppModalDialog* JavaScriptAppModalDialog::CreateNativeDialog() { 85 NativeAppModalDialog* JavaScriptAppModalDialog::CreateNativeDialog() {
86 return JavaScriptDialogManager::GetInstance() 86 return JavaScriptDialogManager::GetInstance()
87 ->native_dialog_factory() 87 ->native_dialog_factory()
88 ->CreateNativeJavaScriptDialog(this); 88 ->CreateNativeJavaScriptDialog(this);
89 } 89 }
90 90
91 bool JavaScriptAppModalDialog::IsJavaScriptModalDialog() { 91 bool JavaScriptAppModalDialog::IsJavaScriptModalDialog() {
92 return true; 92 return true;
93 } 93 }
94 94
95 void JavaScriptAppModalDialog::Invalidate(bool suppress_callbacks) { 95 void JavaScriptAppModalDialog::Invalidate() {
96 if (!IsValid()) 96 if (!IsValid())
97 return; 97 return;
98 98
99 AppModalDialog::Invalidate(suppress_callbacks); 99 AppModalDialog::Invalidate();
100 if (!suppress_callbacks) 100 CallDialogClosedCallback(false, base::string16());
101 CallDialogClosedCallback(false, base::string16());
102 if (native_dialog()) 101 if (native_dialog())
103 CloseModalDialog(); 102 CloseModalDialog();
104 } 103 }
105 104
106 void JavaScriptAppModalDialog::OnCancel(bool suppress_js_messages) { 105 void JavaScriptAppModalDialog::OnCancel(bool suppress_js_messages) {
107 // We need to do this before WM_DESTROY (WindowClosing()) as any parent frame 106 // We need to do this before WM_DESTROY (WindowClosing()) as any parent frame
108 // will receive its activation messages before this dialog receives 107 // will receive its activation messages before this dialog receives
109 // WM_DESTROY. The parent frame would then try to activate any modal dialogs 108 // WM_DESTROY. The parent frame would then try to activate any modal dialogs
110 // that were still open in the ModalDialogQueue, which would send activation 109 // that were still open in the ModalDialogQueue, which would send activation
111 // back to this one. The framework should be improved to handle this, so this 110 // back to this one. The framework should be improved to handle this, so this
(...skipping 36 matching lines...) Expand 10 before | Expand all | Expand 10 after
148 // data from the map owned by ::JavaScriptDialogManager. Make sure 147 // data from the map owned by ::JavaScriptDialogManager. Make sure
149 // to only use the data if still present. http://crbug.com/236476 148 // to only use the data if still present. http://crbug.com/236476
150 ExtraDataMap::iterator extra_data = extra_data_map_->find(web_contents()); 149 ExtraDataMap::iterator extra_data = extra_data_map_->find(web_contents());
151 if (extra_data != extra_data_map_->end()) { 150 if (extra_data != extra_data_map_->end()) {
152 extra_data->second.has_already_shown_a_dialog_ = true; 151 extra_data->second.has_already_shown_a_dialog_ = true;
153 extra_data->second.suppress_javascript_messages_ = suppress_js_messages; 152 extra_data->second.suppress_javascript_messages_ = suppress_js_messages;
154 } 153 }
155 154
156 // On Views, we can end up coming through this code path twice :(. 155 // On Views, we can end up coming through this code path twice :(.
157 // See crbug.com/63732. 156 // See crbug.com/63732.
158 AppModalDialog::Invalidate(false); 157 AppModalDialog::Invalidate();
159 } 158 }
160 159
161 void JavaScriptAppModalDialog::CallDialogClosedCallback(bool success, 160 void JavaScriptAppModalDialog::CallDialogClosedCallback(bool success,
162 const base::string16& user_input) { 161 const base::string16& user_input) {
163 // TODO(joenotcharles): Both the callers of this function also check IsValid 162 // TODO(joenotcharles): Both the callers of this function also check IsValid
164 // and call AppModalDialog::Invalidate, but in different orders. If the 163 // and call AppModalDialog::Invalidate, but in different orders. If the
165 // difference is not significant, more common code could be moved here. 164 // difference is not significant, more common code could be moved here.
166 UMA_HISTOGRAM_MEDIUM_TIMES( 165 UMA_HISTOGRAM_MEDIUM_TIMES(
167 "JSDialogs.FineTiming.TimeBetweenDialogCreatedAndSameDialogClosed", 166 "JSDialogs.FineTiming.TimeBetweenDialogCreatedAndSameDialogClosed",
168 base::TimeTicks::Now() - creation_time_); 167 base::TimeTicks::Now() - creation_time_);
169 if (!callback_.is_null()) { 168 if (!callback_.is_null()) {
170 callback_.Run(success, user_input); 169 callback_.Run(success, user_input);
171 callback_.Reset(); 170 callback_.Reset();
172 } 171 }
173 } 172 }
174 173
175 } // namespace app_modal 174 } // namespace app_modal
OLDNEW
« no previous file with comments | « components/app_modal/javascript_app_modal_dialog.h ('k') | components/app_modal/javascript_dialog_manager.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698