Replace TOUCH_UI condition in WebUI with dynamic flag.

Replace TOUCH_UI condition in WebUI with dynamic flag.

Touch-specific builds are being retired.  Instead we want to be able to enable touch-friendly UI dynamically.  For now I'm adding a --touch-optimized-ui flag which can be used to enable the touch-optimized CSS tweaks we've made to WebUI.

This undoes the tweak to use the non-ChromeOS language UI for touch_ui builds.  The conditional HTML for the add-language-overlay-page is non-trivial to convert to something dynamic - I'll do something in a separate changelist.

Also fix a number of issues where overrides of WebUI virtuals (like RenderViewCreated) weren't properly delegating back to their base class (or were delegating to WebUI instead of the immediate base ChromeWebUI).

Also removes the Touch-specific User-Agent string.

BUG=105315
TEST=


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

[Rick Byers, 2011-11-25 14:28:50]

Hi,
We're eliminating the TOUCH_UI build and merging it's functionality with the
USE_AURA build.  Erik, can you please review this change in how we select
touch-friendly WebUI?  I think it's directly analogous to how we do OS-specific
or toolkit-specific UI.  We'll want to be more sophisticated about this over
time, but I think this is the best place to start.

Darin, as OWNER please review the tiny change under 'webkit' to remove the
TOUCH_UI specific user-agent string.

Thanks,
   Rick

[darin (slow to review), 2011-11-28 17:15:07]

http://codereview.chromium.org/8695007/diff/9001/webkit/glue/user_agent.cc
File webkit/glue/user_agent.cc (left):

http://codereview.chromium.org/8695007/diff/9001/webkit/glue/user_agent.cc#ol...
webkit/glue/user_agent.cc:90: #if defined(TOUCH_UI)
LGTM

[Rick Byers, 2011-11-30 15:34:10]

Rob can you please take a look?

[flackr, 2011-11-30 16:00:46]

LGTM with nits.

http://codereview.chromium.org/8695007/diff/9001/chrome/browser/ui/webui/chro...
File chrome/browser/ui/webui/chrome_web_ui.cc (right):

http://codereview.chromium.org/8695007/diff/9001/chrome/browser/ui/webui/chro...
chrome/browser/ui/webui/chrome_web_ui.cc:47:
render_view_host->SetWebUIProperty("touchOptimized","true");
Add a space before "true".

http://codereview.chromium.org/8695007/diff/9001/chrome/browser/ui/webui/chro...
File chrome/browser/ui/webui/chrome_web_ui.h (right):

http://codereview.chromium.org/8695007/diff/9001/chrome/browser/ui/webui/chro...
chrome/browser/ui/webui/chrome_web_ui.h:25: // Overridden from
WebUI::RenderViewCreated
Method name in overridden comment is redundant, also add punctuation at end of
line, : is most common for these overridden from comments.

[Rick Byers, 2011-11-30 16:23:06]

Thanks Rob!

http://codereview.chromium.org/8695007/diff/9001/chrome/browser/ui/webui/chro...
File chrome/browser/ui/webui/chrome_web_ui.cc (right):

http://codereview.chromium.org/8695007/diff/9001/chrome/browser/ui/webui/chro...
chrome/browser/ui/webui/chrome_web_ui.cc:47:
render_view_host->SetWebUIProperty("touchOptimized","true");
On 2011/11/30 16:00:46, flackr wrote:
> Add a space before "true".

Done.

http://codereview.chromium.org/8695007/diff/9001/chrome/browser/ui/webui/chro...
File chrome/browser/ui/webui/chrome_web_ui.h (right):

http://codereview.chromium.org/8695007/diff/9001/chrome/browser/ui/webui/chro...
chrome/browser/ui/webui/chrome_web_ui.h:25: // Overridden from
WebUI::RenderViewCreated
On 2011/11/30 16:00:46, flackr wrote:
> Method name in overridden comment is redundant, also add punctuation at end of
> line, : is most common for these overridden from comments.

Done.

