Refactor about:sync's events framework

Refactor about:sync's events framework

This CL changes the way we implement event callbacks on the sync
internals debug page.  We plan to rely more on this framwork in the
future.  This CL ensures we'll be building on a solid foundation.

It replaces chrome_sync.js's custom event listener implementation with
EventTarget from Chrome's internal JavaScript libraries.  Sync doesn't
require anything special in its implementation of event callbacks, so it
makes sense to use shared code for this.

Rather than defining a set of event listener callbacks, all events are
now routed through a single chrome.sync.events EventTarget.  From the
listener's point of view, this doesn't change much, since they still
need to register for a specific event type.  However, it does mean that
chrome_sync.js no longer needs to be aware of all the possible event
types.  Many of the event definitions have been moved into sync_log.js.

This change also introduces a new chrome.sync.handleEvent function to be
called from the C++ layer.  This new scheme will fail a bit more
gracefully than the old system would if the C++ and JavaScript list of
event definitions ever got out of sync.

Finally, this CL adds some more tests for the 'event log' tab of
about:sync.  This helps to ensure that the new system doesn't break
any existing functionality.

BUG=328606
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=250835

[rlarocque, 2014-02-11 22:09:02]

Please review.

A bit of background:

I'm no JS expert, but some parts of the current chrome_sync.js event-handling
code looked wrong to me.  It seems odd that it implemented its own event
listener boilerplate code.  The fact that it pre-defined a list of events and
instantiated a set of event-handling objects during init also seemed strange.  

This is my attempt to make it look a bit more normal.  I'm not entirely sure
what normal is supposed to look like, so I may have missed the mark.

In a few CLs from now, I'd like to replace the current RPC-like set of function
definitions with events.  I want to make sure that events code is in a good
state before that happens.

[Dan Beam, 2014-02-11 23:21:48]

https://codereview.chromium.org/160083002/diff/1/chrome/browser/resources/syn...
File chrome/browser/resources/sync_internals/about.js (right):

https://codereview.chromium.org/160083002/diff/1/chrome/browser/resources/syn...
chrome/browser/resources/sync_internals/about.js:80:
chrome.sync.removeEventListener(
should this be chrome.sync.events.removeEventListener?

https://codereview.chromium.org/160083002/diff/1/chrome/browser/resources/syn...
File chrome/browser/resources/sync_internals/chrome_sync.js (right):

https://codereview.chromium.org/160083002/diff/1/chrome/browser/resources/syn...
chrome/browser/resources/sync_internals/chrome_sync.js:23: // Handle a reply,
assuming that messages are processed in FIFO order.
^ this is an odd assumption

https://codereview.chromium.org/160083002/diff/1/chrome/browser/resources/syn...
chrome/browser/resources/sync_internals/chrome_sync.js:64: var start = new
Date();
var start = Date.now();

https://codereview.chromium.org/160083002/diff/1/chrome/browser/resources/syn...
chrome/browser/resources/sync_internals/chrome_sync.js:72: return ((new
Date()).getTime() - start.getTime()) / 1000.0;
FYI: all numbers in JavaScript are doubles

return (Date.now() - start) / 1000;

https://codereview.chromium.org/160083002/diff/1/chrome/browser/resources/syn...
chrome/browser/resources/sync_internals/chrome_sync.js:79: * @param {dictionary}
details A collection of event-specific details.
  @param {Object} details ...

or

  @param {!Object} details ...

if |details| can't be null

https://codereview.chromium.org/160083002/diff/1/chrome/browser/resources/syn...
chrome/browser/resources/sync_internals/chrome_sync.js:81: var handleEvent =
function(name, details) {
nit: s/handleEvent/dispatchEvent/

https://codereview.chromium.org/160083002/diff/1/chrome/browser/resources/syn...
chrome/browser/resources/sync_internals/chrome_sync.js:85: }
};

https://codereview.chromium.org/160083002/diff/1/chrome/browser/resources/syn...
chrome/browser/resources/sync_internals/chrome_sync.js:93: }
^ where is this actually used?

https://codereview.chromium.org/160083002/diff/1/chrome/browser/resources/syn...
File chrome/browser/resources/sync_internals/sync_log.js (right):

https://codereview.chromium.org/160083002/diff/1/chrome/browser/resources/syn...
chrome/browser/resources/sync_internals/sync_log.js:35: 'onActionableError',
sort

