MD Settings: Certificate manager, populate UI.

MD Settings: Certificate manager, populate UI.

 - Display all certificates.
 - Display menus for manipulating certificates.
 - Hook up a few of the menu actions.

Actions that require dialogs, will be hooked up in follow up CL.

BUG=547073
Committed: https://crrev.com/a5fdfbe215a6d9c9a8822082a45085625b5477c2
Cr-Commit-Position: refs/heads/master@{#383347}

[dpapad, 2016-03-18 22:39:04]

Description was changed from

==========
MD Settings: Certificate manager, basic UI.

BUG=547073
==========

to

==========
MD Settings: Certificate manager, basic UI.

 - Display all certificates.
 - Display menus for manipulating certificates.
 - Hook up a few of the menu actions.

Actions that require dialogs, will be hooked up in follow up CL.

BUG=547073
==========

[dpapad, 2016-03-19 01:18:14]

Description was changed from

==========
MD Settings: Certificate manager, basic UI.

 - Display all certificates.
 - Display menus for manipulating certificates.
 - Hook up a few of the menu actions.

Actions that require dialogs, will be hooked up in follow up CL.

BUG=547073
==========

to

==========
MD Settings: Certificate manager, populate UI.

 - Display all certificates.
 - Display menus for manipulating certificates.
 - Hook up a few of the menu actions.

Actions that require dialogs, will be hooked up in follow up CL.

BUG=547073
==========

[dpapad, 2016-03-19 01:28:53]

dpapad@chromium.org changed reviewers:
+ dbeam@chromium.org

[dpapad, 2016-03-19 01:28:53]

Screenshots at http://imgur.com/a/jhNjp.

This CL focuses on adding the new UI and populating it. Polish (colors, font
styles etc) and optimizations (maybe iron-list instead of dom-repeat) are left
for follow up CLs.

[dpapad, 2016-03-22 00:56:44]

Friendly ping.

[Dan Beam, 2016-03-22 02:08:29]

https://codereview.chromium.org/1819563002/diff/80001/chrome/browser/resource...
File
chrome/browser/resources/settings/certificate_manager_page/certificate_entry.html
(right):

https://codereview.chromium.org/1819563002/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/certificate_manager_page/certificate_entry.html:24:
<iron-collapse opened="[[expanded_]]" no-animation>
do we use no-animation everywhere?

https://codereview.chromium.org/1819563002/diff/80001/chrome/browser/resource...
File
chrome/browser/resources/settings/certificate_manager_page/certificate_list.js
(right):

https://codereview.chromium.org/1819563002/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/certificate_manager_page/certificate_list.js:10:
is: 'settings-certificate-list',
nit: \n

https://codereview.chromium.org/1819563002/diff/80001/chrome/browser/resource...
File
chrome/browser/resources/settings/certificate_manager_page/certificate_manager_page.html
(right):

https://codereview.chromium.org/1819563002/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/certificate_manager_page/certificate_manager_page.html:16:
--paper-tabs-selection-bar-color: blue;
where did you get this color from?

https://codereview.chromium.org/1819563002/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/certificate_manager_page/certificate_manager_page.html:19:
<paper-tabs selected="{{selected}}" no-slide>
why no-slide?

https://codereview.chromium.org/1819563002/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/certificate_manager_page/certificate_manager_page.html:29:
-->
just mirror the enum somewhere on |this|, as in

  properties: {
    certType_: settings.CertificateType,
  },

then, from here:

  certificate-type="[[certType_.PERSONAL]]"
  if="[[isTabSelected_(selected, certType_.PERSONAL)]]"

or whatevers

https://codereview.chromium.org/1819563002/diff/80001/chrome/browser/resource...
File
chrome/browser/resources/settings/certificate_manager_page/certificate_manager_page.js
(right):

https://codereview.chromium.org/1819563002/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/certificate_manager_page/certificate_manager_page.js:47:
browserProxy_: null,
nit: if you only use this once, it may be cheaper/simpler to acquire access only
when needed, i.e. drop |this.browserProxy_| and just do:

  settings.CertificatesBrowserProxyImpl.getInstance().refreshCertificates();

it would additionally defer constructing the class until necessary

https://codereview.chromium.org/1819563002/diff/80001/chrome/browser/resource...
File
chrome/browser/resources/settings/certificate_manager_page/certificate_subentry.html
(right):

https://codereview.chromium.org/1819563002/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/certificate_manager_page/certificate_subentry.html:15:
box-shadow: 0 2px 6px grey;
should grey be a --paper or --google var()?

https://codereview.chromium.org/1819563002/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/certificate_manager_page/certificate_subentry.html:26:
background-color: #f0f0f0;
same with this color: can it be a --paper or --google var?

https://codereview.chromium.org/1819563002/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/certificate_manager_page/certificate_subentry.html:37:
<div class="dropdown-content">
indent off (should only be \s\s instead of \s\s\s\s)

https://codereview.chromium.org/1819563002/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/certificate_manager_page/certificate_subentry.html:56:
<div>
this should be </div>

https://codereview.chromium.org/1819563002/diff/80001/chrome/test/data/webui/...
File chrome/test/data/webui/settings/certificate_manager_page_test.js (right):

https://codereview.chromium.org/1819563002/diff/80001/chrome/test/data/webui/...
chrome/test/data/webui/settings/certificate_manager_page_test.js:346:
paperTabsElement.selected = 0;
what is different about how the selection code works between selected = {0, 1,
2, 3}?  why not just test one of these?

https://codereview.chromium.org/1819563002/diff/80001/chrome/test/data/webui/...
chrome/test/data/webui/settings/certificate_manager_page_test.js:366:
assertCertificateListLength(0 /* personalCerts */, 0);
can you use an enum instead of copying the constant?

[dpapad, 2016-03-22 18:07:02]

https://codereview.chromium.org/1819563002/diff/80001/chrome/browser/resource...
File
chrome/browser/resources/settings/certificate_manager_page/certificate_entry.html
(right):

https://codereview.chromium.org/1819563002/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/certificate_manager_page/certificate_entry.html:24:
<iron-collapse opened="[[expanded_]]" no-animation>
On 2016/03/22 at 02:08:28, Dan Beam wrote:
> do we use no-animation everywhere?

Not that I know of. The reason I am using it here is for performance (and
probably as a temporary solution). If I don't use no-animation here performance
is degraded, because I also need to remove the dom-if below (otherwise the
iron-collapse does not work).

I would like to explore alternatives (iron-list or other tricks) once the page
has been marked as functional.

https://codereview.chromium.org/1819563002/diff/80001/chrome/browser/resource...
File
chrome/browser/resources/settings/certificate_manager_page/certificate_list.js
(right):

https://codereview.chromium.org/1819563002/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/certificate_manager_page/certificate_list.js:10:
is: 'settings-certificate-list',
On 2016/03/22 at 02:08:28, Dan Beam wrote:
> nit: \n

Done.

https://codereview.chromium.org/1819563002/diff/80001/chrome/browser/resource...
File
chrome/browser/resources/settings/certificate_manager_page/certificate_manager_page.html
(right):

https://codereview.chromium.org/1819563002/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/certificate_manager_page/certificate_manager_page.html:16:
--paper-tabs-selection-bar-color: blue;
On 2016/03/22 at 02:08:28, Dan Beam wrote:
> where did you get this color from?

It is blue-ish on the mocks. Should probably be changed to a MD spec blue-ish
color. As said in the CL description, the focus of this CL is functionality, not
so much polish.

https://codereview.chromium.org/1819563002/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/certificate_manager_page/certificate_manager_page.html:19:
<paper-tabs selected="{{selected}}" no-slide>
On 2016/03/22 at 02:08:28, Dan Beam wrote:
> why no-slide?

Removed. It looked funky IMO, and the mocks don't specify slide or no-slide.
Also we don't use paper-tabs anywhere else in Settings, so there is no other
example.

https://codereview.chromium.org/1819563002/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/certificate_manager_page/certificate_manager_page.html:29:
-->
On 2016/03/22 at 02:08:28, Dan Beam wrote:
> just mirror the enum somewhere on |this|, as in
> 
>   properties: {
>     certType_: settings.CertificateType,
>   },
> 
> then, from here:
> 
>   certificate-type="[[certType_.PERSONAL]]"
>   if="[[isTabSelected_(selected, certType_.PERSONAL)]]"
> 
> or whatevers

1/2 Done. I mirrored the enum, such that I don't need to replicate the string
literals.

The 2nd part of your suggestion (modifying the signature of isTabSelected)
requires keeping track of additional unnecessary state (certificate type to
numerical index, such that it can be compared to "selected"), so I don't see the
benefit.

https://codereview.chromium.org/1819563002/diff/80001/chrome/browser/resource...
File
chrome/browser/resources/settings/certificate_manager_page/certificate_manager_page.js
(right):

https://codereview.chromium.org/1819563002/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/certificate_manager_page/certificate_manager_page.js:47:
browserProxy_: null,
On 2016/03/22 at 02:08:28, Dan Beam wrote:
> nit: if you only use this once, it may be cheaper/simpler to acquire access
only when needed, i.e. drop |this.browserProxy_| and just do:
> 
>   settings.CertificatesBrowserProxyImpl.getInstance().refreshCertificates();
> 
> it would additionally defer constructing the class until necessary

Done.

https://codereview.chromium.org/1819563002/diff/80001/chrome/browser/resource...
File
chrome/browser/resources/settings/certificate_manager_page/certificate_subentry.html
(right):

https://codereview.chromium.org/1819563002/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/certificate_manager_page/certificate_subentry.html:15:
box-shadow: 0 2px 6px grey;
On 2016/03/22 at 02:08:28, Dan Beam wrote:
> should grey be a --paper or --google var()?

Yes, probably. Added TODO. I copied this value from similar menu in
searchEngines, see
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res....
Prefer to do polish in separate CL though.

https://codereview.chromium.org/1819563002/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/certificate_manager_page/certificate_subentry.html:26:
background-color: #f0f0f0;
On 2016/03/22 at 02:08:28, Dan Beam wrote:
> same with this color: can it be a --paper or --google var?

Done.

https://codereview.chromium.org/1819563002/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/certificate_manager_page/certificate_subentry.html:37:
<div class="dropdown-content">
On 2016/03/22 at 02:08:28, Dan Beam wrote:
> indent off (should only be \s\s instead of \s\s\s\s)

Done.

https://codereview.chromium.org/1819563002/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/certificate_manager_page/certificate_subentry.html:56:
<div>
On 2016/03/22 at 02:08:28, Dan Beam wrote:
> this should be </div>

Done. Good catch.

https://codereview.chromium.org/1819563002/diff/80001/chrome/test/data/webui/...
File chrome/test/data/webui/settings/certificate_manager_page_test.js (right):

https://codereview.chromium.org/1819563002/diff/80001/chrome/test/data/webui/...
chrome/test/data/webui/settings/certificate_manager_page_test.js:346:
paperTabsElement.selected = 0;
On 2016/03/22 at 02:08:28, Dan Beam wrote:
> what is different about how the selection code works between selected = {0, 1,
2, 3}?  why not just test one of these?

Later in this test I am testing that those 4 sections are populated according to
the information coming from the test browser proxy (see lines 382-385). So here
I am simply forcing the 4 dom-ifs to be flipped to true and therefore these tabs
to be added to the DOM.

Testing only one of these means that there is less test coverage.

https://codereview.chromium.org/1819563002/diff/80001/chrome/test/data/webui/...
chrome/test/data/webui/settings/certificate_manager_page_test.js:366:
assertCertificateListLength(0 /* personalCerts */, 0);
On 2016/03/22 at 02:08:29, Dan Beam wrote:
> can you use an enum instead of copying the constant?

Done.

[Dan Beam, 2016-03-22 19:21:25]

lgtm w/nits

https://codereview.chromium.org/1819563002/diff/100001/chrome/browser/resourc...
File
chrome/browser/resources/settings/certificate_manager_page/certificate_manager_page.html
(right):

https://codereview.chromium.org/1819563002/diff/100001/chrome/browser/resourc...
chrome/browser/resources/settings/certificate_manager_page/certificate_manager_page.html:15:
:host {
/* TODO(dpapad): real color. */

or something like that

https://codereview.chromium.org/1819563002/diff/100001/chrome/browser/resourc...
chrome/browser/resources/settings/certificate_manager_page/certificate_manager_page.html:26:
<div>
does this need a dom-if as well?

https://codereview.chromium.org/1819563002/diff/100001/chrome/browser/resourc...
File
chrome/browser/resources/settings/certificate_manager_page/certificate_subentry.html
(right):

https://codereview.chromium.org/1819563002/diff/100001/chrome/browser/resourc...
chrome/browser/resources/settings/certificate_manager_page/certificate_subentry.html:16:
such menus (like in search engines section) */
nit: * such menus (like in search engines section) */
     ^

https://codereview.chromium.org/1819563002/diff/100001/chrome/browser/resourc...
chrome/browser/resources/settings/certificate_manager_page/certificate_subentry.html:18:
}
nit: \n between each rule?

https://codereview.chromium.org/1819563002/diff/100001/chrome/browser/resourc...
chrome/browser/resources/settings/certificate_manager_page/certificate_subentry.html:37:
<iron-dropdown vertical-align="top" horizontal-align="right"
does this flip in RTL?

[dpapad, 2016-03-22 20:39:09]

https://codereview.chromium.org/1819563002/diff/100001/chrome/browser/resourc...
File
chrome/browser/resources/settings/certificate_manager_page/certificate_manager_page.html
(right):

https://codereview.chromium.org/1819563002/diff/100001/chrome/browser/resourc...
chrome/browser/resources/settings/certificate_manager_page/certificate_manager_page.html:15:
:host {
On 2016/03/22 at 19:21:25, Dan Beam wrote:
> /* TODO(dpapad): real color. */
> 
> or something like that

Done.

https://codereview.chromium.org/1819563002/diff/100001/chrome/browser/resourc...
chrome/browser/resources/settings/certificate_manager_page/certificate_manager_page.html:26:
<div>
On 2016/03/22 at 19:21:25, Dan Beam wrote:
> does this need a dom-if as well?

This tab is always displayed since it is the default one (selected defaults to
0), so don't see a benefit from dom-ifing this one.

https://codereview.chromium.org/1819563002/diff/100001/chrome/browser/resourc...
File
chrome/browser/resources/settings/certificate_manager_page/certificate_subentry.html
(right):

https://codereview.chromium.org/1819563002/diff/100001/chrome/browser/resourc...
chrome/browser/resources/settings/certificate_manager_page/certificate_subentry.html:16:
such menus (like in search engines section) */
On 2016/03/22 at 19:21:25, Dan Beam wrote:
> nit: * such menus (like in search engines section) */
>      ^

Done.

https://codereview.chromium.org/1819563002/diff/100001/chrome/browser/resourc...
chrome/browser/resources/settings/certificate_manager_page/certificate_subentry.html:18:
}
On 2016/03/22 at 19:21:25, Dan Beam wrote:
> nit: \n between each rule?

Done.

https://codereview.chromium.org/1819563002/diff/100001/chrome/browser/resourc...
chrome/browser/resources/settings/certificate_manager_page/certificate_subentry.html:37:
<iron-dropdown vertical-align="top" horizontal-align="right"
On 2016/03/22 at 19:21:25, Dan Beam wrote:
> does this flip in RTL?

Just tried this. Something weird is happening. The 1st time the menu opens at a
clearly wrong position. Then, closing and re-opening the menu makes it look
correctly. Added a TODO to keep track (BTW search engines similar menu does not
open at all in RTL).

[dpapad, 2016-03-22 21:01:58]

The CQ bit was checked by dpapad@chromium.org

[dpapad, 2016-03-22 21:01:59]

The patchset sent to the CQ was uploaded after l-g-t-m from dbeam@chromium.org
Link to the patchset: https://codereview.chromium.org/1819563002/#ps120001
(title: "Address comments.")

[commit-bot: I haz the power, 2016-03-22 21:02:22]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1819563002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1819563002/120001

[commit-bot: I haz the power, 2016-03-22 22:26:23]

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

[commit-bot: I haz the power, 2016-03-22 22:26:24]

Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[dpapad, 2016-03-23 01:34:34]

The CQ bit was checked by dpapad@chromium.org

[commit-bot: I haz the power, 2016-03-23 01:34:48]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1819563002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1819563002/120001

[commit-bot: I haz the power, 2016-03-23 02:33:34]

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

[commit-bot: I haz the power, 2016-03-23 02:33:35]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[dpapad, 2016-03-23 23:14:47]

On 2016/03/23 at 02:33:35, commit-bot wrote:
> Try jobs failed on following builders:
>   win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

After looking at the test failures, I realize there is one more thing that needs
to be fixed. Currently there is a chrome.send('refreshCertificates') message
being sent from the UI that is not handled for cases where USE_NSS ifdef
variable is false. I'll look into the fix and update this CL accordingly.

[dpapad, 2016-03-24 01:40:17]

On 2016/03/23 at 23:14:47, dpapad wrote:
> On 2016/03/23 at 02:33:35, commit-bot wrote:
> > Try jobs failed on following builders:
> >   win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
> 
> After looking at the test failures, I realize there is one more thing that
needs to be fixed. Currently there is a chrome.send('refreshCertificates')
message being sent from the UI that is not handled for cases where USE_NSS ifdef
variable is false. I'll look into the fix and update this CL accordingly.

Fixed. @dbeam please take a look at patch #8.

[Dan Beam, 2016-03-24 17:57:16]

lgtm, but i don't like continuing this trend

https://codereview.chromium.org/1819563002/diff/140001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc
(right):

https://codereview.chromium.org/1819563002/diff/140001/chrome/browser/ui/webu...
chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc:266:
#endif
why are you pushing your handler's logic into a class named "localized strings"?

[dpapad, 2016-03-24 18:07:03]

https://codereview.chromium.org/1819563002/diff/140001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc
(right):

https://codereview.chromium.org/1819563002/diff/140001/chrome/browser/ui/webu...
chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc:266:
#endif
On 2016/03/24 at 17:57:16, Dan Beam wrote:
> why are you pushing your handler's logic into a class named "localized
strings"?

Because the handler only exists if USE_NSS_CERTS is true but loadTimeData
"useNssCerts" needs to be available either way, see
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/....
I am not trying to continue any trend you don't like, happy to talk about
alternatives.

[Dan Beam, 2016-03-24 18:19:34]

https://codereview.chromium.org/1819563002/diff/140001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc
(right):

https://codereview.chromium.org/1819563002/diff/140001/chrome/browser/ui/webu...
chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc:266:
#endif
On 2016/03/24 18:07:03, dpapad wrote:
> On 2016/03/24 at 17:57:16, Dan Beam wrote:
> > why are you pushing your handler's logic into a class named "localized
> strings"?
> 
> Because the handler only exists if USE_NSS_CERTS is true but loadTimeData
> "useNssCerts" needs to be available either way, see
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/....
> I am not trying to continue any trend you don't like, happy to talk about
> alternatives.

why not just use <if> in the markup instead?
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...

[dpapad, 2016-03-24 18:28:16]

On 2016/03/24 at 18:19:34, dbeam wrote:
>
https://codereview.chromium.org/1819563002/diff/140001/chrome/browser/ui/webu...
> File
chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc
(right):
> 
>
https://codereview.chromium.org/1819563002/diff/140001/chrome/browser/ui/webu...
>
chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc:266:
#endif
> On 2016/03/24 18:07:03, dpapad wrote:
> > On 2016/03/24 at 17:57:16, Dan Beam wrote:
> > > why are you pushing your handler's logic into a class named "localized
> > strings"?
> > 
> > Because the handler only exists if USE_NSS_CERTS is true but loadTimeData
> > "useNssCerts" needs to be available either way, see
> >
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/....
> > I am not trying to continue any trend you don't like, happy to talk about
> > alternatives.
> 
> why not just use <if> in the markup instead?
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...

Trying this now in a separate CL. Why didn't you suggest this earlier?
As revealed by the TODO at
https://codereview.chromium.org/1819563002/diff2/120001:140001/chrome/browser...
this is what I was intending to do as a long term solution.

[dpapad, 2016-03-24 19:37:09]

Patchset #9 (id:160001) has been deleted

[dpapad, 2016-03-24 21:33:43]

https://codereview.chromium.org/1819563002/diff/140001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc
(right):

https://codereview.chromium.org/1819563002/diff/140001/chrome/browser/ui/webu...
chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc:266:
#endif
On 2016/03/24 at 18:19:34, Dan Beam wrote:
> On 2016/03/24 18:07:03, dpapad wrote:
> > On 2016/03/24 at 17:57:16, Dan Beam wrote:
> > > why are you pushing your handler's logic into a class named "localized
> > strings"?
> > 
> > Because the handler only exists if USE_NSS_CERTS is true but loadTimeData
> > "useNssCerts" needs to be available either way, see
> >
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/....
> > I am not trying to continue any trend you don't like, happy to talk about
> > alternatives.
> 
> why not just use <if> in the markup instead?
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...

Done at https://codereview.chromium.org/1831093002. Once that CL lands, I will
rebase this one (and there will be no need for loadTimeDtaa useNssCerts
anymore).

[dpapad, 2016-03-25 18:24:01]

https://codereview.chromium.org/1819563002/diff/140001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc
(right):

https://codereview.chromium.org/1819563002/diff/140001/chrome/browser/ui/webu...
chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc:266:
#endif
On 2016/03/24 at 21:33:43, dpapad wrote:
> On 2016/03/24 at 18:19:34, Dan Beam wrote:
> > On 2016/03/24 18:07:03, dpapad wrote:
> > > On 2016/03/24 at 17:57:16, Dan Beam wrote:
> > > > why are you pushing your handler's logic into a class named "localized
> > > strings"?
> > > 
> > > Because the handler only exists if USE_NSS_CERTS is true but loadTimeData
> > > "useNssCerts" needs to be available either way, see
> > >
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/....
> > > I am not trying to continue any trend you don't like, happy to talk about
> > > alternatives.
> > 
> > why not just use <if> in the markup instead?
> >
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...
> 
> Done at https://codereview.chromium.org/1831093002. Once that CL lands, I will
rebase this one (and there will be no need for loadTimeDtaa useNssCerts
anymore).

Done. Parent CL landed. Removed 'useNssCerts' loadTimeData since it is not
needed anymore.

[dpapad, 2016-03-25 19:15:39]

The CQ bit was checked by dpapad@chromium.org

[dpapad, 2016-03-25 19:15:40]

The patchset sent to the CQ was uploaded after l-g-t-m from dbeam@chromium.org
Link to the patchset: https://codereview.chromium.org/1819563002/#ps180001
(title: "Rebase")

[commit-bot: I haz the power, 2016-03-25 19:15:53]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1819563002/180001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1819563002/180001

[commit-bot: I haz the power, 2016-03-25 20:36:58]

Committed patchset #9 (id:180001)

[commit-bot: I haz the power, 2016-03-25 20:38:17]

Description was changed from

==========
MD Settings: Certificate manager, populate UI.

 - Display all certificates.
 - Display menus for manipulating certificates.
 - Hook up a few of the menu actions.

Actions that require dialogs, will be hooked up in follow up CL.

BUG=547073
==========

to

==========
MD Settings: Certificate manager, populate UI.

 - Display all certificates.
 - Display menus for manipulating certificates.
 - Hook up a few of the menu actions.

Actions that require dialogs, will be hooked up in follow up CL.

BUG=547073

Committed: https://crrev.com/a5fdfbe215a6d9c9a8822082a45085625b5477c2
Cr-Commit-Position: refs/heads/master@{#383347}
==========

[commit-bot: I haz the power, 2016-03-25 20:38:18]

Patchset 9 (id:??) landed as
https://crrev.com/a5fdfbe215a6d9c9a8822082a45085625b5477c2
Cr-Commit-Position: refs/heads/master@{#383347}