Enable chrome://device-emulator in non-official NDEBUG builds.

Enable chrome://device-emulator in non-official NDEBUG builds.

This page is useful to chromium developers working with
ChromeOS builds on Linux -- even if they are running Release
development builds.

Also, add to chrome://about page which lists all internal pages.

Committed: https://crrev.com/431efdf14cc04a5d710e5ec62f95b5405dc8ee17
Cr-Commit-Position: refs/heads/master@{#430040}

[scheib, 2016-10-21 05:50:17]

The CQ bit was checked by scheib@chromium.org to run a CQ dry run

[scheib, 2016-10-21 05:50:38]

scheib@chromium.org changed reviewers:
+ michaelpg@chromium.org

[commit-bot: I haz the power, 2016-10-21 05:50:42]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[scheib, 2016-10-21 05:52:26]

https://codereview.chromium.org/2435033004/diff/1/chrome/common/url_constants.cc
File chrome/common/url_constants.cc (right):

https://codereview.chromium.org/2435033004/diff/1/chrome/common/url_constants...
chrome/common/url_constants.cc:611: kChromeUIBluetoothInternalsHost,
FYI the indent here is based on "git cl format", automated formatting for the
win.

[commit-bot: I haz the power, 2016-10-21 06:55:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-21 06:55:19]

Dry run: This issue passed the CQ dry run.

[michaelpg, 2016-10-22 03:25:29]

https://codereview.chromium.org/2435033004/diff/1/chrome/common/url_constants.cc
File chrome/common/url_constants.cc (right):

https://codereview.chromium.org/2435033004/diff/1/chrome/common/url_constants...
chrome/common/url_constants.cc:611: kChromeUIBluetoothInternalsHost,
On 2016/10/21 05:52:26, scheib wrote:
> FYI the indent here is based on "git cl format", automated formatting for the
> win.

The 2-space indentation is quite common, consistent with the rest of the file
(kChromeDebugURLs), and AFAIK acceptable.

For the sake of blame history I would revert this change.

[scheib, 2016-10-22 05:04:04]

On 2016/10/22 03:25:29, michaelpg wrote:
>
https://codereview.chromium.org/2435033004/diff/1/chrome/common/url_constants.cc
> File chrome/common/url_constants.cc (right):
> 
>
https://codereview.chromium.org/2435033004/diff/1/chrome/common/url_constants...
> chrome/common/url_constants.cc:611: kChromeUIBluetoothInternalsHost,
> On 2016/10/21 05:52:26, scheib wrote:
> > FYI the indent here is based on "git cl format", automated formatting for
the
> > win.
> 
> The 2-space indentation is quite common, consistent with the rest of the file
> (kChromeDebugURLs), and AFAIK acceptable.
> 
> For the sake of blame history I would revert this change.

"By policy, Clang's formatting of code should always be accepted in code
reviews."
https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md

[michaelpg, 2016-10-22 23:28:29]

michaelpg@chromium.org changed reviewers:
+ stevenjb@chromium.org

[michaelpg, 2016-10-22 23:28:29]

+stevenjb for FYI

https://codereview.chromium.org/2435033004/diff/1/chrome/browser/ui/webui/chr...
File chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc (right):

https://codereview.chromium.org/2435033004/diff/1/chrome/browser/ui/webui/chr...
chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc:519: return
&NewWebUI<DeviceEmulatorUI>;
IIUC, removing the !NDEBUG preprocessor checks will make this code compile on
Official builds, but the DeviceEmulatorUI and MessageHandler themselves are
still not compiled on official builds. How could this link?

https://codereview.chromium.org/2435033004/diff/1/chrome/common/url_constants.cc
File chrome/common/url_constants.cc (right):

https://codereview.chromium.org/2435033004/diff/1/chrome/common/url_constants...
chrome/common/url_constants.cc:611: kChromeUIBluetoothInternalsHost,
On 2016/10/22 03:25:29, michaelpg wrote:
> On 2016/10/21 05:52:26, scheib wrote:
> > FYI the indent here is based on "git cl format", automated formatting for
the
> > win.
> 
> The 2-space indentation is quite common, consistent with the rest of the file
> (kChromeDebugURLs), and AFAIK acceptable.
> 
> For the sake of blame history I would revert this change.

Acknowledged. Also, I changed my mind -- 2-space indentation apparently is not
acceptable, because this form of array initialization is a braced-init-list, so
the style guide mandates 4 spaces:
https://google.github.io/styleguide/cppguide.html#Braced_Initializer_List_Format

That being said... it would be nice if this file could be made internally
consistent. I wouldn't gate on that, though, just my compilation concerns.

https://codereview.chromium.org/2435033004/diff/1/chrome/common/url_constants...
chrome/common/url_constants.cc:619: kChromeUIDeviceEmulatorHost,
Is it OK to include this when the WebUI itself doesn't exist?
* running on Chrome OS, or
* using an official build

[stevenjb, 2016-10-24 17:18:55]

https://codereview.chromium.org/2435033004/diff/1/chrome/browser/ui/webui/chr...
File chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc (right):

https://codereview.chromium.org/2435033004/diff/1/chrome/browser/ui/webui/chr...
chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc:519: return
&NewWebUI<DeviceEmulatorUI>;
On 2016/10/22 23:28:29, michaelpg wrote:
> IIUC, removing the !NDEBUG preprocessor checks will make this code compile on
> Official builds, but the DeviceEmulatorUI and MessageHandler themselves are
> still not compiled on official builds. How could this link?

+1, I think this code and the include need to be wrapped in
defined(OFFICIAL_BUILD).

[scheib, 2016-11-03 23:57:55]

The CQ bit was checked by scheib@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-03 23:58:53]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[scheib, 2016-11-03 23:58:59]