[commit-bot: I haz the power, 2011-11-30 16:23:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rbyers@chromium.org/8695007/16001

[commit-bot: I haz the power, 2011-11-30 16:23:33]

Presubmit check for 8695007-16001 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit Messages **
If this change requires manual test instructions to QA team, add
TEST=[instructions].

** Presubmit ERRORS **
Missing LGTM from an OWNER for:
chrome/browser/resources/options/language_add_language_overlay.html,chrome/browser/resources/options/language_options.js,chrome/browser/resources/shared/js/cr/ui/card_slider.js,chrome/browser/resources/shared/js/cr.js,chrome/browser/resources/options/language_add_language_overlay.js,chrome/browser/resources/ntp4/new_tab.html,chrome/browser/resources/options/options.js,chrome/browser/resources/options/language_options.css,chrome/browser/resources/options/options_page.css

Presubmit checks took 2.3s to calculate.

[Rick Byers, 2011-11-30 16:33:57]

Looks like I need an LGTM from estade as well.  Evan, can you please review this
one too?

[arv (Not doing code reviews), 2011-11-30 17:36:05]

LGTM

I apologize for not seeing this earlier. Feel free to ping me on IM if I do not
respond to a code review within a day.

http://codereview.chromium.org/8695007/diff/16001/chrome/browser/resources/op...
File chrome/browser/resources/options/options.js (right):

http://codereview.chromium.org/8695007/diff/16001/chrome/browser/resources/op...
chrome/browser/resources/options/options.js:106: if
($('language-options-virtual-keyboard')) {
This looks a bit ad hoc. I kind of preferred the old way here.

[Rick Byers, 2011-11-30 17:52:25]

On 2011/11/30 17:36:05, arv wrote:
> LGTM
> 
> I apologize for not seeing this earlier. Feel free to ping me on IM if I do
not
> respond to a code review within a day.

Thanks Erik.  I realized today I needed Evan to LGTM the NTP4 change anyway
(since he's the only owner of ntp4), so figured it wasn't worth hassling you
too.

[Evan Stade, 2011-11-30 23:36:09]

lgtm sorry for delay.

http://codereview.chromium.org/8695007/diff/16001/chrome/browser/ui/webui/ntp...
File chrome/browser/ui/webui/ntp/new_tab_ui.cc (right):

http://codereview.chromium.org/8695007/diff/16001/chrome/browser/ui/webui/ntp...
chrome/browser/ui/webui/ntp/new_tab_ui.cc:157: }
\n

[Rick Byers, 2011-12-01 01:21:44]

Forgot to publish this response

http://codereview.chromium.org/8695007/diff/16001/chrome/browser/resources/op...
File chrome/browser/resources/options/options.js (right):

http://codereview.chromium.org/8695007/diff/16001/chrome/browser/resources/op...
chrome/browser/resources/options/options.js:106: if
($('language-options-virtual-keyboard')) {
On 2011/11/30 17:36:06, arv wrote:
> This looks a bit ad hoc. I kind of preferred the old way here.

Yeah, I debated this.  We could plumb through a new cr.hasVirtualKeyboard flag
(we can't use cr.isTouchOptimized because that's no longer connected with there
being support for virtual keyboards).  But really what's going on here is that
VirtualKeyboardManager.getInstance assumes that the DOM elements for the
keyboard (all under language-options-virtual-keyboard) exist.  So I thought it
actually made sense to have the DOM conditional on an ifdef, and then have the
JavaScript all conditional on what pieces are present in the DOM.  This seemed
cleaner to me then adding but-yet-another cr.isFOO property

Alternately perhaps we should convert this to a build-time condition: use <if
expr="pp_ifdef(USE_VIRTUAL_KEYBOARD)"> inside the .js file just like inside the
.html file?  It seems ugly (and confusing to IDEs) to have <if>s inside
JavaScript though.

If we want to base it off the ifdef without <if>, then perhaps instead of
cr.hasVirtualKeyboard, we should add cr.hasBuildDefine('USE_VIRTUAL_KEYBOARD') -
plumb all possible pre-processor symbols through at once rather than piecemeal?

[Rick Byers, 2011-12-01 01:29:24]

Thank Evan!

http://codereview.chromium.org/8695007/diff/16001/chrome/browser/ui/webui/ntp...
File chrome/browser/ui/webui/ntp/new_tab_ui.cc (right):

http://codereview.chromium.org/8695007/diff/16001/chrome/browser/ui/webui/ntp...
chrome/browser/ui/webui/ntp/new_tab_ui.cc:157: }
On 2011/11/30 23:36:09, Evan Stade wrote:
> \n

Done.

[commit-bot: I haz the power, 2011-12-01 01:29:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rbyers@chromium.org/8695007/25001

[commit-bot: I haz the power, 2011-12-01 03:03:47]

Change committed as 112387