[Extensions Page] Fix the flicker of "No extensions" before extension data loads

[Extensions Page] Fix the flicker of "No extensions" before extension data loads

There was a flicker caused because we remove the 'loading'
class from the page before the extensions list finishes
updating itself (asynchronously).

BUG=469053
BUG=469332
Committed: https://crrev.com/5a08803841e50d800538bc4e8da2373c817ab114
Cr-Commit-Position: refs/heads/master@{#321869}

[Devlin, 2015-03-20 17:01:21]

Patchset #1 (id:1) has been deleted

[Devlin, 2015-03-20 17:01:59]

rdevlin.cronin@chromium.org changed reviewers:
+ dbeam@chromium.org

[Devlin, 2015-03-20 17:02:58]

Dan, mind taking a look?  There are, of course, a bunch of other ways we could
fix this - but I kinda like this one since it gets rid of the
"ExtensionList.prototype.data_ = extensionsData;" privacy violation, and makes
the update a bit more clear.  WDYT?

[Dan Beam, 2015-03-20 18:18:23]

i think by far the simplest way would just to be:

  <div id=no-extensions hidden></div>
  <div id=extensions-list hidden></div>

  function gotExtensions(list) {
    $('extensions-list').hidden = !list.length;
    $('no-extensions').hidden = list.length;
  }

[Dan Beam, 2015-03-20 18:19:27]

i think that'd be a bit easier to understand than the current css hackery

[Devlin, 2015-03-20 19:49:43]

On 2015/03/20 18:19:27, Dan Beam wrote:
> i think that'd be a bit easier to understand than the current css hackery

Done.

[Dan Beam, 2015-03-20 23:13:27]

https://codereview.chromium.org/1025703003/diff/40001/chrome/browser/resource...
File chrome/browser/resources/extensions/extensions.css (left):

https://codereview.chromium.org/1025703003/diff/40001/chrome/browser/resource...
chrome/browser/resources/extensions/extensions.css:6: transition-duration: 0ms
!important;
might want to keep this

https://codereview.chromium.org/1025703003/diff/40001/chrome/browser/resource...
File chrome/browser/resources/extensions/extensions.html (right):

https://codereview.chromium.org/1025703003/diff/40001/chrome/browser/resource...
chrome/browser/resources/extensions/extensions.html:125: <div
id="footer-section" hidden>
can you put #footer-section and #extension-settings-list in a wrapper and just
hide that wrapper?

[Devlin, 2015-03-21 01:07:19]

https://codereview.chromium.org/1025703003/diff/40001/chrome/browser/resource...
File chrome/browser/resources/extensions/extensions.css (left):

https://codereview.chromium.org/1025703003/diff/40001/chrome/browser/resource...
chrome/browser/resources/extensions/extensions.css:6: transition-duration: 0ms
!important;
On 2015/03/20 23:13:27, Dan Beam wrote:
> might want to keep this

Was thinking about it, but if we keep it, then we have to also keep the
'loading' class stuff in extensions.js, which I thought was one of the main
targets of the hackery. :)  I've tried it locally, and it *seems* to look okay. 
If we'd rather err on the side of caution, and keep this (and the loading
stuff), then we certainly can - WDYT?

https://codereview.chromium.org/1025703003/diff/40001/chrome/browser/resource...
File chrome/browser/resources/extensions/extensions.html (right):

https://codereview.chromium.org/1025703003/diff/40001/chrome/browser/resource...
chrome/browser/resources/extensions/extensions.html:125: <div
id="footer-section" hidden>
On 2015/03/20 23:13:27, Dan Beam wrote:
> can you put #footer-section and #extension-settings-list in a wrapper and just
> hide that wrapper?

Yep, done.

[Dan Beam, 2015-03-21 01:14:10]

https://codereview.chromium.org/1025703003/diff/40001/chrome/browser/resource...
File chrome/browser/resources/extensions/extensions.css (left):

