Implement InputWindowDialog with WebUI for bookmark folder management.

Implement InputWindowDialog with WebUI for bookmark folder management.

WebUI version of InputWindowDialog is used by the chrome built with touchui=1 or use_aura=1.

BUG=97850
TEST=Manually tested with touchui=1 or use_aura=1


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=107096

[tfarina, 2011-10-15 03:55:14]

http://codereview.chromium.org/8298019/diff/2001/chrome/browser/ui/input_wind...
File chrome/browser/ui/input_window_dialog_gtk.h (right):

http://codereview.chromium.org/8298019/diff/2001/chrome/browser/ui/input_wind...
chrome/browser/ui/input_window_dialog_gtk.h:10: #include
"base/memory/scoped_ptr.h"
please add a blank line above.

http://codereview.chromium.org/8298019/diff/2001/chrome/browser/ui/input_wind...
chrome/browser/ui/input_window_dialog_gtk.h:18: const std::string& window_title,
include <string>

http://codereview.chromium.org/8298019/diff/2001/chrome/browser/ui/input_wind...
chrome/browser/ui/input_window_dialog_gtk.h:42: };
DISALLOW_ and basictypes.h for it.

[mazda, 2011-10-17 17:52:25]

Thank you for the comments.

http://codereview.chromium.org/8298019/diff/2001/chrome/browser/ui/input_wind...
File chrome/browser/ui/input_window_dialog_gtk.h (right):

http://codereview.chromium.org/8298019/diff/2001/chrome/browser/ui/input_wind...
chrome/browser/ui/input_window_dialog_gtk.h:10: #include
"base/memory/scoped_ptr.h"
On 2011/10/15 03:55:14, tfarina wrote:
> please add a blank line above.

Done.

http://codereview.chromium.org/8298019/diff/2001/chrome/browser/ui/input_wind...
chrome/browser/ui/input_window_dialog_gtk.h:18: const std::string& window_title,
On 2011/10/15 03:55:14, tfarina wrote:
> include <string>

Done.

http://codereview.chromium.org/8298019/diff/2001/chrome/browser/ui/input_wind...
chrome/browser/ui/input_window_dialog_gtk.h:42: };
On 2011/10/15 03:55:14, tfarina wrote:
> DISALLOW_ and basictypes.h for it.

Done.

[xiyuan, 2011-10-18 16:11:32]

http://codereview.chromium.org/8298019/diff/5001/chrome/browser/browser_resou...
File chrome/browser/browser_resources.grd (right):

http://codereview.chromium.org/8298019/diff/5001/chrome/browser/browser_resou...
chrome/browser/browser_resources.grd:49: <include
name="IDR_INPUT_WINDOW_DIALOG_CSS" file="resources\input_window_dialog.css"
type="BINDATA" />
You don't need to list css here because the way input_window_dialog.html include
this css file would make grit to inline it.

http://codereview.chromium.org/8298019/diff/5001/chrome/browser/resources/inp...
File chrome/browser/resources/input_window_dialog.css (right):