https://codereview.chromium.org/160083002/diff/1/chrome/browser/resources/syn...
chrome/browser/resources/sync_internals/sync_log.js:40: ]
nit: ],

https://codereview.chromium.org/160083002/diff/1/chrome/browser/resources/syn...
chrome/browser/resources/sync_internals/sync_log.js:59: }
};

https://codereview.chromium.org/160083002/diff/1/chrome/browser/resources/syn...
chrome/browser/resources/sync_internals/sync_log.js:63:
eventsByCategory[categoryName].forEach(function(eventName) {
nit: because this doesn't need to make a new scope, it'd be better to use
vanilla for-loop. but it's bounded input... so i don't care that much.

https://codereview.chromium.org/160083002/diff/1/chrome/browser/ui/webui/sync...
File chrome/browser/ui/webui/sync_internals_browsertest.js (right):

https://codereview.chromium.org/160083002/diff/1/chrome/browser/ui/webui/sync...
chrome/browser/ui/webui/sync_internals_browsertest.js:255:
connectionEvent.details = { 'status': 'CONNECTION_OK' }
nit: unfortunately (because it differs from C++), JS prefers:

  connectionEvent.details = {status: 'CONNECTION_OK'};

https://codereview.chromium.org/160083002/diff/1/chrome/browser/ui/webui/sync...
chrome/browser/ui/webui/sync_internals_browsertest.js:269:
assertNotEquals(undefined, submoduleName, firstRow);
nit: don't think you need to explicitly say undefined here, i.e.:

  assertNotEquals(submoduleName, firstRow);

and so on

https://codereview.chromium.org/160083002/diff/1/chrome/browser/ui/webui/sync...
chrome/browser/ui/webui/sync_internals_browsertest.js:274: 'submoduleName=' +
submoduleName);
nit:

expectGE(submoduleName.indexOf('manager'), 0);

[rlarocque, 2014-02-12 00:39:55]

Addressed most comments.  PTAL.

https://codereview.chromium.org/160083002/diff/1/chrome/browser/resources/syn...
File chrome/browser/resources/sync_internals/about.js (right):

https://codereview.chromium.org/160083002/diff/1/chrome/browser/resources/syn...
chrome/browser/resources/sync_internals/about.js:80:
chrome.sync.removeEventListener(
On 2014/02/11 23:21:48, Dan Beam wrote:
> should this be chrome.sync.events.removeEventListener?

Good catch.  Fixed.

I've also added a unit test that would have caught this error.

https://codereview.chromium.org/160083002/diff/1/chrome/browser/resources/syn...
File chrome/browser/resources/sync_internals/chrome_sync.js (right):

https://codereview.chromium.org/160083002/diff/1/chrome/browser/resources/syn...
chrome/browser/resources/sync_internals/chrome_sync.js:23: // Handle a reply,
assuming that messages are processed in FIFO order.
On 2014/02/11 23:21:48, Dan Beam wrote:
> ^ this is an odd assumption

I agree.

Somehow we've been able to get away with this assumption for a long time.  But
I'm taking the sync code in a direction where responses will become less
deterministic.  Which is why I need to refactor the about:sync page to depend
more on events, and less on this async callback interface.

All of the makeSyncFunction()-defined functions will be removed soon.

https://codereview.chromium.org/160083002/diff/1/chrome/browser/resources/syn...
chrome/browser/resources/sync_internals/chrome_sync.js:64: var start = new
Date();
On 2014/02/11 23:21:48, Dan Beam wrote:
> var start = Date.now();

Done.

https://codereview.chromium.org/160083002/diff/1/chrome/browser/resources/syn...
chrome/browser/resources/sync_internals/chrome_sync.js:72: return ((new
Date()).getTime() - start.getTime()) / 1000.0;
On 2014/02/11 23:21:48, Dan Beam wrote:
> FYI: all numbers in JavaScript are doubles
> 
> return (Date.now() - start) / 1000;

Done.

https://codereview.chromium.org/160083002/diff/1/chrome/browser/resources/syn...
chrome/browser/resources/sync_internals/chrome_sync.js:79: * @param {dictionary}
details A collection of event-specific details.
On 2014/02/11 23:21:48, Dan Beam wrote:
>   @param {Object} details ...
> 
> or
> 
>   @param {!Object} details ...
> 
> if |details| can't be null

The C++ code currently guarantees that it will contain at least an empty
dictionary.  I'm not sure if that's better or worse than returning null for that
case.  

Changed to {!Object}.

https://codereview.chromium.org/160083002/diff/1/chrome/browser/resources/syn...
chrome/browser/resources/sync_internals/chrome_sync.js:81: var handleEvent =
function(name, details) {
On 2014/02/11 23:21:48, Dan Beam wrote:
> nit: s/handleEvent/dispatchEvent/

Done.

https://codereview.chromium.org/160083002/diff/1/chrome/browser/resources/syn...
chrome/browser/resources/sync_internals/chrome_sync.js:85: }
On 2014/02/11 23:21:48, Dan Beam wrote:
> };

Done.

https://codereview.chromium.org/160083002/diff/1/chrome/browser/resources/syn...
chrome/browser/resources/sync_internals/chrome_sync.js:93: }
On 2014/02/11 23:21:48, Dan Beam wrote:
> ^ where is this actually used?

