MD Settings: Certificate manager, hooking up all dialogs.

MD Settings: Certificate manager, hooking up all dialogs.

 - When a menu option is selected, dispatching a CertificateActionEvent.
 - Listening for that event from certificate_manager_page.js and opening
   appropriate dialog.
 - Also listening for the certificates-error event, such that the error dialog can
   be shown.

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

[dpapad, 2016-03-22 21:08:17]

Description was changed from

==========
MD Settings: Certificate manager, hooking up all dialogs.

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

to

==========
MD Settings: Certificate manager, hooking up all dialogs.

 - When a menu option is selected, dispatching a CertificateAction event.
 - Listening for that event from certificate_manager_page.js and opening
   appropriate dialog.
 - Also listening for the certificates-error event, such that the error dialog
can
   be shown.

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

[dpapad, 2016-03-22 21:57:06]

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

[dpapad, 2016-03-22 21:57:07]

Finally! The CL we have already talked about during previous reviews.

[dpapad, 2016-03-22 21:57:36]

Description was changed from

==========
MD Settings: Certificate manager, hooking up all dialogs.

 - When a menu option is selected, dispatching a CertificateAction event.
 - Listening for that event from certificate_manager_page.js and opening
   appropriate dialog.
 - Also listening for the certificates-error event, such that the error dialog
can
   be shown.

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

to

==========
MD Settings: Certificate manager, hooking up all dialogs.

 - When a menu option is selected, dispatching a CertificateActionEvent.
 - Listening for that event from certificate_manager_page.js and opening
   appropriate dialog.
 - Also listening for the certificates-error event, such that the error dialog
can
   be shown.

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

[tommycli, 2016-03-23 19:28:05]

lgtm. No substantive comments, just a bunch of nitpicking below.

Also, I think your patch breaks some bots that don't have USE_NSS_CERTS defined.

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