https://codereview.chromium.org/1025703003/diff/40001/chrome/browser/resource...
chrome/browser/resources/extensions/extensions.css:6: transition-duration: 0ms
!important;
On 2015/03/21 01:07:18, Devlin wrote:
> On 2015/03/20 23:13:27, Dan Beam wrote:
> > might want to keep this
> 
> Was thinking about it, but if we keep it, then we have to also keep the
> 'loading' class stuff in extensions.js, which I thought was one of the main
> targets of the hackery. :)  I've tried it locally, and it *seems* to look
okay. 
> If we'd rather err on the side of caution, and keep this (and the loading
> stuff), then we certainly can - WDYT?

does this make the developer tools UI (buttons + promo) slide in?

https://codereview.chromium.org/1025703003/diff/60001/chrome/browser/resource...
File chrome/browser/resources/extensions/extension_list.js (right):

https://codereview.chromium.org/1025703003/diff/60001/chrome/browser/resource...
chrome/browser/resources/extensions/extension_list.js:246: return
this.extensions_ ? this.extensions_.length : 0;
this isn't the end of the world, but maybe just add:

  /** @private {!Array<WhateverThisTypeIs>} */
  this.extensions_ = [];

in the ctor so this is always around, because this is one of the few places in
this file it's actually null checked

https://codereview.chromium.org/1025703003/diff/60001/chrome/browser/resource...
File chrome/browser/resources/extensions/extensions.css (left):

https://codereview.chromium.org/1025703003/diff/60001/chrome/browser/resource...
chrome/browser/resources/extensions/extensions.css:170: .empty-extension-list ~
#footer-section {
i just really meant removing this.  leaving .loading sounds fine to me

[Devlin, 2015-03-21 01:32:23]

https://codereview.chromium.org/1025703003/diff/40001/chrome/browser/resource...
File chrome/browser/resources/extensions/extensions.css (left):

https://codereview.chromium.org/1025703003/diff/40001/chrome/browser/resource...
chrome/browser/resources/extensions/extensions.css:6: transition-duration: 0ms
!important;
On 2015/03/21 01:14:10, Dan Beam wrote:
> On 2015/03/21 01:07:18, Devlin wrote:
> > On 2015/03/20 23:13:27, Dan Beam wrote:
> > > might want to keep this
> > 
> > Was thinking about it, but if we keep it, then we have to also keep the
> > 'loading' class stuff in extensions.js, which I thought was one of the main
> > targets of the hackery. :)  I've tried it locally, and it *seems* to look
> okay. 
> > If we'd rather err on the side of caution, and keep this (and the loading
> > stuff), then we certainly can - WDYT?
> 
> does this make the developer tools UI (buttons + promo) slide in?

It didn't look like it made slide in the first time (sliding otherwise still
worked).  But moot if we leave in .loading.

https://codereview.chromium.org/1025703003/diff/60001/chrome/browser/resource...
File chrome/browser/resources/extensions/extension_list.js (right):

https://codereview.chromium.org/1025703003/diff/60001/chrome/browser/resource...
chrome/browser/resources/extensions/extension_list.js:246: return
this.extensions_ ? this.extensions_.length : 0;
On 2015/03/21 01:14:10, Dan Beam wrote:
> this isn't the end of the world, but maybe just add:
> 
>   /** @private {!Array<WhateverThisTypeIs>} */
>   this.extensions_ = [];
> 
> in the ctor so this is always around, because this is one of the few places in
> this file it's actually null checked

Done in the decorator (since we use cr.ui.define, we can't easily edit the
constructor).

https://codereview.chromium.org/1025703003/diff/60001/chrome/browser/resource...
File chrome/browser/resources/extensions/extensions.css (left):