Thanks, PTAL

https://codereview.chromium.org/2435033004/diff/1/chrome/browser/ui/webui/chr...
File chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc (right):

https://codereview.chromium.org/2435033004/diff/1/chrome/browser/ui/webui/chr...
chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc:519: return
&NewWebUI<DeviceEmulatorUI>;
On 2016/10/24 17:18:55, stevenjb wrote:
> On 2016/10/22 23:28:29, michaelpg wrote:
> > IIUC, removing the !NDEBUG preprocessor checks will make this code compile
on
> > Official builds, but the DeviceEmulatorUI and MessageHandler themselves are
> > still not compiled on official builds. How could this link?
> 
> +1, I think this code and the include need to be wrapped in
> defined(OFFICIAL_BUILD).

Done.

https://codereview.chromium.org/2435033004/diff/1/chrome/common/url_constants.cc
File chrome/common/url_constants.cc (right):

https://codereview.chromium.org/2435033004/diff/1/chrome/common/url_constants...
chrome/common/url_constants.cc:611: kChromeUIBluetoothInternalsHost,
On 2016/10/22 23:28:29, michaelpg wrote:
> On 2016/10/22 03:25:29, michaelpg wrote:
> > On 2016/10/21 05:52:26, scheib wrote:
> > > FYI the indent here is based on "git cl format", automated formatting for
> the
> > > win.
> > 
> > The 2-space indentation is quite common, consistent with the rest of the
file
> > (kChromeDebugURLs), and AFAIK acceptable.
> > 
> > For the sake of blame history I would revert this change.
> 
> Acknowledged. Also, I changed my mind -- 2-space indentation apparently is not
> acceptable, because this form of array initialization is a braced-init-list,
so
> the style guide mandates 4 spaces:
>
https://google.github.io/styleguide/cppguide.html#Braced_Initializer_List_Format
> 
> That being said... it would be nice if this file could be made internally
> consistent. I wouldn't gate on that, though, just my compilation concerns.

Done, corrected the other array as well.

https://codereview.chromium.org/2435033004/diff/1/chrome/common/url_constants...
chrome/common/url_constants.cc:619: kChromeUIDeviceEmulatorHost,
On 2016/10/22 23:28:29, michaelpg wrote:
> Is it OK to include this when the WebUI itself doesn't exist?
> * running on Chrome OS, or
> * using an official build

Done.

[commit-bot: I haz the power, 2016-11-04 00:04:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-04 00:04:23]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[scheib, 2016-11-04 21:00:56]

The CQ bit was checked by scheib@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-04 21:02:16]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[michaelpg, 2016-11-04 21:20:14]

lgtm

https://codereview.chromium.org/2435033004/diff/1/chrome/common/url_constants.cc
File chrome/common/url_constants.cc (right):

https://codereview.chromium.org/2435033004/diff/1/chrome/common/url_constants...
chrome/common/url_constants.cc:611: kChromeUIBluetoothInternalsHost,
On 2016/11/03 23:58:59, scheib wrote:
> On 2016/10/22 23:28:29, michaelpg wrote:
> > On 2016/10/22 03:25:29, michaelpg wrote:
> > > On 2016/10/21 05:52:26, scheib wrote:
> > > > FYI the indent here is based on "git cl format", automated formatting
for
> > the
> > > > win.
> > > 
> > > The 2-space indentation is quite common, consistent with the rest of the
> file
> > > (kChromeDebugURLs), and AFAIK acceptable.
> > > 
> > > For the sake of blame history I would revert this change.
> > 
> > Acknowledged. Also, I changed my mind -- 2-space indentation apparently is
not
> > acceptable, because this form of array initialization is a braced-init-list,
> so
> > the style guide mandates 4 spaces:
> >
>
https://google.github.io/styleguide/cppguide.html#Braced_Initializer_List_Format
> > 
> > That being said... it would be nice if this file could be made internally
> > consistent. I wouldn't gate on that, though, just my compilation concerns.
> 
> Done, corrected the other array as well.

Thanks.

[scheib, 2016-11-04 22:11:01]

The CQ bit was unchecked by scheib@chromium.org

[scheib, 2016-11-04 22:11:01]

The CQ bit was checked by scheib@chromium.org

[commit-bot: I haz the power, 2016-11-04 22:11:45]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-04 22:39:28]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-11-04 22:42:45]

Description was changed from

==========
Enable chrome://device-emulator in non-official NDEBUG builds.

This page is useful to chromium developers working with
ChromeOS builds on Linux -- even if they are running Release
development builds.

Also, add to chrome://about page which lists all internal pages.
==========

to

==========
Enable chrome://device-emulator in non-official NDEBUG builds.

This page is useful to chromium developers working with
ChromeOS builds on Linux -- even if they are running Release
development builds.

Also, add to chrome://about page which lists all internal pages.

Committed: https://crrev.com/431efdf14cc04a5d710e5ec62f95b5405dc8ee17
Cr-Commit-Position: refs/heads/master@{#430040}
==========

[commit-bot: I haz the power, 2016-11-04 22:42:46]

Patchset 3 (id:??) landed as
https://crrev.com/431efdf14cc04a5d710e5ec62f95b5405dc8ee17
Cr-Commit-Position: refs/heads/master@{#430040}