https://codereview.chromium.org/1815733004/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/certificate_manager_page/ca_trust_edit_dialog.js:63:
function(error) {
Nit: Can we redo the indentation on this a bit?

It took me a while to understand that the function(error) was another parameter
of the then() call because of indendation.

https://codereview.chromium.org/1815733004/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/certificate_manager_page/ca_trust_edit_dialog.js:65:
this.dispatchEvent(
Any particular reason we're using dispatchEvent here instead of fire? I've seen
fire more commonly used around our Settings code.

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

https://codereview.chromium.org/1815733004/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/certificate_manager_page/certificate_delete_confirmation_dialog.js:89:
function(error) {
Nit: Same comment here, the indentation confused me.

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

https://codereview.chromium.org/1815733004/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/certificate_manager_page/certificate_manager_page.js:145:
openDialog_: function(dialogTagName, domIfBooleanName) {
The comment above really helps me understand it!

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

https://codereview.chromium.org/1815733004/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/certificate_manager_page/certificate_password_decryption_dialog.js:45:
function(error) {
nit: indent

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

https://codereview.chromium.org/1815733004/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/certificate_manager_page/certificate_subentry.js:85:
onError_: function(error) {
Since this can also indicate that the user clicked Cancel, can we think of a
better name than onError_? Maybe onCertificateActionRejected_ ?

https://codereview.chromium.org/1815733004/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/certificate_manager_page/certificate_subentry.js:135:
}.bind(this), this.onError_.bind(this));
nit: All of these this.onError_.bind(this) args should be on a new line to make
clear they are a second arg of the then() fn.

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

https://codereview.chromium.org/1815733004/diff/80001/chrome/test/data/webui/...
chrome/test/data/webui/settings/certificate_manager_page_test.js:163:
target.addEventListener(eventType, resolve);
Is it a problem that we never remove the event listener?

https://codereview.chromium.org/1815733004/diff/80001/chrome/test/data/webui/...
chrome/test/data/webui/settings/certificate_manager_page_test.js:166: 
nit: extra newline

https://codereview.chromium.org/1815733004/diff/80001/chrome/test/data/webui/...
chrome/test/data/webui/settings/certificate_manager_page_test.js:211: return
browserProxy.whenCalled('editCaCertificateTrust');
Interesting. I assume you are just linearizing the then chain?

https://codereview.chromium.org/1815733004/diff/80001/chrome/test/data/webui/...
chrome/test/data/webui/settings/certificate_manager_page_test.js:367: var
whenErrorEventFired = waitForEvent('certificates-error', dialog);
It is a bit unfortunate that we have to attach this handler beforehand since it
makes it harder to read. I don't have a concrete suggestion for improving this
though.

Maybe declare this in setup()? Maybe call this errorEventPromise or something to
make clear it's a promise that's fulfilled only when there's an error?

https://codereview.chromium.org/1815733004/diff/80001/chrome/test/data/webui/...
chrome/test/data/webui/settings/certificate_manager_page_test.js:445: var
waitForActionEvent = function() {
Same comment as above, it would be easier for me to read if this was
actionEventPromise, so it was crystal clear it was a promise only fulfilled by
the action event.

https://codereview.chromium.org/1815733004/diff/80001/chrome/test/data/webui/...
chrome/test/data/webui/settings/certificate_manager_page_test.js:485: var
whenActionEventFired = waitForActionEvent();
Same comment as above, I just found this required a bit of thinking to
understand.

[dpapad, 2016-03-24 22:06:11]

Test failures where caused by the parent CL
(https://codereview.chromium.org/1819563002) and are now fixed.

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

https://codereview.chromium.org/1815733004/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/certificate_manager_page/ca_trust_edit_dialog.js:63:
function(error) {
On 2016/03/23 at 19:28:04, tommycli wrote:
> Nit: Can we redo the indentation on this a bit?
> 
> It took me a while to understand that the function(error) was another
parameter of the then() call because of indendation.

Can you suggest an alternative indentation that works? I could not find another
one given the existing constraints. The existence of @param prevents me from
moving "function (error)" to the previous line.

https://codereview.chromium.org/1815733004/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/certificate_manager_page/ca_trust_edit_dialog.js:65:
this.dispatchEvent(
On 2016/03/23 at 19:28:04, tommycli wrote:
> Any particular reason we're using dispatchEvent here instead of fire? I've
seen fire more commonly used around our Settings code.

Changed all of them to fire() for consistency.

The reason I was not using it is that "fire()" seems like an unnecessary wrapper
around what I was doing. If you take a look at the implementation
(https://github.com/Polymer/polymer/blob/441103159f7b9fc9af567b2caea145de21a95...),
it mostly makes sense if the event caching feature of Polymer was being used.

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

https://codereview.chromium.org/1815733004/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/certificate_manager_page/certificate_delete_confirmation_dialog.js:89:
function(error) {
On 2016/03/23 at 19:28:04, tommycli wrote:
> Nit: Same comment here, the indentation confused me.

See previous response to similar comment.

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

https://codereview.chromium.org/1815733004/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/certificate_manager_page/certificate_manager_page.js:145:
openDialog_: function(dialogTagName, domIfBooleanName) {
On 2016/03/23 at 19:28:04, tommycli wrote:
> The comment above really helps me understand it!

Great!

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

https://codereview.chromium.org/1815733004/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/certificate_manager_page/certificate_subentry.js:85:
onError_: function(error) {
On 2016/03/23 at 19:28:04, tommycli wrote:
> Since this can also indicate that the user clicked Cancel, can we think of a
better name than onError_? Maybe onCertificateActionRejected_ ?

Done. I chose "onRejected_" which is shorter but still conveys the fact that a
rejection is not necessarily an error.

https://codereview.chromium.org/1815733004/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/certificate_manager_page/certificate_subentry.js:135:
}.bind(this), this.onError_.bind(this));
On 2016/03/23 at 19:28:04, tommycli wrote:
> nit: All of these this.onError_.bind(this) args should be on a new line to
make clear they are a second arg of the then() fn.

Done.

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

https://codereview.chromium.org/1815733004/diff/80001/chrome/test/data/webui/...
chrome/test/data/webui/settings/certificate_manager_page_test.js:163:
target.addEventListener(eventType, resolve);
On 2016/03/23 at 19:28:05, tommycli wrote:
> Is it a problem that we never remove the event listener?

It is not a problem, for two reasons.
1) "target" is destroyed in teardown() of every test, so the listener is
destroyed along with it.
2) Even if the event fired twice (which does not happen), a promise can only be
fulfilled once, and silently ignores any subsequent calls to resolve(),
reject().

https://codereview.chromium.org/1815733004/diff/80001/chrome/test/data/webui/...
chrome/test/data/webui/settings/certificate_manager_page_test.js:166: 
On 2016/03/23 at 19:28:05, tommycli wrote:
> nit: extra newline

Removed.

https://codereview.chromium.org/1815733004/diff/80001/chrome/test/data/webui/...
chrome/test/data/webui/settings/certificate_manager_page_test.js:211: return
browserProxy.whenCalled('editCaCertificateTrust');
On 2016/03/23 at 19:28:05, tommycli wrote:
> Interesting. I assume you are just linearizing the then chain?

Yes, this is a typical case of chaining promises one after the other.

return promiseA.then(function(resultA) {
  // use resultA here
  return promiseB;
}).then(function(resultB) {
  // use resultB here
});

Instead of the equivalent, but less readable

return promiseA.then(function(resultA) {
  // use resultA here
  return promiseB.then(function(resultB) {
    // use resultB here
  });
});

https://codereview.chromium.org/1815733004/diff/80001/chrome/test/data/webui/...
chrome/test/data/webui/settings/certificate_manager_page_test.js:367: var
whenErrorEventFired = waitForEvent('certificates-error', dialog);
On 2016/03/23 at 19:28:05, tommycli wrote:
> It is a bit unfortunate that we have to attach this handler beforehand since
it makes it harder to read. I don't have a concrete suggestion for improving
this though.
> 
> Maybe declare this in setup()? Maybe call this errorEventPromise or something
to make clear it's a promise that's fulfilled only when there's an error?

Done. I moved it to the setup() per your 1st suggestion, although now this
variable is alive in 3 different scopes instead of only being alive in the scope
it is being used.

Regarding your 2nd suggestion of renaming: The convention I have been using to
indicate that some variable is a promise is prefixing it with "when", instead of
suffixing it with "Promise". The former makes it read more naturally
"whenFoo.then(doSomething)" compared "fooPromise.then(doSomething)".

Therefore, I was hoping that seeing "whenErrorEventFired" already makes it clear
that it is a Promise firing once "error event fired".

https://codereview.chromium.org/1815733004/diff/80001/chrome/test/data/webui/...
chrome/test/data/webui/settings/certificate_manager_page_test.js:445: var
waitForActionEvent = function() {
On 2016/03/23 at 19:28:04, tommycli wrote:
> Same comment as above, it would be easier for me to read if this was
actionEventPromise, so it was crystal clear it was a promise only fulfilled by
the action event.

Added JSDoc for the local function which I think makes it crystal clear. See
previous comment regarding explanation of not using the "Promise" suffix.

https://codereview.chromium.org/1815733004/diff/80001/chrome/test/data/webui/...
chrome/test/data/webui/settings/certificate_manager_page_test.js:485: var
whenActionEventFired = waitForActionEvent();
On 2016/03/23 at 19:28:04, tommycli wrote:
> Same comment as above, I just found this required a bit of thinking to
understand.

I am sensing that you are not very fond/comfortable with the Promise usage here
(because of repeated related comments). Would you prefer to stop converting
event-to-promise with waitForEvent() helper and do the following instead?

test('MenuOptions_Edit', function() {
  ...
  return new Promise(function(resolve, reject) {
    subentry.addEventListener(settings.CertificateActionEvent, function(event) {
      // do assertions here.
      resolve();
    });
  });
});

My intention with the event-to-promise waitForEvent() helper was to make things
more readable by reducing the repetition of "return new Promise(....)",  given
the fact that a Promise needs to be returned by test(function() {}) to indicate
an async test to Mocha.

[dpapad, 2016-03-24 22:11:17]

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

https://codereview.chromium.org/1815733004/diff/80001/chrome/test/data/webui/...
chrome/test/data/webui/settings/certificate_manager_page_test.js:485: var
whenActionEventFired = waitForActionEvent();
To be more precise I forgot some stuff in my snippet above

test('MenuOptions_Edit', function() {
  ...
  return new Promise(function(resolve, reject) {
    subentry.addEventListener(settings.CertificateActionEvent, function(event) {
      try {
        // do assertions here..
      } catch (e) {
        reject(e);
      }
      resolve();
    });
    MockInteractions.tap(editButton);
  });
});

I am trying to avoid repeating this verbose snippet over and over (by using the
event-to-promise helper).

[tommycli, 2016-03-24 22:34:14]

still lgtm

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

https://codereview.chromium.org/1815733004/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/certificate_manager_page/ca_trust_edit_dialog.js:63:
function(error) {
On 2016/03/24 22:06:10, dpapad wrote:
> On 2016/03/23 at 19:28:04, tommycli wrote:
> > Nit: Can we redo the indentation on this a bit?
> > 
> > It took me a while to understand that the function(error) was another
> parameter of the then() call because of indendation.
> 
> Can you suggest an alternative indentation that works? I could not find
another
> one given the existing constraints. The existence of @param prevents me from
> moving "function (error)" to the previous line.

Does this work? I realize it's a great deal of indenting, but it disambiguates
it for me.

    this.browserProxy_.editCaCertificateTrust(
        this.model.id, this.$.ssl.checked,
        this.$.email.checked, this.$.objSign.checked).then(
            function() {
              this.$.spinner.active = false;
              this.$.dialog.close();
            }.bind(this),
            /** @param {!CertificatesError} error */
            function(error) {
              // TODO(dpapad): Display error here.
            });

https://codereview.chromium.org/1815733004/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/certificate_manager_page/ca_trust_edit_dialog.js:65:
this.dispatchEvent(
On 2016/03/24 22:06:10, dpapad wrote:
> On 2016/03/23 at 19:28:04, tommycli wrote:
> > Any particular reason we're using dispatchEvent here instead of fire? I've
> seen fire more commonly used around our Settings code.
> 
> Changed all of them to fire() for consistency.
> 
> The reason I was not using it is that "fire()" seems like an unnecessary
wrapper
> around what I was doing. If you take a look at the implementation
>
(https://github.com/Polymer/polymer/blob/441103159f7b9fc9af567b2caea145de21a95...),
> it mostly makes sense if the event caching feature of Polymer was being used.

Arg... that's unfortunate. I'm not so sure about my suggestion anymore. I guess
for consistency it's probably worth paying the performance cost.

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

https://codereview.chromium.org/1815733004/diff/80001/chrome/test/data/webui/...
chrome/test/data/webui/settings/certificate_manager_page_test.js:367: var
whenErrorEventFired = waitForEvent('certificates-error', dialog);
On 2016/03/24 22:06:11, dpapad wrote:
> On 2016/03/23 at 19:28:05, tommycli wrote:
> > It is a bit unfortunate that we have to attach this handler beforehand since
> it makes it harder to read. I don't have a concrete suggestion for improving
> this though.
> > 
> > Maybe declare this in setup()? Maybe call this errorEventPromise or
something
> to make clear it's a promise that's fulfilled only when there's an error?
> 
> Done. I moved it to the setup() per your 1st suggestion, although now this
> variable is alive in 3 different scopes instead of only being alive in the
scope
> it is being used.
> 
> Regarding your 2nd suggestion of renaming: The convention I have been using to
> indicate that some variable is a promise is prefixing it with "when", instead
of
> suffixing it with "Promise". The former makes it read more naturally
> "whenFoo.then(doSomething)" compared "fooPromise.then(doSomething)".
> 
> Therefore, I was hoping that seeing "whenErrorEventFired" already makes it
clear
> that it is a Promise firing once "error event fired".

Ah given your point about the scope maybe the original location was better.

About the naming: I agree that the whenFoo.then(something) is very easy to
understand. However, when you just "return whenErrorEventFired;" it's less
obvious to me that it's a promise.

Maybe just comment above it "// A promise that's resolved when the event is
fired" ?

In any case this is just nitpicking, it's all good with me ultimately.

https://codereview.chromium.org/1815733004/diff/80001/chrome/test/data/webui/...
chrome/test/data/webui/settings/certificate_manager_page_test.js:485: var
whenActionEventFired = waitForActionEvent();
On 2016/03/24 22:11:17, dpapad wrote:
> To be more precise I forgot some stuff in my snippet above
> 
> test('MenuOptions_Edit', function() {
>   ...
>   return new Promise(function(resolve, reject) {
>     subentry.addEventListener(settings.CertificateActionEvent, function(event)
{
>       try {
>         // do assertions here..
>       } catch (e) {
>         reject(e);
>       }
>       resolve();
>     });
>     MockInteractions.tap(editButton);
>   });
> });
> 
> I am trying to avoid repeating this verbose snippet over and over (by using
the
> event-to-promise helper).

Hi Demetrios, Although the above would be easy to understand, I agree with you
that it's too verbose.

I think I'm just having trouble with the waitForActionEvent() method, since it's
not clear to me that it returns a Promise.

When it's used as waitForFoo().then(bar), then it's very clear.

But when it's just 'return waitForFoo()', it confuses me. This may just be
because I'm not as familiar with this usage pattern.

What if it was named eventToPromise? Ultimately these are all fine.

[dpapad, 2016-03-25 17:36:03]

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

https://codereview.chromium.org/1815733004/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/certificate_manager_page/ca_trust_edit_dialog.js:63:
function(error) {
On 2016/03/24 at 22:34:14, tommycli wrote:
> On 2016/03/24 22:06:10, dpapad wrote:
> > On 2016/03/23 at 19:28:04, tommycli wrote:
> > > Nit: Can we redo the indentation on this a bit?
> > > 
> > > It took me a while to understand that the function(error) was another
> > parameter of the then() call because of indendation.
> > 
> > Can you suggest an alternative indentation that works? I could not find
another
> > one given the existing constraints. The existence of @param prevents me from
> > moving "function (error)" to the previous line.
> 
> Does this work? I realize it's a great deal of indenting, but it disambiguates
it for me.
> 
>     this.browserProxy_.editCaCertificateTrust(
>         this.model.id, this.$.ssl.checked,
>         this.$.email.checked, this.$.objSign.checked).then(
>             function() {
>               this.$.spinner.active = false;
>               this.$.dialog.close();
>             }.bind(this),
>             /** @param {!CertificatesError} error */
>             function(error) {
>               // TODO(dpapad): Display error here.
>             });

Done.

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

https://codereview.chromium.org/1815733004/diff/80001/chrome/test/data/webui/...
chrome/test/data/webui/settings/certificate_manager_page_test.js:485: var
whenActionEventFired = waitForActionEvent();
On 2016/03/24 at 22:34:14, tommycli wrote:
> On 2016/03/24 22:11:17, dpapad wrote:
> > To be more precise I forgot some stuff in my snippet above
> > 
> > test('MenuOptions_Edit', function() {
> >   ...
> >   return new Promise(function(resolve, reject) {
> >     subentry.addEventListener(settings.CertificateActionEvent,
function(event) {
> >       try {
> >         // do assertions here..
> >       } catch (e) {
> >         reject(e);
> >       }
> >       resolve();
> >     });
> >     MockInteractions.tap(editButton);
> >   });
> > });
> > 
> > I am trying to avoid repeating this verbose snippet over and over (by using
the
> > event-to-promise helper).
> 
> Hi Demetrios, Although the above would be easy to understand, I agree with you
that it's too verbose.
> 
> I think I'm just having trouble with the waitForActionEvent() method, since
it's not clear to me that it returns a Promise.
> 
> When it's used as waitForFoo().then(bar), then it's very clear.
> 
> But when it's just 'return waitForFoo()', it confuses me. This may just be
because I'm not as familiar with this usage pattern.
> 
> What if it was named eventToPromise? Ultimately these are all fine.

Thanks for explaining the points of confusion Tommy, it helped me understand the
issue. So, I followed your suggestions
1) Renamed waitForEvent() -> eventToPromise()
2) Renamed waitForActionEvent() -> actionEventToPromise()
3) Renamed variable whenActionEventFired to waitForActionEvent and added a
comment mentioning that it is a promise in places where I am chaining with
'return waitForActionEvent;'

[tommycli, 2016-03-25 17:38:16]

cool thanks for hearing me out. I am not confused anymore.

[dpapad, 2016-03-25 21:15:42]

The CQ bit was checked by dpapad@chromium.org

[dpapad, 2016-03-25 21:15:42]

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

[commit-bot: I haz the power, 2016-03-25 21:16:18]

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

[commit-bot: I haz the power, 2016-03-25 22:36:09]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-03-25 22:37:25]

Description was changed from

==========
MD Settings: Certificate manager, hooking up all dialogs.

 - When a menu option is selected, dispatching a CertificateActionEvent.
 - Listening for that event from certificate_manager_page.js and opening
   appropriate dialog.
 - Also listening for the certificates-error event, such that the error dialog
can
   be shown.

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

to

==========
MD Settings: Certificate manager, hooking up all dialogs.

 - When a menu option is selected, dispatching a CertificateActionEvent.
 - Listening for that event from certificate_manager_page.js and opening
   appropriate dialog.
 - Also listening for the certificates-error event, such that the error dialog
can
   be shown.

BUG=547073

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

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

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