https://codereview.chromium.org/1025703003/diff/60001/chrome/browser/resource...
chrome/browser/resources/extensions/extensions.css:170: .empty-extension-list ~
#footer-section {
On 2015/03/21 01:14:10, Dan Beam wrote:
> i just really meant removing this.  leaving .loading sounds fine to me

Oh, okay. Added back.

[Dan Beam, 2015-03-21 01:44:20]

https://codereview.chromium.org/1025703003/diff/80001/chrome/browser/resource...
File chrome/browser/resources/extensions/extension_list.js (right):

https://codereview.chromium.org/1025703003/diff/80001/chrome/browser/resource...
chrome/browser/resources/extensions/extension_list.js:147: var ExtensionList =
cr.ui.define('div');
if cr.ui.define() is limiting, do this instead

function ExtensionList(<maybe args>) {
  var el = cr.doc.createElement('div');
  el.__proto__ = ExtensionList.prototype;
  // use <maybe args> here
  return el;
}

https://codereview.chromium.org/1025703003/diff/80001/chrome/browser/resource...
chrome/browser/resources/extensions/extension_list.js:203: /** Needed because we
use cr.ui.define. */
see above

[Devlin, 2015-03-21 04:50:59]

Patchset #5 (id:100001) has been deleted

[Devlin, 2015-03-21 04:54:00]

https://codereview.chromium.org/1025703003/diff/80001/chrome/browser/resource...
File chrome/browser/resources/extensions/extension_list.js (right):

https://codereview.chromium.org/1025703003/diff/80001/chrome/browser/resource...
chrome/browser/resources/extensions/extension_list.js:147: var ExtensionList =
cr.ui.define('div');
On 2015/03/21 01:44:20, Dan Beam wrote:
> if cr.ui.define() is limiting, do this instead
> 
> function ExtensionList(<maybe args>) {
>   var el = cr.doc.createElement('div');
>   el.__proto__ = ExtensionList.prototype;
>   // use <maybe args> here
>   return el;
> }

Done.

https://codereview.chromium.org/1025703003/diff/80001/chrome/browser/resource...
chrome/browser/resources/extensions/extension_list.js:203: /** Needed because we
use cr.ui.define. */
On 2015/03/21 01:44:20, Dan Beam wrote:
> see above

Done.

[Devlin, 2015-03-23 20:04:17]

Dan, friendly ping?

[Dan Beam, 2015-03-23 20:09:15]

lgtm

https://codereview.chromium.org/1025703003/diff/120001/chrome/browser/resourc...
File chrome/browser/resources/extensions/extension_list.js (right):

https://codereview.chromium.org/1025703003/diff/120001/chrome/browser/resourc...
chrome/browser/resources/extensions/extension_list.js:273: var node = /** @type
{ExtensionFocusRow} */ ($(extension.id));
why is this cast necessary?

[Devlin, 2015-03-23 20:15:18]

https://codereview.chromium.org/1025703003/diff/120001/chrome/browser/resourc...
File chrome/browser/resources/extensions/extension_list.js (right):

https://codereview.chromium.org/1025703003/diff/120001/chrome/browser/resourc...
chrome/browser/resources/extensions/extension_list.js:273: var node = /** @type
{ExtensionFocusRow} */ ($(extension.id));
On 2015/03/23 20:09:15, Dan Beam wrote:
> why is this cast necessary?

Hmm... odd.  At one point, I was getting a compiler error because updateNode
(line 277) requires an ExtensionFocusRow (and $ presumably returns an
HTMLElement). But trying again, it seems totally fine. Weird.

Removed the cast.

[Devlin, 2015-03-23 20:15:21]

The CQ bit was checked by rdevlin.cronin@chromium.org

[Devlin, 2015-03-23 20:15:22]

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

[commit-bot: I haz the power, 2015-03-23 20:15:50]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1025703003/140001

[commit-bot: I haz the power, 2015-03-23 22:20:21]

Committed patchset #6 (id:140001)

[commit-bot: I haz the power, 2015-03-23 22:21:41]

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