That's left over from a bad rebase.  Removed.

https://codereview.chromium.org/160083002/diff/1/chrome/browser/resources/syn...
File chrome/browser/resources/sync_internals/sync_log.js (right):

https://codereview.chromium.org/160083002/diff/1/chrome/browser/resources/syn...
chrome/browser/resources/sync_internals/sync_log.js:35: 'onActionableError',
On 2014/02/11 23:21:48, Dan Beam wrote:
> sort

Done.

https://codereview.chromium.org/160083002/diff/1/chrome/browser/resources/syn...
chrome/browser/resources/sync_internals/sync_log.js:40: ]
On 2014/02/11 23:21:48, Dan Beam wrote:
> nit: ],

Done.

https://codereview.chromium.org/160083002/diff/1/chrome/browser/resources/syn...
chrome/browser/resources/sync_internals/sync_log.js:59: }
On 2014/02/11 23:21:48, Dan Beam wrote:
> };

Done.

https://codereview.chromium.org/160083002/diff/1/chrome/browser/resources/syn...
chrome/browser/resources/sync_internals/sync_log.js:63:
eventsByCategory[categoryName].forEach(function(eventName) {
On 2014/02/11 23:21:48, Dan Beam wrote:
> nit: because this doesn't need to make a new scope, it'd be better to use
> vanilla for-loop. but it's bounded input... so i don't care that much.

Fixed.

https://codereview.chromium.org/160083002/diff/1/chrome/browser/ui/webui/sync...
File chrome/browser/ui/webui/sync_internals_browsertest.js (right):

https://codereview.chromium.org/160083002/diff/1/chrome/browser/ui/webui/sync...
chrome/browser/ui/webui/sync_internals_browsertest.js:255:
connectionEvent.details = { 'status': 'CONNECTION_OK' }
On 2014/02/11 23:21:48, Dan Beam wrote:
> nit: unfortunately (because it differs from C++), JS prefers:
> 
>   connectionEvent.details = {status: 'CONNECTION_OK'};

Done.

https://codereview.chromium.org/160083002/diff/1/chrome/browser/ui/webui/sync...
chrome/browser/ui/webui/sync_internals_browsertest.js:269:
assertNotEquals(undefined, submoduleName, firstRow);
On 2014/02/11 23:21:48, Dan Beam wrote:
> nit: don't think you need to explicitly say undefined here, i.e.:
> 
>   assertNotEquals(submoduleName, firstRow);
> 
> and so on

Wouldn't that end up comparing submoduleName != firstRow?

https://codereview.chromium.org/160083002/diff/1/chrome/browser/ui/webui/sync...
chrome/browser/ui/webui/sync_internals_browsertest.js:274: 'submoduleName=' +
submoduleName);
On 2014/02/11 23:21:48, Dan Beam wrote:
> nit:
> 
> expectGE(submoduleName.indexOf('manager'), 0);

Done.

[Dan Beam, 2014-02-12 00:46:19]

https://codereview.chromium.org/160083002/diff/1/chrome/browser/ui/webui/sync...
File chrome/browser/ui/webui/sync_internals_browsertest.js (right):

https://codereview.chromium.org/160083002/diff/1/chrome/browser/ui/webui/sync...
chrome/browser/ui/webui/sync_internals_browsertest.js:264: assertLE(4,
firstRow.children.length);
shouldn't this be assertGE(), btw?

https://codereview.chromium.org/160083002/diff/1/chrome/browser/ui/webui/sync...
chrome/browser/ui/webui/sync_internals_browsertest.js:269:
assertNotEquals(undefined, submoduleName, firstRow);
On 2014/02/12 00:39:55, rlarocque wrote:
> On 2014/02/11 23:21:48, Dan Beam wrote:
> > nit: don't think you need to explicitly say undefined here, i.e.:
> > 
> >   assertNotEquals(submoduleName, firstRow);
> > 
> > and so on
> 
> Wouldn't that end up comparing submoduleName != firstRow?

ah, I see.  these checks will never fail AFAIK as Element.textContent will
return '' in all cases if there's no text.  if you want to check something
perhaps something like this:

  assertGE(submoduleName.length, 0);

[rlarocque, 2014-02-12 01:00:45]

Patch updated.

https://codereview.chromium.org/160083002/diff/1/chrome/browser/ui/webui/sync...
File chrome/browser/ui/webui/sync_internals_browsertest.js (right):

https://codereview.chromium.org/160083002/diff/1/chrome/browser/ui/webui/sync...
chrome/browser/ui/webui/sync_internals_browsertest.js:264: assertLE(4,
firstRow.children.length);
On 2014/02/12 00:46:19, Dan Beam wrote:
> shouldn't this be assertGE(), btw?

Let's just go with assertEquals().  If someone adds an extra column they'd
probably need to update this test anyway.

https://codereview.chromium.org/160083002/diff/1/chrome/browser/ui/webui/sync...
chrome/browser/ui/webui/sync_internals_browsertest.js:269:
assertNotEquals(undefined, submoduleName, firstRow);
On 2014/02/12 00:46:19, Dan Beam wrote:
> On 2014/02/12 00:39:55, rlarocque wrote:
> > On 2014/02/11 23:21:48, Dan Beam wrote:
> > > nit: don't think you need to explicitly say undefined here, i.e.:
> > > 
> > >   assertNotEquals(submoduleName, firstRow);
> > > 
> > > and so on
> > 
> > Wouldn't that end up comparing submoduleName != firstRow?
> 
> ah, I see.  these checks will never fail AFAIK as Element.textContent will
> return '' in all cases if there's no text.  if you want to check something
> perhaps something like this:
> 
>   assertGE(submoduleName.length, 0);

I think the idea here was to make sure that the DOM elements I'm looking for
actually exist before looking inside them.  It's a pattern I use often in C++
tests.  In this case, it's both unnecessary and wrongly implemented.

Maybe the test will be better off without these checks.  We'll just let it fail
on lines 273-278.

[Dan Beam, 2014-02-12 01:24:59]

lgtm

https://codereview.chromium.org/160083002/diff/210001/chrome/browser/resource...
File chrome/browser/resources/sync_internals/chrome_sync.js (right):

https://codereview.chromium.org/160083002/diff/210001/chrome/browser/resource...
chrome/browser/resources/sync_internals/chrome_sync.js:71: get elapsedSeconds()
{
nit: getters should be cheap, similar to unix_hacker() accessors in C++.  this
feels more like a GetElapsedSeconds() in C++, as it's actually querying the time
(which probably queries some system time), so it probably ought to be a function
instead of an ES5 getter.  Here's how I would write it:

  /**
   * A simple timer to measure elapsed time.
   * @constructor
   */
  function Timer() {
    /**
     * The time that this Timer was created.
     * @type {number}
     * @private
     * @const
     */
    this.start_ = Date.now();
  }

  /**
   * @return {number} The elapsed seconds since this Timer was 
   *     created.
   */
  Timer.prototype.getElapsedSeconds = function() {
    return (Date.now() - this.start_) / 1000;
  };

  /** @return {!Timer} An object which measures elapsed time. */
  chrome.sync.makeTimer = function() {
    return new Timer;
  };

however returning an internal type may not be totally useful, but whatevs.

the minimal change would be:

  s/get elapsedSeconds/getElapsedSeconds: function/

and change the callsites to include calling parens().

https://codereview.chromium.org/160083002/diff/210001/chrome/browser/resource...
File chrome/browser/resources/sync_internals/sync_log.js (right):

https://codereview.chromium.org/160083002/diff/210001/chrome/browser/resource...
chrome/browser/resources/sync_internals/sync_log.js:60: }
};

