Skip to content

Commit

Permalink
Bug 1194547 - Clean up app screenshoting a bit. r=albertopq
Browse files Browse the repository at this point in the history
* don't show the screenshot overlay when |ready| is called
* make sure methods than return promises _always_ return promises
* make sure we screenshot correctly when exiting an app displaying the keyboard
* don't do useless screenshot work for AttentionWindows
  • Loading branch information
etiennesegonzac committed Aug 27, 2015
1 parent 628a4c4 commit a5463e0
Show file tree
Hide file tree
Showing 6 changed files with 17 additions and 28 deletions.
17 changes: 10 additions & 7 deletions apps/system/js/app_window.js
Original file line number Diff line number Diff line change
Expand Up @@ -1398,6 +1398,12 @@
// will be null if there is no blob
var screenshotURL = this.requestScreenshotURL();

if (!screenshotURL) {
this.element.classList.add('no-screenshot');
this.screenshotOverlay.style.backgroundImage = 'none';
return Promise.resolve();
}

// return promise to make sure the image is ready
var promise = new Promise((resolve) => {
var image = document.createElement('img');
Expand All @@ -1406,9 +1412,9 @@
}.bind(this);
image.src = screenshotURL;
});
this.screenshotOverlay.style.backgroundImage = screenshotURL ?
'url(' + screenshotURL + ')' : 'none';
this.element.classList.toggle('no-screenshot', !screenshotURL);
this.screenshotOverlay.style.backgroundImage =
'url(' + screenshotURL + ')';
this.element.classList.remove('no-screenshot');
return promise;
};

Expand Down Expand Up @@ -1689,7 +1695,7 @@
this.debug('request RESIZE...active? ', this.isActive());
var bottom = this.getBottomMostWindow();
if (!bottom.shouldResize() || this.isTransitioning()) {
return;
return Promise.resolve();
}
if (this.frontWindow) {
return Promise.all(
Expand Down Expand Up @@ -1985,9 +1991,6 @@
if (!this.element) {
return;
}
if (this._screenshotBlob) {
this._showScreenshotOverlay();
}

this.debug('requesting to open');

Expand Down
7 changes: 4 additions & 3 deletions apps/system/js/app_window_manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -319,14 +319,12 @@
var switching = appCurrent && !appCurrent.isHomescreen &&
!appNext.isHomescreen;

this._updateActiveApp(appNext.instanceID);

var that = this;
if (appCurrent && this.service.query('keyboardEnabled')) {
this.stopRecording();

// Ask keyboard to hide before we switch the app.
window.addEventListener('keyboardhidden', function onhiddenkeyboard() {
window.addEventListener('keyboardhidden', function onhiddenkeyboard() {
window.removeEventListener('keyboardhidden', onhiddenkeyboard);
that.switchApp(appCurrent, appNext, switching);
});
Expand Down Expand Up @@ -363,7 +361,10 @@
*/
switchApp: function awm_switchApp(appCurrent, appNext, switching,
openAnimation, closeAnimation) {

this.debug('before ready check' + appCurrent + appNext);
this._updateActiveApp(appNext.instanceID);

appNext.ready(function() {
if (appNext.isDead()) {
if (!appNext.isHomescreen) {
Expand Down
2 changes: 0 additions & 2 deletions apps/system/js/attention_window.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@
this.debug('intance id: ' + this.instanceID);
return `<div class="${this.CLASS_LIST}" id="${this.instanceID}">
<div class="browser-container"></div>
<div class="screenshot-overlay"></div>
</div>`;
};

Expand Down Expand Up @@ -122,7 +121,6 @@
this.browserContainer.insertBefore(this.browser.element, null);
this.frame = this.element;
this.iframe = this.browser.element;
this.screenshotOverlay = this.element.querySelector('.screenshot-overlay');

this._registerEvents();
this.installSubComponents();
Expand Down
1 change: 0 additions & 1 deletion apps/system/js/callscreen_window.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,6 @@
this.browserContainer.insertBefore(this.browser.element, null);
this.frame = this.element;
this.iframe = this.browser.element;
this.screenshotOverlay = this.element.querySelector('.screenshot-overlay');

this._registerEvents();
this.installSubComponents();
Expand Down
6 changes: 3 additions & 3 deletions apps/system/test/unit/app_window_manager_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -789,8 +789,6 @@ suite('system/AppWindowManager', function() {
});

test('app to app', function() {
var stub_updateActiveApp = this.sinon.stub(subject,
'_updateActiveApp');
injectRunningApps(app1, app2);
subject._activeApp = app1;
var stubSwitchApp = this.sinon.stub(subject, 'switchApp');
Expand All @@ -802,7 +800,6 @@ suite('system/AppWindowManager', function() {
assert.isTrue(stubSwitchApp.called);
assert.deepEqual(stubSwitchApp.getCall(0).args[0], app1);
assert.deepEqual(stubSwitchApp.getCall(0).args[1], app2);
assert.isTrue(stub_updateActiveApp.called);
});

test('Continunous app open requests', function() {
Expand Down Expand Up @@ -927,12 +924,15 @@ suite('system/AppWindowManager', function() {
var stubReady = this.sinon.stub(app2, 'ready');
var stubAppNextOpen = this.sinon.stub(app2, 'open');
var stubAppCurrentClose = this.sinon.stub(app1, 'close');
var stub_updateActiveApp = this.sinon.stub(subject,
'_updateActiveApp');
subject.switchApp(app1, app2, true);
stubReady.yield();
assert.isTrue(stubAppNextOpen.called);
assert.isTrue(stubAppCurrentClose.called);
assert.isTrue(stubAppNextOpen.calledWith('invoked'));
assert.isTrue(stubAppCurrentClose.calledWith('invoking'));
assert.isTrue(stub_updateActiveApp.called);
});

test('close app to cardsview', function() {
Expand Down
12 changes: 0 additions & 12 deletions apps/system/test/unit/app_window_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1135,18 +1135,6 @@ suite('system/AppWindow', function() {
this.sinon.clock.tick(0);
assert.isTrue(callback.calledOnce);
});

test('Call _showScreenshotOverlay', function() {
app1._screenshotBlob = 'fakeBlob';
app1.ready();
assert.isTrue(showScreenshotOverlay.calledOnce);
});

test('Do not call _showScreenshotOverlay', function() {
app1._screenshotBlob = null;
app1.ready();
assert.isFalse(showScreenshotOverlay.called);
});
});

suite('Browser Mixin', function() {
Expand Down

0 comments on commit a5463e0

Please sign in to comment.