Merge pull request #20007 from wilsonpage/1016492

Bug 1016492 - [B2G][Flame][Camera] - Working animation / spinner missing

apps/camera/js/app.js

@@ -59,8 +59,10 @@ function App(options) {
 }
 
 /**
- * Runs all the methods
- * to boot the app.
+ * Runs all the methods to boot the app.
+ *
+ * The loading screen is shown until the
+ * camera is 'ready', then it is taken down.
  *
  * @public
  */
@@ -71,8 +73,8 @@ App.prototype.boot = function() {
   this.initializeViews();
   this.runControllers();
   this.injectViews();
-  this.booted = true;
   this.showLoading();
+  this.booted = true;
   debug('booted');
 };
 
@@ -103,15 +105,7 @@ App.prototype.runControllers = function() {
  * @param  {String} path
  */
 App.prototype.loadController = function(path) {
-  var self = this;
-  this.require([path, 'lib/string-utils'],
-    function(controller, StringUtils) {
-      var name = StringUtils.toCamelCase(
-        StringUtils.lastPathComponent(path));
-
-      self.controllers[name] = controller(self);
-    }
-  );
+  this.require([path], function(controller) { controller(this); }.bind(this));
 };
 
 /**
@@ -153,6 +147,8 @@ App.prototype.bindEvents = function() {
   // App
   this.once('viewfinder:visible', this.onCriticalPathDone);
   this.once('storage:checked:healthy', this.geolocationWatch);
+  this.on('camera:takingpicture', this.showLoading);
+  this.on('camera:ready', this.clearLoading);
   this.on('visible', this.onVisible);
   this.on('hidden', this.onHidden);
 
@@ -218,10 +214,11 @@ App.prototype.onCriticalPathDone = function() {
   var start = window.performance.timing.domLoading;
   var took = Date.now() - start;
 
+  // Indicate critical path is done to help track performance
   PerformanceTestingHelper.dispatch('startup-path-done');
   console.log('critical-path took %s', took + 'ms');
 
-  this.clearLoading();
+  // Load non-critical modules
   this.loadController(this.controllers.previewGallery);
   this.loadController(this.controllers.storage);
   this.loadController(this.controllers.confirm);
@@ -348,6 +345,7 @@ App.prototype.clearLoading = function() {
   clearTimeout(this.loadingTimeout);
   if (!view) { return; }
   view.hide(view.destroy);
+  this.views.loading = null;
 };
 
 });

apps/camera/js/controllers/camera.js

@@ -48,6 +48,7 @@ CameraController.prototype.bindEvents = function() {
   camera.on('change:focus', app.firer('camera:focusstatechanged'));
   camera.on('facesdetected', app.firer('camera:facesdetected'));
   camera.on('filesizelimitreached', this.onFileSizeLimitReached);
+  camera.on('takingpicture', app.firer('camera:takingpicture'));
   camera.on('change:recording', app.setter('recording'));
   camera.on('newcamera', app.firer('camera:newcamera'));
   camera.on('newimage', app.firer('camera:newimage'));
@@ -124,7 +125,7 @@ CameraController.prototype.onSettingsConfigured = function() {
   hardware.onsuccess = function(evt) {
     var device = evt.target.result['deviceinfo.hardware'];
     if (device === 'mako') {
-      
+
       // Nexus 4 needs zoom preview adjustment since the viewfinder preview
       // stream does not automatically reflect the current zoom value.
       self.settings.zoom.set('useZoomPreviewAdjustment', true);

apps/camera/js/lib/camera/camera.js

@@ -748,37 +748,41 @@ Camera.prototype.takePicture = function(options) {
     fileFormat: 'jpeg'
   };
 
-  // If position has been
-  // passed in, add it to
-  // the config object.
+  // If position has been passed in,
+  // add it to the config object.
   if (position) {
     config.position = position;
   }
 
+  // Front camera is inverted, so flip rotation
   rotation = selectedCamera === 'front' ? -rotation : rotation;
-  debug('take picture');
-  this.emit('busy');
+
+  // If the camera focus is 'continuous' or 'infinity'
+  // we can take the picture straight away.
   if (this.focus.getMode() === 'auto') {
     this.set('focus', 'focusing');
     this.focus.focus(onFocused);
   } else {
-    self.mozCamera.takePicture(config, onSuccess, onError);
+    takePicture();
   }
 
   function onFocused(err) {
-    if (err) {
-      self.set('focus', 'fail');
-    } else {
-      self.set('focus', 'focused');
-    }
+    var focus = err ? 'fail' : 'focused';
+    self.set('focus', focus);
+    takePicture();
+  }
+
+  function takePicture() {
+    self.emit('takingpicture');
     self.mozCamera.takePicture(config, onSuccess, onError);
   }
 
   function onError(error) {
     var title = navigator.mozL10n.get('error-saving-title');
     var text = navigator.mozL10n.get('error-saving-text');
-    // if taking a picture fails because there's already
-    // a picture being taken we ignore it
+
+    // if taking a picture fails because there's
+    // already a picture being taken we ignore it.
     if (error === 'TakePictureAlreadyInProgress') {
       complete();
     } else {
@@ -798,9 +802,13 @@ Camera.prototype.takePicture = function(options) {
   }
 
   function complete() {
-    // If we are in C-AF mode, we have to call resume() in
-    // order to get the camera to resume focusing on what we point it at.
+
+    // If we are in C-AF mode, we have
+    // to call resume() in order to get
+    // the camera to resume focusing
+    // on what we point it at.
     self.focus.resume();
+
     self.set('focus', 'none');
     self.ready();
   }

apps/camera/js/main.js

@@ -1,10 +1,6 @@
 define(function(require) {
 'use strict';
 
-var timing = window.performance.timing;
-var domLoaded = timing.domComplete - timing.domLoading;
-console.log('domloaded in %s', domLoaded + 'ms');
-
 /**
  * Module Dependencies
  */
@@ -16,18 +12,19 @@ var settings = new Settings(settingsData);
 var Camera = require('lib/camera/camera');
 var App = require('app');
 
-/**
-  * Create globals specified in the config file
-  */
+// Log dom-loaded to keep perf on our radar
+var timing = window.performance.timing;
+var domLoaded = timing.domComplete - timing.domLoading;
+console.log('domloaded in %s', domLoaded + 'ms');
+
+// Create globals specified in the config file
 if (settingsData.globals) {
   for (var key in settingsData.globals) {
     window[key] = settingsData.globals[key];
   }
 }
 
-/**
- * Create new `App`
- */
+// Create new `App`
 var app = window.app = new App({
   settings: settings,
   geolocation: new GeoLocation(),
@@ -62,8 +59,9 @@ var app = window.app = new App({
   }
 });
 
-// Fetch persistent settings,
-// Check for activities, then boot
+// We start the camera loading straight
+// away (async), as this is the slowest
+// part of the boot process.
 app.camera.load();
 app.settings.fetch();
 app.boot();

apps/camera/style/loading-screen.css

@@ -4,7 +4,9 @@
   justify-content: center;
   align-items: center;
   position: absolute;
-  left: 0; top: 0;
+  left: 0;
+  top: 0;
+  z-index: 1;
   width: 100%;
   height: 100%;
   background: rgba(0,0,0,0.7);

apps/camera/test/unit/app_test.js

@@ -111,7 +111,14 @@ suite('app', function() {
 
     // More complex stubs
     options.activity.check.callsArg(0);
+
+    // Sometimes we have to spy on the prototype,
+    // this is because methods get bound and passed
+    // directly as callbacks. We set spys on prototype
+    // methods before any of this happens, so that the
+    // spy is always at the root of any call.
     this.sandbox.spy(this.App.prototype, 'boot');
+    this.sandbox.spy(this.App.prototype, 'clearLoading');
 
     // Aliases
     this.settings = options.settings;
@@ -124,6 +131,7 @@ suite('app', function() {
     this.sandbox.spy(this.app, 'once');
     this.sandbox.spy(this.app, 'emit');
     this.sandbox.spy(this.app, 'firer');
+    this.sandbox.spy(this.app, 'showLoading');
   });
 
   teardown(function() {
@@ -209,6 +217,20 @@ suite('app', function() {
       assert.ok(this.app.once.calledWith('storage:checked:healthy', geolocationWatch));
     });
 
+    test('Should show loading screen', function() {
+      sinon.assert.calledOnce(this.app.showLoading);
+    });
+
+    test('Should clear loading screen when camera is ready', function() {
+      var on = this.app.on.withArgs('camera:ready');
+      var callback = on.args[0][1];
+
+      // Call the callback and make sure
+      // that `clearLoading` was called.
+      callback();
+      sinon.assert.calledOnce(this.App.prototype.clearLoading);
+    });
+
     suite('App#geolocationWatch()', function() {
       test('Should *not* watch location if in activity', function() {
         this.app.hidden = false;
@@ -270,10 +292,6 @@ suite('app', function() {
         assert.isTrue(loadController.calledWith(controllers.battery));
         assert.isTrue(loadController.calledWith(controllers.sounds));
       });
-
-      test('Should clear loading screen', function() {
-        sinon.assert.called(this.app.clearLoading);
-      });
     });
   });
 
@@ -387,5 +405,10 @@ suite('app', function() {
       sinon.assert.called(view.hide);
       assert.ok(view.destroy.calledAfter(view.hide));
     });
+
+    test('Should clear reference to `app.views.loading`', function() {
+      this.app.clearLoading();
+      assert.equal(this.app.views.loading, null);
+    });
   });
 });

apps/camera/test/unit/lib/camera/camera_test.js

@@ -510,6 +510,12 @@ suite('lib/camera/camera', function() {
       assert.isTrue(this.camera.mozCamera.takePicture.called);
     });
 
+    test('Should emit a `takingpicture` event', function() {
+      sinon.stub(this.camera, 'emit');
+      this.camera.takePicture({});
+      sinon.assert.calledWith(this.camera.emit, 'takingpicture');
+    });
+
     test('Should still take picture even when focus fails', function() {
       this.camera.focus.focus = sinon.stub().callsArgWith(0, 'some error');
       this.camera.takePicture({});