https://codereview.chromium.org/160083002/diff/210001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/sync_internals_browsertest.js (right):

https://codereview.chromium.org/160083002/diff/210001/chrome/browser/ui/webui...
chrome/browser/ui/webui/sync_internals_browsertest.js:167: HARD_CODED_ABOUT_INFO
= {
nit:

/** @const */
var HARD_CODED_ABOUT_INFO = {

https://codereview.chromium.org/160083002/diff/210001/chrome/browser/ui/webui...
chrome/browser/ui/webui/sync_internals_browsertest.js:349:
connectionEvent.details = { 'status': 'CONNECTION_OK' }
remove \s between { and } here, too

https://codereview.chromium.org/160083002/diff/210001/chrome/browser/ui/webui...
chrome/browser/ui/webui/sync_internals_browsertest.js:359:
expectTrue(eventDumpText.indexOf('CONNECTION_OK') != -1);
^ update these as well, IMO, to use expectNE

[rlarocque, 2014-02-12 02:07:38]

Thanks for the quick and thorough reviews.  I learned a lot from your comments.

I've updated the patch.  I'll send it to the trybots tonight and commit tomorrow
if the results come back green.

https://codereview.chromium.org/160083002/diff/210001/chrome/browser/resource...
File chrome/browser/resources/sync_internals/chrome_sync.js (right):

https://codereview.chromium.org/160083002/diff/210001/chrome/browser/resource...
chrome/browser/resources/sync_internals/chrome_sync.js:71: get elapsedSeconds()
{
On 2014/02/12 01:25:00, Dan Beam wrote:
> nit: getters should be cheap, similar to unix_hacker() accessors in C++.  this
> feels more like a GetElapsedSeconds() in C++, as it's actually querying the
time
> (which probably queries some system time), so it probably ought to be a
function
> instead of an ES5 getter.  Here's how I would write it:
> 
>   /**
>    * A simple timer to measure elapsed time.
>    * @constructor
>    */
>   function Timer() {
>     /**
>      * The time that this Timer was created.
>      * @type {number}
>      * @private
>      * @const
>      */
>     this.start_ = Date.now();
>   }
> 
>   /**
>    * @return {number} The elapsed seconds since this Timer was 
>    *     created.
>    */
>   Timer.prototype.getElapsedSeconds = function() {
>     return (Date.now() - this.start_) / 1000;
>   };
> 
>   /** @return {!Timer} An object which measures elapsed time. */
>   chrome.sync.makeTimer = function() {
>     return new Timer;
>   };
> 
> however returning an internal type may not be totally useful, but whatevs.
> 
> the minimal change would be:
> 
>   s/get elapsedSeconds/getElapsedSeconds: function/
> 
> and change the callsites to include calling parens().

Thanks!

I didn't have any strong feelings about the old implementation.  This one looks
better.  I'll take it.

https://codereview.chromium.org/160083002/diff/210001/chrome/browser/resource...
File chrome/browser/resources/sync_internals/sync_log.js (right):

https://codereview.chromium.org/160083002/diff/210001/chrome/browser/resource...
chrome/browser/resources/sync_internals/sync_log.js:60: }
On 2014/02/12 01:25:00, Dan Beam wrote:
> };

Done.

https://codereview.chromium.org/160083002/diff/210001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/sync_internals_browsertest.js (right):

https://codereview.chromium.org/160083002/diff/210001/chrome/browser/ui/webui...
chrome/browser/ui/webui/sync_internals_browsertest.js:167: HARD_CODED_ABOUT_INFO
= {
On 2014/02/12 01:25:00, Dan Beam wrote:
> nit:
> 
> /** @const */
> var HARD_CODED_ABOUT_INFO = {

Done.

https://codereview.chromium.org/160083002/diff/210001/chrome/browser/ui/webui...
chrome/browser/ui/webui/sync_internals_browsertest.js:349:
connectionEvent.details = { 'status': 'CONNECTION_OK' }
On 2014/02/12 01:25:00, Dan Beam wrote:
> remove \s between { and } here, too

Done.

https://codereview.chromium.org/160083002/diff/210001/chrome/browser/ui/webui...
chrome/browser/ui/webui/sync_internals_browsertest.js:359:
expectTrue(eventDumpText.indexOf('CONNECTION_OK') != -1);
On 2014/02/12 01:25:00, Dan Beam wrote:
> ^ update these as well, IMO, to use expectNE

I went with GE 0, since that's what I ended up using on lines 338-343.

[Dan Beam, 2014-02-12 03:27:32]

slgtm, just FYI

https://codereview.chromium.org/160083002/diff/280001/chrome/browser/resource...
File chrome/browser/resources/sync_internals/chrome_sync.js (right):

https://codereview.chromium.org/160083002/diff/280001/chrome/browser/resource...
chrome/browser/resources/sync_internals/chrome_sync.js:79: }
nit: whenever you say

  var blah = function() {};

you're using a function expression.  this is similar to:

  var blah = 1;

if you write this as:

  var blah = 1

in most cases it'll work.  however, it relies on some pretty wonky rules.  for
instance:

  return {
    makeTimer: makeTimer,
  };

works as you'd expect whereas:

  return
  {
    makeTimer: makeTimer,
  };

will throw no errors but silently return undefined as it's interpreted more as:

  return;
  // useless code ignored

additionally, a function specific bug I've seen before:

  var a = function(arg) {
    return {doSomething: function(){ alert(1); }};
  }
  (new Class).doSomething();

will immediately invoke the function with (new Class) as |arg| and then call
#doSomething on the result.

function *declarations* like |Timer| don't need a ; because they're slightly
different than function *expressions*.

tl;dr - please add a ; whenever doing "= function(){...};"

https://codereview.chromium.org/160083002/diff/280001/chrome/browser/resource...
chrome/browser/resources/sync_internals/chrome_sync.js:84: }
^ and here

[rlarocque, 2014-02-12 18:32:06]

On 2014/02/12 03:27:32, Dan Beam wrote:
> slgtm, just FYI
> 
>
https://codereview.chromium.org/160083002/diff/280001/chrome/browser/resource...
> File chrome/browser/resources/sync_internals/chrome_sync.js (right):
> 
>
https://codereview.chromium.org/160083002/diff/280001/chrome/browser/resource...
> chrome/browser/resources/sync_internals/chrome_sync.js:79: }
> nit: whenever you say
> 
>   var blah = function() {};
> 
> you're using a function expression.  this is similar to:
> 
>   var blah = 1;
> 
> if you write this as:
> 
>   var blah = 1
> 
> in most cases it'll work.  however, it relies on some pretty wonky rules.  for
> instance:
> 
>   return {
>     makeTimer: makeTimer,
>   };
> 
> works as you'd expect whereas:
> 
>   return
>   {
>     makeTimer: makeTimer,
>   };
> 
> will throw no errors but silently return undefined as it's interpreted more
as:
> 
>   return;
>   // useless code ignored
> 
> additionally, a function specific bug I've seen before:
> 
>   var a = function(arg) {
>     return {doSomething: function(){ alert(1); }};
>   }
>   (new Class).doSomething();
> 
> will immediately invoke the function with (new Class) as |arg| and then call
> #doSomething on the result.
> 
> function *declarations* like |Timer| don't need a ; because they're slightly
> different than function *expressions*.
> 
> tl;dr - please add a ; whenever doing "= function(){...};"
> 
>
https://codereview.chromium.org/160083002/diff/280001/chrome/browser/resource...
> chrome/browser/resources/sync_internals/chrome_sync.js:84: }
> ^ and here

Thanks and done.

[rlarocque, 2014-02-12 18:33:10]

The CQ bit was checked by rlarocque@chromium.org

[commit-bot: I haz the power, 2014-02-12 18:35:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rlarocque@chromium.org/160083002/240002

[commit-bot: I haz the power, 2014-02-12 19:07:16]

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

[commit-bot: I haz the power, 2014-02-12 19:07:17]

Retried try job too often on win_rel for step(s) base_unittests, browser_tests,
interactive_ui_tests, net_unittests, unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[rlarocque, 2014-02-12 19:23:16]

The CQ bit was checked by rlarocque@chromium.org

[commit-bot: I haz the power, 2014-02-12 19:26:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rlarocque@chromium.org/160083002/240002

[commit-bot: I haz the power, 2014-02-12 23:24:53]

Change committed as 250835