Added usefull utility methods to Screen and ScreenContext.

chrome/browser/resources/chromeos/login/screen_context.js

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 /**
  * @fileoverview Implementation of ScreenContext class: key-value storage for
  * values that are shared between C++ and JS sides.
  */
 cr.define('login', function() {
+  'use strict';
+
   function require(condition, message) {
     if (!condition) {
       throw Error(message);
     }
   }
 
   function checkKeyIsValid(key) {
     var keyType = typeof key;
     require(keyType === 'string', 'Invalid type of key: "' + keyType + '".');
   }
 
-  function checkValueIsValid(value) {
+  function checkValueIsValid(value, t) {

[ygorshenin1, 2014/06/11 10:44:00]

nit: from the function definition and usage it's not obvious what's the purpose
of the |t| argument. Could you please add a comment to this method?

[Denis Kuznetsov (DE-MUC), 2014/06/11 14:29:17]

Also, guide recommends to avoid abbreviations for parameter names.

[dzhioev (left Google), 2014/06/18 14:43:39]

It is a garbage that I forgot to remove.

     var valueType = typeof value;
     require(['string', 'boolean', 'number'].indexOf(valueType) != -1,
-        'Invalid type of value: "' + valueType + '".');
+        'Invalid type of value ' + t + ': "' + valueType + '".');
   }
 
-  function ScreenContext() {
+  function ScreenContext(screen) {
+    this.screen_ = screen;

[ygorshenin1, 2014/06/11 10:44:00]

Cound you please point where |screen_| is used?

[dzhioev (left Google), 2014/06/18 14:43:39]

It is not used anymore. I forgot to remove it.

     this.storage_ = {};
     this.changes_ = {};
+    this.observers_ = {};
   }
 
   ScreenContext.prototype = {
     /**
      * Returns stored value for |key| or |defaultValue| if key not found in
      * storage. Throws Error if key not found and |defaultValue| omitted.
      */
     get: function(key, defaultValue) {
       checkKeyIsValid(key);
       if (this.hasKey(key)) {
         return this.storage_[key];
       } else if (typeof defaultValue !== 'undefined') {
         return defaultValue;
       } else {
         throw Error('Key "' + key + '" not found.');
       }
     },
 
     /**
      * Sets |value| for |key|. Returns true if call changes state of context,
      * false otherwise.
      */
     set: function(key, value) {
       checkKeyIsValid(key);
-      checkValueIsValid(value);
+      checkValueIsValid(value, 2);

[Denis Kuznetsov (DE-MUC), 2014/06/11 14:29:17]

It is not obvious what "2" means.
Could you use some named constant instead?

[dzhioev (left Google), 2014/06/18 14:43:39]

Removed.

       if (this.hasKey(key) && this.storage_[key] === value)
         return false;
       this.changes_[key] = value;
       this.storage_[key] = value;
       return true;
     },
 
     hasKey: function(key) {
       checkKeyIsValid(key);
       return this.storage_.hasOwnProperty(key);
     },
 
     hasChanges: function() {
       return Object.keys(this.changes_).length > 0;
     },
 
     /**
      * Applies |changes| to context. Returns Array of changed keys' names.
      */
     applyChanges: function(changes) {
       require(!this.hasChanges(), 'Context has changes.');
-      Object.keys(changes).forEach(function(key) {
+      var oldValues = {};
+      for (var key in changes) {
         checkKeyIsValid(key);
-        checkValueIsValid(changes[key]);
+        checkValueIsValid(changes[key], 1);

[Denis Kuznetsov (DE-MUC), 2014/06/11 14:29:17]

What is the "1"?

[dzhioev (left Google), 2014/06/18 14:43:39]

Removed.

+        oldValues[key] = this.storage_[key];
         this.storage_[key] = changes[key];
-      }, this);
+      }
+      var observers = this.cloneObservers_();
+      for (var key in changes) {
+        if (observers.hasOwnProperty(key)) {
+          var keyObservers = observers[key];
+          for (var observerIndex in keyObservers)
+            keyObservers[observerIndex](changes[key], oldValues[key], key);
+        }
+      }
       return Object.keys(changes);
     },
 
     /**
      * Returns changes made on context since previous call.
      */
     getChangesAndReset: function() {
       var result = this.changes_;
       this.changes_ = {};
       return result;
-    }
+    },
+
+    observe: function(key, observer) {

[Denis Kuznetsov (DE-MUC), 2014/06/11 14:29:17]

subscribe/unsubscribe?
"Observe" is the local action.

[dzhioev (left Google), 2014/06/18 14:43:39]

I used same wording as new JS API use (
http://www.html5rocks.com/en/tutorials/es7/observe ). I can rename to
subscribe/unsubscribe if you insist.

[Denis Kuznetsov (DE-MUC), 2014/06/19 19:16:36]

Then it's even more relevant : we need to avoid name clashing with built-in
names, as it can lead to unpredictable errors somewhere in the future. 

[dzhioev (left Google), 2014/06/20 07:24:16]

OK, renamed to familiar (add/remove)Observer.

+      if (!this.observers_.hasOwnProperty(key))
+        this.observers_[key] = [];
+      if (this.observers_[key].indexOf(observer) !== -1) {
+        console.warn('Observer already registered.');
+        return;
+      }
+      this.observers_[key].push(observer);
+    },
+
+    unobserve: function(observer) {
+      for (var key in this.observers_) {
+        var observerIndex = this.observers_[key].indexOf(observer);
+        if (observerIndex != -1)
+          this.observers_[key].splice(observerIndex, 1);
+      }
+    },
+
+    cloneObservers_: function() {
+      var result = {};
+      for (var key in this.observers_)
+        result[key] = this.observers_[key].slice();
+      return result;
+    },
+

[ygorshenin1, 2014/06/11 10:44:00]

nit: drop empty line and trailing comma.

[dzhioev (left Google), 2014/06/18 14:43:39]

Done.

   };
 
   return {
     ScreenContext: ScreenContext
   };
 });