http://codereview.chromium.org/8298019/diff/5001/chrome/browser/resources/inp...
chrome/browser/resources/input_window_dialog.css:6: html, body {
nit: split into two lines. It's good to keep each css selector on its own line.
i.e
html,
body {...

http://codereview.chromium.org/8298019/diff/5001/chrome/browser/resources/inp...
File chrome/browser/resources/input_window_dialog.html (right):

http://codereview.chromium.org/8298019/diff/5001/chrome/browser/resources/inp...
chrome/browser/resources/input_window_dialog.html:14: <script
src="chrome://input-window-dialog/input_window_dialog.js"></script>
You can just use <script src="input_window_dialog.js"></script> and grit would
inline the js file for you.

http://codereview.chromium.org/8298019/diff/5001/chrome/browser/ui/webui/inpu...
File chrome/browser/ui/webui/input_window_dialog_ui.cc (right):

http://codereview.chromium.org/8298019/diff/5001/chrome/browser/ui/webui/inpu...
chrome/browser/ui/webui/input_window_dialog_ui.cc:29:
IDR_INPUT_WINDOW_DIALOG_CSS);
You can get rid of the above two since we can make grit to inline them with the
html.

http://codereview.chromium.org/8298019/diff/5001/chrome/browser/ui/webui/inpu...
File chrome/browser/ui/webui/input_window_dialog_webui.cc (right):

http://codereview.chromium.org/8298019/diff/5001/chrome/browser/ui/webui/inpu...
chrome/browser/ui/webui/input_window_dialog_webui.cc:8: #include <vector>
you don't need them again since they are included in webui.h already.

http://codereview.chromium.org/8298019/diff/5001/chrome/browser/ui/webui/inpu...
chrome/browser/ui/webui/input_window_dialog_webui.cc:23: namespace {
nit: insert an empty line after this line and before the closing "}".
i.e.
namespace {

const int...

}

http://codereview.chromium.org/8298019/diff/5001/chrome/browser/ui/webui/inpu...
File chrome/browser/ui/webui/input_window_dialog_webui.h (right):

http://codereview.chromium.org/8298019/diff/5001/chrome/browser/ui/webui/inpu...
chrome/browser/ui/webui/input_window_dialog_webui.h:64: //  string value in the
text field.
nit: align with previous line.

[mazda, 2011-10-18 17:32:02]

Thank you for the review.
Could you take another look at the change?

http://codereview.chromium.org/8298019/diff/5001/chrome/browser/browser_resou...
File chrome/browser/browser_resources.grd (right):

http://codereview.chromium.org/8298019/diff/5001/chrome/browser/browser_resou...
chrome/browser/browser_resources.grd:49: <include
name="IDR_INPUT_WINDOW_DIALOG_CSS" file="resources\input_window_dialog.css"
type="BINDATA" />
On 2011/10/18 16:11:32, xiyuan wrote:
> You don't need to list css here because the way input_window_dialog.html
include
> this css file would make grit to inline it.

Done.

http://codereview.chromium.org/8298019/diff/5001/chrome/browser/resources/inp...
File chrome/browser/resources/input_window_dialog.css (right):

http://codereview.chromium.org/8298019/diff/5001/chrome/browser/resources/inp...
chrome/browser/resources/input_window_dialog.css:6: html, body {
On 2011/10/18 16:11:32, xiyuan wrote:
> nit: split into two lines. It's good to keep each css selector on its own
line.
> i.e
> html,
> body {...

Done.

http://codereview.chromium.org/8298019/diff/5001/chrome/browser/resources/inp...
File chrome/browser/resources/input_window_dialog.html (right):

http://codereview.chromium.org/8298019/diff/5001/chrome/browser/resources/inp...
chrome/browser/resources/input_window_dialog.html:14: <script
src="chrome://input-window-dialog/input_window_dialog.js"></script>
On 2011/10/18 16:11:32, xiyuan wrote:
> You can just use <script src="input_window_dialog.js"></script> and grit would
> inline the js file for you.

Done.

http://codereview.chromium.org/8298019/diff/5001/chrome/browser/ui/webui/inpu...
File chrome/browser/ui/webui/input_window_dialog_ui.cc (right):

http://codereview.chromium.org/8298019/diff/5001/chrome/browser/ui/webui/inpu...
chrome/browser/ui/webui/input_window_dialog_ui.cc:29:
IDR_INPUT_WINDOW_DIALOG_CSS);
On 2011/10/18 16:11:32, xiyuan wrote:
> You can get rid of the above two since we can make grit to inline them with
the
> html.

Done.
It seems "input_window_dialog.js" is necessary. Otherwise JS error occurs.

http://codereview.chromium.org/8298019/diff/5001/chrome/browser/ui/webui/inpu...
File chrome/browser/ui/webui/input_window_dialog_webui.cc (right):

http://codereview.chromium.org/8298019/diff/5001/chrome/browser/ui/webui/inpu...
chrome/browser/ui/webui/input_window_dialog_webui.cc:8: #include <vector>
On 2011/10/18 16:11:32, xiyuan wrote:
> you don't need them again since they are included in webui.h already.

Done.

http://codereview.chromium.org/8298019/diff/5001/chrome/browser/ui/webui/inpu...
chrome/browser/ui/webui/input_window_dialog_webui.cc:23: namespace {
On 2011/10/18 16:11:32, xiyuan wrote:
> nit: insert an empty line after this line and before the closing "}".
> i.e.
> namespace {
> 
> const int...
> 
> }

Done.

http://codereview.chromium.org/8298019/diff/5001/chrome/browser/ui/webui/inpu...
File chrome/browser/ui/webui/input_window_dialog_webui.h (right):

http://codereview.chromium.org/8298019/diff/5001/chrome/browser/ui/webui/inpu...
chrome/browser/ui/webui/input_window_dialog_webui.h:64: //  string value in the
text field.
On 2011/10/18 16:11:32, xiyuan wrote:
> nit: align with previous line.

Done.

[xiyuan, 2011-10-18 18:58:44]

lgtm

http://codereview.chromium.org/8298019/diff/19002/chrome/browser/browser_reso...
File chrome/browser/browser_resources.grd (right):

http://codereview.chromium.org/8298019/diff/19002/chrome/browser/browser_reso...
chrome/browser/browser_resources.grd:48: <include
name="IDR_INPUT_WINDOW_DIALOG_JS" file="resources\input_window_dialog.js"
type="BINDATA" />
you no longer need this since it's inlined now.

[mazda, 2011-10-18 19:16:08]

Thank you for the review!

http://codereview.chromium.org/8298019/diff/19002/chrome/browser/browser_reso...
File chrome/browser/browser_resources.grd (right):

http://codereview.chromium.org/8298019/diff/19002/chrome/browser/browser_reso...
chrome/browser/browser_resources.grd:48: <include
name="IDR_INPUT_WINDOW_DIALOG_JS" file="resources\input_window_dialog.js"
type="BINDATA" />
On 2011/10/18 18:58:45, xiyuan wrote:
> you no longer need this since it's inlined now.

Done.

[commit-bot: I haz the power, 2011-10-24 04:50:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mazda@chromium.org/8298019/22001

[commit-bot: I haz the power, 2011-10-24 04:50:41]

Failed to send try job 8298019-22001: 2

[commit-bot: I haz the power, 2011-10-25 10:02:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mazda@chromium.org/8298019/27001

[commit-bot: I haz the power, 2011-10-25 11:16:40]

Change committed as 107096