Move JavaScriptDialogManager, JavascriptAppModalDialogViews to components/app_modal_dialogs

components/app_modal_dialogs/javascript_dialog_manager.cc

Index: components/app_modal_dialogs/javascript_dialog_manager.cc
diff --git a/components/app_modal_dialogs/javascript_dialog_manager.cc b/components/app_modal_dialogs/javascript_dialog_manager.cc
new file mode 100644
index 0000000000000000000000000000000000000000..c370da52be504d637ddb6f687bf514c32f20fa13
--- /dev/null
+++ b/components/app_modal_dialogs/javascript_dialog_manager.cc
@@ -0,0 +1,26 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/app_modal_dialogs/javascript_dialog_manager.h"

[msw, 2014/11/05 03:53:55]

You can remove this file and let clients use the Impl class via
javascript_dialog_manager_impl.h directly (and rename that to drop "Impl").

[oshima, 2014/11/05 21:42:18]

I did ask this question in #20, and avi@ seems to be ok with this.

There is already content::JavaScriptDialogManager, so I want to avoid having the
class with same name
even if it's in a separate namespace.

I'm happy to change to whatever you and avi@ agrees though. avi@, what is your
opinion?

[msw, 2014/11/05 22:59:06]

Avi missed this question, and I don't have a strong opinion. You can leave it
as-is for now and consider removing this file in a followup cleanup CL.

[Avi (use Gerrit), 2014/11/06 05:26:57]

Hmmm. Yes, I did miss this.

Impl makes sense in something like content, where there is a public/ directory
with an interface, and a private implementation. What does an impl class mean
for a component? Is it meaningful at all?

I'm completely OK with two classes named the same thing in different namespaces,
if it makes sense, and I think that it easily makes sense here.

[oshima, 2014/11/06 06:32:06]

JavaScriptDialogManager is an interface in public directory
(content/public/browser),
and this provides the implementation of it. This used to be
ChromeJavaScriptDialogManager when it was in
chrome/browser, and I just needed some name to replace it.
I'm happy to do that if that's what you want, but let me to do it in a separate
CL.

+
+#include "base/memory/scoped_ptr.h"

[msw, 2014/11/05 03:53:55]

nit: the header includes this, so it's not needed here.

[oshima, 2014/11/05 21:42:18]

Done.

+#include "components/app_modal_dialogs/javascript_dialog_extensions_client.h"
+#include "components/app_modal_dialogs/javascript_dialog_manager_impl.h"
+#include "components/app_modal_dialogs/javascript_native_dialog_factory.h"
+
+content::JavaScriptDialogManager* GetJavaScriptDialogManagerInstance() {
+  return JavaScriptDialogManagerImpl::GetInstance();
+}
+
+void SetJavaScriptDialogExtensionsClient(
+    scoped_ptr<JavaScriptDialogExtensionsClient> new_client) {
+  JavaScriptDialogManagerImpl::GetInstance()->SetExtensionsClient(

[msw, 2014/11/05 03:53:55]

nit: use GetJavaScriptDialogManagerInstance.

[oshima, 2014/11/05 21:42:18]

The type needs to be Impl. (The method above returns
content::JavaScriptDialogManager*)

+      new_client.Pass());
+}
+
+void SetJavaScriptNativeDialogFactory(

[msw, 2014/11/05 03:53:55]

nit: reorder to match declarations (if you keep this file).

[oshima, 2014/11/05 21:42:18]

Done.

+    scoped_ptr<JavaScriptNativeDialogFactory> new_factory) {
+  JavaScriptDialogManagerImpl::GetInstance()->SetNativeDialogFactory(

[msw, 2014/11/05 03:53:55]

ditto nit: use GetJavaScriptDialogManagerInstance.

[oshima, 2014/11/05 21:42:18]

same here

+      new_factory.Pass());
+}