Use new earcons in ChromeVox Next.

Use new earcons in ChromeVox Next.

Add a new Earcon class that generates sounds using
the new cvox2 EarconEngine. Swap it in dynamically
when we're in a ChromeVox Next mode. Add support for
testing earcons to MockFeedback and write an initial
test that all of the correct earcons are outputted
for controls.

BUG=491243
Committed: https://crrev.com/6f3dbc166c172aa3d78e06b181bdb5fb5b09d774
Cr-Commit-Position: refs/heads/master@{#354366}

[dmazzoni, 2015-08-28 23:42:11]

dmazzoni@chromium.org changed reviewers:
+ dtseng@chromium.org, plundblad@chromium.org

[Peter Lundblad, 2015-09-07 21:15:53]

Hi,

A few comments while I took a first look.

https://codereview.chromium.org/1319093003/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/chromevox/cvox2/background/background.js
(right):

https://codereview.chromium.org/1319093003/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/cvox2/background/background.js:719:
if (mode === ChromeVoxMode.FORCE_NEXT || mode === ChromeVoxMode.NEXT) {
I think this means that classic could play cvox2 earcons. Perhaps we shouldn't
change cvox.ChromeVox.earcons to avoid this.

https://codereview.chromium.org/1319093003/diff/1/chrome/browser/resources/ch...
File
chrome/browser/resources/chromeos/chromevox/cvox2/background/earcons_background.js
(right):

https://codereview.chromium.org/1319093003/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/cvox2/background/earcons_background.js:12:
goog.provide('Cvox2EarconsBackground');
Do we need the cvox2 and background parts of this name?  Everything
cvox2 we put unqualified in the top-level namespace and everything
(perhpas with some exceptions) in cvox2 should happen in the background page.

https://codereview.chromium.org/1319093003/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/cvox2/background/earcons_background.js:23:
goog.base(this);
I've stopped using this for new code. Just .call the base cosntructor.

https://codereview.chromium.org/1319093003/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/cvox2/background/earcons_background.js:28:
goog.inherits(Cvox2EarconsBackground, cvox.AbstractEarcons);
And this.

https://codereview.chromium.org/1319093003/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/cvox2/background/earcons_background.js:88:
this.engine_.startProgress();
How does this work if there's more than one page loading at the same time?

[dmazzoni, 2015-09-07 23:43:05]

https://codereview.chromium.org/1319093003/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/chromevox/cvox2/background/background.js
(right):

https://codereview.chromium.org/1319093003/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/cvox2/background/background.js:719:
if (mode === ChromeVoxMode.FORCE_NEXT || mode === ChromeVoxMode.NEXT) {
On 2015/09/07 21:15:52, Peter Lundblad wrote:
> I think this means that classic could play cvox2 earcons. Perhaps we shouldn't
> change cvox.ChromeVox.earcons to avoid this.

My intent here is that you get classic earcons in classic mode and in COMPAT
mode. You should only be in NEXT or FORCE_NEXT if you explicitly opt in, right?
Any urls we whitelist should be COMPAT.

https://codereview.chromium.org/1319093003/diff/1/chrome/browser/resources/ch...
File
chrome/browser/resources/chromeos/chromevox/cvox2/background/earcons_background.js
(right):

https://codereview.chromium.org/1319093003/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/cvox2/background/earcons_background.js:12:
goog.provide('Cvox2EarconsBackground');
On 2015/09/07 21:15:53, Peter Lundblad wrote:
> Do we need the cvox2 and background parts of this name?  Everything
> cvox2 we put unqualified in the top-level namespace and everything
> (perhpas with some exceptions) in cvox2 should happen in the background page.

Open to suggestions, but for a short period of time we need to switch between
two earcon implementations so I want it to be clear.

[dmazzoni, 2015-09-22 19:18:18]

Ready for review. I addressed the feedback and added a test.

https://codereview.chromium.org/1319093003/diff/1/chrome/browser/resources/ch...
File
chrome/browser/resources/chromeos/chromevox/cvox2/background/earcons_background.js
(right):

https://codereview.chromium.org/1319093003/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/cvox2/background/earcons_background.js:12:
goog.provide('Cvox2EarconsBackground');
On 2015/09/07 23:43:04, dmazzoni wrote:
> On 2015/09/07 21:15:53, Peter Lundblad wrote:
> > Do we need the cvox2 and background parts of this name?  Everything
> > cvox2 we put unqualified in the top-level namespace and everything
> > (perhpas with some exceptions) in cvox2 should happen in the background
page.
> 
> Open to suggestions, but for a short period of time we need to switch between
> two earcon implementations so I want it to be clear.

After discussing with David I renamed the old class so now we have
ClassicEarcons and NextEarcons. How does that sound?

https://codereview.chromium.org/1319093003/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/cvox2/background/earcons_background.js:23:
goog.base(this);
On 2015/09/07 21:15:53, Peter Lundblad wrote:
> I've stopped using this for new code. Just .call the base cosntructor.

Done.

https://codereview.chromium.org/1319093003/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/cvox2/background/earcons_background.js:28:
goog.inherits(Cvox2EarconsBackground, cvox.AbstractEarcons);
On 2015/09/07 21:15:52, Peter Lundblad wrote:
> And this.

Done.

https://codereview.chromium.org/1319093003/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/chromevox/cvox2/background/earcons_background.js:88:
this.engine_.startProgress();
On 2015/09/07 21:15:53, Peter Lundblad wrote:
> How does this work if there's more than one page loading at the same time?

Added a TODO. I want this to only matter when the page has focus, but we need
better focus tracking to be able to do that.

[Peter Lundblad, 2015-09-24 14:56:27]

https://codereview.chromium.org/1319093003/diff/20001/chrome/browser/resource...
File
chrome/browser/resources/chromeos/chromevox/cvox2/background/next_earcons.js
(right):

https://codereview.chromium.org/1319093003/diff/20001/chrome/browser/resource...
chrome/browser/resources/chromeos/chromevox/cvox2/background/next_earcons.js:37:
NextEarcons.prototype.getName = function() {
Why not the object literal assigning to the prototype as with other new code?

https://codereview.chromium.org/1319093003/diff/20001/chrome/browser/resource...
chrome/browser/resources/chromeos/chromevox/cvox2/background/next_earcons.js:49:
if (window['console']) {
What environment do you anticipate this running in that doesn't have
console.log?

https://codereview.chromium.org/1319093003/diff/20001/chrome/browser/resource...
File chrome/browser/resources/chromeos/chromevox/testing/mock_feedback.js
(right):

https://codereview.chromium.org/1319093003/diff/20001/chrome/browser/resource...
chrome/browser/resources/chromeos/chromevox/testing/mock_feedback.js:6: *
@fileoverview This file contains the |MockFeedback| class which is
Please add unit tests for the additions to the mock.

[dmazzoni, 2015-09-28 16:33:05]

https://codereview.chromium.org/1319093003/diff/20001/chrome/browser/resource...
File
chrome/browser/resources/chromeos/chromevox/cvox2/background/next_earcons.js
(right):

https://codereview.chromium.org/1319093003/diff/20001/chrome/browser/resource...
chrome/browser/resources/chromeos/chromevox/cvox2/background/next_earcons.js:37:
NextEarcons.prototype.getName = function() {
On 2015/09/24 14:56:27, Peter Lundblad wrote:
> Why not the object literal assigning to the prototype as with other new code?

Done.

https://codereview.chromium.org/1319093003/diff/20001/chrome/browser/resource...
chrome/browser/resources/chromeos/chromevox/cvox2/background/next_earcons.js:49:
if (window['console']) {
On 2015/09/24 14:56:27, Peter Lundblad wrote:
> What environment do you anticipate this running in that doesn't have
> console.log?

Lol, I just inherited this from the other file. Maybe that was from back when
our tests ran on Firefox too because we didn't have enough Chrome WebDriver
configurations?

Fixed here and in two other source files too.

https://codereview.chromium.org/1319093003/diff/20001/chrome/browser/resource...
File chrome/browser/resources/chromeos/chromevox/testing/mock_feedback.js
(right):

https://codereview.chromium.org/1319093003/diff/20001/chrome/browser/resource...
chrome/browser/resources/chromeos/chromevox/testing/mock_feedback.js:6: *
@fileoverview This file contains the |MockFeedback| class which is
On 2015/09/24 14:56:27, Peter Lundblad wrote:
> Please add unit tests for the additions to the mock.

Done.

[Peter Lundblad, 2015-09-29 09:27:34]

https://codereview.chromium.org/1319093003/diff/80001/chrome/browser/resource...
File
chrome/browser/resources/chromeos/chromevox/chromevox/background/background.js
(right):

https://codereview.chromium.org/1319093003/diff/80001/chrome/browser/resource...
chrome/browser/resources/chromeos/chromevox/chromevox/background/background.js:331:
cvox.ChromeVox.earcons.enabled = msg['value'];
We now have two copies of the enabled flag, one for AbstractEarcons
implementation.  That's going to lead to confusing inconsistencies. Could
the flag be read from localStorage each time it is needed?

https://codereview.chromium.org/1319093003/diff/80001/chrome/browser/resource...
File chrome/browser/resources/chromeos/chromevox/cvox2/background/background.js
(right):

https://codereview.chromium.org/1319093003/diff/80001/chrome/browser/resource...
chrome/browser/resources/chromeos/chromevox/cvox2/background/background.js:122:
this.classicEarcons_ = cvox.ChromeVox.earcons || new cvox.ClassicEarcons();
This constructor now depends on only being called once. Should becauseensure
that somehow?

https://codereview.chromium.org/1319093003/diff/80001/chrome/browser/resource...
chrome/browser/resources/chromeos/chromevox/cvox2/background/background.js:758:
}
I am always cautious when introducing state that is not orthogonal
to other state. Could we make cvox.ChromeVox.earcons a getter that would return
the correct instance depending on mode. This way they wouldn't risk getting
out of sync if mode in the future gets modified somewhere else.

https://codereview.chromium.org/1319093003/diff/80001/chrome/browser/resource...
File
chrome/browser/resources/chromeos/chromevox/cvox2/background/next_earcons.js
(right):

https://codereview.chromium.org/1319093003/diff/80001/chrome/browser/resource...
chrome/browser/resources/chromeos/chromevox/cvox2/background/next_earcons.js:8:
*
nit: empty line

https://codereview.chromium.org/1319093003/diff/80001/chrome/browser/resource...
chrome/browser/resources/chromeos/chromevox/cvox2/background/next_earcons.js:26:
* The ChromeVox Next earcon engine.
nit: redundant.

https://codereview.chromium.org/1319093003/diff/80001/chrome/browser/resource...
chrome/browser/resources/chromeos/chromevox/cvox2/background/next_earcons.js:31:
};
The enabled flag is not initialized from the prefs.

https://codereview.chromium.org/1319093003/diff/80001/chrome/browser/resource...
chrome/browser/resources/chromeos/chromevox/cvox2/background/next_earcons.js:34:
NextEarcons.prototype = {
nit: no need for double line break.

https://codereview.chromium.org/1319093003/diff/80001/chrome/browser/resource...
chrome/browser/resources/chromeos/chromevox/cvox2/background/next_earcons.js:69:
break;
Can you add a comment explaining why this is not handled?

https://codereview.chromium.org/1319093003/diff/80001/chrome/browser/resource...
chrome/browser/resources/chromeos/chromevox/cvox2/background/next_earcons.js:84:
break;
Can you add a comment explaining why these are not handled?

https://codereview.chromium.org/1319093003/diff/80001/chrome/browser/resource...
chrome/browser/resources/chromeos/chromevox/cvox2/background/next_earcons.js:95:
case cvox.Earcon.RECOVER_FOCUS:
Can you add a comment explaining why this is not handled?

https://codereview.chromium.org/1319093003/diff/80001/chrome/browser/resource...
File
chrome/browser/resources/chromeos/chromevox/cvox2/background/output_test.extjs
(right):

https://codereview.chromium.org/1319093003/diff/80001/chrome/browser/resource...
chrome/browser/resources/chromeos/chromevox/cvox2/background/output_test.extjs:116:
{value: 0, start: 0, end: 0},
Where does this span of the number 0 come from and what does it mean?

https://codereview.chromium.org/1319093003/diff/80001/chrome/browser/resource...
chrome/browser/resources/chromeos/chromevox/cvox2/background/output_test.extjs:230:
{value: 0, start: 0, end: 0},
Where does 0 come from?

[dmazzoni, 2015-10-01 20:41:43]

Fixed the output bug and other nits, but I still need to initialize the enabled
pref and figure out an alternative to swapping between two earcon
implementations.

I'm not sure I like the idea of a getter because it'd complicate swapping in a
different earcon implementation for testing.

https://chromiumcodereview.appspot.com/1319093003/diff/80001/chrome/browser/r...
File
chrome/browser/resources/chromeos/chromevox/cvox2/background/next_earcons.js
(right):

https://chromiumcodereview.appspot.com/1319093003/diff/80001/chrome/browser/r...
chrome/browser/resources/chromeos/chromevox/cvox2/background/next_earcons.js:8:
*
On 2015/09/29 09:27:34, Peter Lundblad wrote:
> nit: empty line

Done.

https://chromiumcodereview.appspot.com/1319093003/diff/80001/chrome/browser/r...
chrome/browser/resources/chromeos/chromevox/cvox2/background/next_earcons.js:26:
* The ChromeVox Next earcon engine.
On 2015/09/29 09:27:34, Peter Lundblad wrote:
> nit: redundant.

Done.

https://chromiumcodereview.appspot.com/1319093003/diff/80001/chrome/browser/r...
chrome/browser/resources/chromeos/chromevox/cvox2/background/next_earcons.js:34:
NextEarcons.prototype = {
On 2015/09/29 09:27:34, Peter Lundblad wrote:
> nit: no need for double line break.

Done.

https://chromiumcodereview.appspot.com/1319093003/diff/80001/chrome/browser/r...
chrome/browser/resources/chromeos/chromevox/cvox2/background/next_earcons.js:69:
break;
On 2015/09/29 09:27:34, Peter Lundblad wrote:
> Can you add a comment explaining why this is not handled?

Oops, I meant to map this one to onWrap().

https://chromiumcodereview.appspot.com/1319093003/diff/80001/chrome/browser/r...
chrome/browser/resources/chromeos/chromevox/cvox2/background/next_earcons.js:84:
break;
On 2015/09/29 09:27:34, Peter Lundblad wrote:
> Can you add a comment explaining why these are not handled?

Done.

https://chromiumcodereview.appspot.com/1319093003/diff/80001/chrome/browser/r...
chrome/browser/resources/chromeos/chromevox/cvox2/background/next_earcons.js:95:
case cvox.Earcon.RECOVER_FOCUS:
On 2015/09/29 09:27:34, Peter Lundblad wrote:
> Can you add a comment explaining why this is not handled?

Done.

https://chromiumcodereview.appspot.com/1319093003/diff/80001/chrome/browser/r...
File
chrome/browser/resources/chromeos/chromevox/cvox2/background/output_test.extjs
(right):

https://chromiumcodereview.appspot.com/1319093003/diff/80001/chrome/browser/r...
chrome/browser/resources/chromeos/chromevox/cvox2/background/output_test.extjs:116:
{value: 0, start: 0, end: 0},
On 2015/09/29 09:27:34, Peter Lundblad wrote:
> Where does this span of the number 0 come from and what does it mean?

Oops, I thought that was because I got rid of the "if (buff.toString())" check
at the top of go() - but I also had a typo in the code to append the earcons.
Fixed.

https://chromiumcodereview.appspot.com/1319093003/diff/80001/chrome/browser/r...
chrome/browser/resources/chromeos/chromevox/cvox2/background/output_test.extjs:230:
{value: 0, start: 0, end: 0},
On 2015/09/29 09:27:34, Peter Lundblad wrote:
> Where does 0 come from?

Same bug as above. Fixed.

[Peter Lundblad, 2015-10-02 08:59:37]

dmazzoni@chromium.org writes:
> Fixed the output bug and other nits, but I still need to initialize the  
> enabled
> pref and figure out an alternative to swapping between two earcon
> implementations.
> 
> I'm not sure I like the idea of a getter because it'd complicate swapping  
> in a
> different earcon implementation for testing.

We can obviously change it to a function, but to keep the slightly nicer
syntax,
can't you just use Object.defineProperty to redefine it in tests?

Thanks,
//Peter

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[dmazzoni, 2015-10-13 23:07:03]

https://codereview.chromium.org/1319093003/diff/80001/chrome/browser/resource...
File
chrome/browser/resources/chromeos/chromevox/chromevox/background/background.js
(right):

https://codereview.chromium.org/1319093003/diff/80001/chrome/browser/resource...
chrome/browser/resources/chromeos/chromevox/chromevox/background/background.js:331:
cvox.ChromeVox.earcons.enabled = msg['value'];
On 2015/09/29 09:27:33, Peter Lundblad wrote:
> We now have two copies of the enabled flag, one for AbstractEarcons
> implementation.  That's going to lead to confusing inconsistencies. Could
> the flag be read from localStorage each time it is needed?

That doesn't work as well because for Classic support, we don't store prefs in
localStorage in the content script.

Instead I made it a static variable of AbstractEarcons. How's that?

https://codereview.chromium.org/1319093003/diff/80001/chrome/browser/resource...
File chrome/browser/resources/chromeos/chromevox/cvox2/background/background.js
(right):

https://codereview.chromium.org/1319093003/diff/80001/chrome/browser/resource...
chrome/browser/resources/chromeos/chromevox/cvox2/background/background.js:122:
this.classicEarcons_ = cvox.ChromeVox.earcons || new cvox.ClassicEarcons();
On 2015/09/29 09:27:33, Peter Lundblad wrote:
> This constructor now depends on only being called once. Should becauseensure
> that somehow?

I believe this isn't an issue anymore. If we constructed it twice it'd be no big
deal.

https://codereview.chromium.org/1319093003/diff/80001/chrome/browser/resource...
chrome/browser/resources/chromeos/chromevox/cvox2/background/background.js:758:
}
On 2015/09/29 09:27:33, Peter Lundblad wrote:
> I am always cautious when introducing state that is not orthogonal
> to other state. Could we make cvox.ChromeVox.earcons a getter that would
return
> the correct instance depending on mode. This way they wouldn't risk getting
> out of sync if mode in the future gets modified somewhere else.

Done, made it a getter.

https://codereview.chromium.org/1319093003/diff/80001/chrome/browser/resource...
File
chrome/browser/resources/chromeos/chromevox/cvox2/background/next_earcons.js
(right):

https://codereview.chromium.org/1319093003/diff/80001/chrome/browser/resource...
chrome/browser/resources/chromeos/chromevox/cvox2/background/next_earcons.js:31:
};
On 2015/09/29 09:27:34, Peter Lundblad wrote:
> The enabled flag is not initialized from the prefs.

Moved to AbstractEarcons instead.

[Peter Lundblad, 2015-10-14 12:27:26]

lgtm

https://codereview.chromium.org/1319093003/diff/140001/chrome/browser/resourc...
File
chrome/browser/resources/chromeos/chromevox/host/interface/abstract_earcons.js
(right):

https://codereview.chromium.org/1319093003/diff/140001/chrome/browser/resourc...
chrome/browser/resources/chromeos/chromevox/host/interface/abstract_earcons.js:55:
if (localStorage['earcons'] === 'false') {
If we're running in the content script, what local storage are we reading?
Is it shared with the page or the cvox extension or something else?

https://codereview.chromium.org/1319093003/diff/140001/chrome/browser/resourc...
chrome/browser/resources/chromeos/chromevox/host/interface/abstract_earcons.js:90:
cvox.AbstractEarcons.prototype.toggle = function() {
I find it slightly unexpected to have an instance method that changes the global
state. If we go this route, consider moving it from the prototype to the class.

[Peter Lundblad, 2015-10-14 12:32:54]

lgtm

dmazzoni@chromium.org writes:
> 
>
https://codereview.chromium.org/1319093003/diff/80001/chrome/browser/resource...
> File
>
chrome/browser/resources/chromeos/chromevox/chromevox/background/background.js
> (right):
> 
>
https://codereview.chromium.org/1319093003/diff/80001/chrome/browser/resource...
>
chrome/browser/resources/chromeos/chromevox/chromevox/background/background.js:331:
> cvox.ChromeVox.earcons.enabled = msg['value'];
> On 2015/09/29 09:27:33, Peter Lundblad wrote:
> > We now have two copies of the enabled flag, one for AbstractEarcons
> > implementation.  That's going to lead to confusing inconsistencies.
> Could
> > the flag be read from localStorage each time it is needed?
> 
> That doesn't work as well because for Classic support, we don't store
> prefs in localStorage in the content script.
> 
> Instead I made it a static variable of AbstractEarcons. How's that?

OK, works. Another option would have been to make it a getter in the background
page and move the boolean flag to ChromeEarcons, but it is minor and I don't
have a strong opinion.
See question about reading localStorage in the content script in a related
comment.

Thanks,
//Peter

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[dmazzoni, 2015-10-14 16:13:28]

https://codereview.chromium.org/1319093003/diff/140001/chrome/browser/resourc...
File
chrome/browser/resources/chromeos/chromevox/host/interface/abstract_earcons.js
(right):

https://codereview.chromium.org/1319093003/diff/140001/chrome/browser/resourc...
chrome/browser/resources/chromeos/chromevox/host/interface/abstract_earcons.js:55:
if (localStorage['earcons'] === 'false') {
On 2015/10/14 12:27:26, Peter Lundblad wrote:
> If we're running in the content script, what local storage are we reading?
> Is it shared with the page or the cvox extension or something else?

It appears to be shared.

To be safe I moved this initialization to ClassicEarcons and NextEarcons. I
didn't want to duplicate, but this seems safer, and plus we'll be getting rid of
ClassicEarcons after a while.

https://codereview.chromium.org/1319093003/diff/140001/chrome/browser/resourc...
chrome/browser/resources/chromeos/chromevox/host/interface/abstract_earcons.js:90:
cvox.AbstractEarcons.prototype.toggle = function() {
On 2015/10/14 12:27:26, Peter Lundblad wrote:
> I find it slightly unexpected to have an instance method that changes the
global
> state. If we go this route, consider moving it from the prototype to the
class.

The content script extends it, though - it sends the message to the background
page.

[dmazzoni, 2015-10-14 16:36:33]

The CQ bit was checked by dmazzoni@chromium.org

[dmazzoni, 2015-10-14 16:36:33]

The patchset sent to the CQ was uploaded after l-g-t-m from
plundblad@chromium.org
Link to the patchset: https://codereview.chromium.org/1319093003/#ps160001
(title: "Only read localStorage from background page")

[commit-bot: I haz the power, 2015-10-14 16:37:03]

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

[commit-bot: I haz the power, 2015-10-14 16:38:49]

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

[commit-bot: I haz the power, 2015-10-14 16:38:50]

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

[dmazzoni, 2015-10-15 20:41:26]

The CQ bit was checked by dmazzoni@chromium.org

[dmazzoni, 2015-10-15 20:41:26]

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

[dmazzoni, 2015-10-15 20:41:47]

The CQ bit was unchecked by dmazzoni@chromium.org

[dmazzoni, 2015-10-15 20:42:36]

The CQ bit was checked by dmazzoni@chromium.org

[commit-bot: I haz the power, 2015-10-15 20:43:28]

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

[commit-bot: I haz the power, 2015-10-15 21:39:33]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2015-10-15 21:41:01]

Patchset 10 (id:??) landed as
https://crrev.com/6f3dbc166c172aa3d78e06b181bdb5fb5b09d774
Cr-Commit-Position: refs/heads/master@{#354366}