[Extensions Page] Add a listener on extension list for "hasExtensions" changing

[Extensions Page] Add a listener on extension list for "hasExtensions" changing

Add a listener on the ExtensionList object for when the count of extensions
moves from 0 to 1, or from 1 to 0 - i.e., when we should show or hide the
list.

BUG=480110
Committed: https://crrev.com/ce954eece274b46f9acabd2e1b383f22050162ab
Cr-Commit-Position: refs/heads/master@{#327201}

[Devlin, 2015-04-23 17:38:30]

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

[Devlin, 2015-04-23 17:38:31]

Dan, please take a look.  Lemme know if you want the listener done differently -
this one's just the easy way out. ;)

[Dan Beam, 2015-04-23 18:01:39]

https://codereview.chromium.org/1105683003/diff/1/chrome/browser/resources/ex...
File chrome/browser/resources/extensions/extension_list.js (right):

https://codereview.chromium.org/1105683003/diff/1/chrome/browser/resources/ex...
chrome/browser/resources/extensions/extension_list.js:252:
this.extensions_.length == 1 &&
why length == 1?  can't we just call this every time?

https://codereview.chromium.org/1105683003/diff/1/chrome/browser/resources/ex...
chrome/browser/resources/extensions/extension_list.js:263:
this.onHasExtensionsChanged();
can't we just call this every time?

https://codereview.chromium.org/1105683003/diff/1/chrome/browser/resources/ex...
chrome/browser/resources/extensions/extension_list.js:268: }.bind(this));
nit:

  if (eventData.event_type == EventType.INSTALLED ||
      eventData.event_type == EventType.UNINSTALLED) {
    extensionList.onHasExtensionsChanged();
  }

https://codereview.chromium.org/1105683003/diff/1/chrome/browser/resources/ex...
File chrome/browser/resources/extensions/extensions.js (right):

https://codereview.chromium.org/1105683003/diff/1/chrome/browser/resources/ex...
chrome/browser/resources/extensions/extensions.js:277: var
onHasExtensionsChanged = function() {
onExtensionChanged (or at the very least HaveExtensions or HasExtension, right
now this makes little sense...)

https://codereview.chromium.org/1105683003/diff/1/chrome/browser/resources/ex...
chrome/browser/resources/extensions/extensions.js:283: };
make this a permanent instance or static method instead of this weird dance
you're doing

[Devlin, 2015-04-23 18:27:32]

https://codereview.chromium.org/1105683003/diff/1/chrome/browser/resources/ex...
File chrome/browser/resources/extensions/extension_list.js (right):

https://codereview.chromium.org/1105683003/diff/1/chrome/browser/resources/ex...
chrome/browser/resources/extensions/extension_list.js:252:
this.extensions_.length == 1 &&
On 2015/04/23 18:01:38, Dan Beam wrote:
> why length == 1?  can't we just call this every time?

Sure.

https://codereview.chromium.org/1105683003/diff/1/chrome/browser/resources/ex...
chrome/browser/resources/extensions/extension_list.js:263:
this.onHasExtensionsChanged();
On 2015/04/23 18:01:38, Dan Beam wrote:
> can't we just call this every time?

Ditto.

https://codereview.chromium.org/1105683003/diff/1/chrome/browser/resources/ex...
chrome/browser/resources/extensions/extension_list.js:268: }.bind(this));
On 2015/04/23 18:01:38, Dan Beam wrote:
> nit:
> 
>   if (eventData.event_type == EventType.INSTALLED ||
>       eventData.event_type == EventType.UNINSTALLED) {
>     extensionList.onHasExtensionsChanged();
>   }

Done.

https://codereview.chromium.org/1105683003/diff/1/chrome/browser/resources/ex...
File chrome/browser/resources/extensions/extensions.js (right):

https://codereview.chromium.org/1105683003/diff/1/chrome/browser/resources/ex...
chrome/browser/resources/extensions/extensions.js:277: var
onHasExtensionsChanged = function() {
On 2015/04/23 18:01:39, Dan Beam wrote:
> onExtensionChanged (or at the very least HaveExtensions or HasExtension, right
> now this makes little sense...)

How about onExtensionCountChanged?  (Before, it was saying that the property
'hasExtensions' on the list has changed, not that the extensions themselves have
changed.  But yes, it was very weirdly phrased.)

https://codereview.chromium.org/1105683003/diff/1/chrome/browser/resources/ex...
chrome/browser/resources/extensions/extensions.js:283: };
On 2015/04/23 18:01:38, Dan Beam wrote:
> make this a permanent instance or static method instead of this weird dance
> you're doing

Done.

[Dan Beam, 2015-04-27 16:17:05]

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

https://codereview.chromium.org/1105683003/diff/20001/chrome/browser/resource...
chrome/browser/resources/extensions/extension_list.js:239:
this.onExtensionCountChanged = null;
why does this need to ever be assigned/created dynamically?

https://codereview.chromium.org/1105683003/diff/20001/chrome/browser/resource...
File chrome/browser/resources/extensions/extensions.js (right):

https://codereview.chromium.org/1105683003/diff/20001/chrome/browser/resource...
chrome/browser/resources/extensions/extensions.js:279:
this.updateListVisibility_();
why does the |this| matter at all for this method?

https://codereview.chromium.org/1105683003/diff/20001/chrome/browser/resource...
chrome/browser/resources/extensions/extensions.js:287: updateListVisibility_:
function() {
make this static

https://codereview.chromium.org/1105683003/diff/20001/chrome/browser/resource...
chrome/browser/resources/extensions/extensions.js:370:
ExtensionSettings.getCurrentOverlay = function() {
^ this is an example of a static method

[Devlin, 2015-04-27 18:28:30]

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

https://codereview.chromium.org/1105683003/diff/20001/chrome/browser/resource...
chrome/browser/resources/extensions/extension_list.js:239:
this.onExtensionCountChanged = null;
On 2015/04/27 16:17:05, Dan Beam wrote:
> why does this need to ever be assigned/created dynamically?

Per offline conversation, extracted this into a delegate.

https://codereview.chromium.org/1105683003/diff/20001/chrome/browser/resource...
File chrome/browser/resources/extensions/extensions.js (right):

https://codereview.chromium.org/1105683003/diff/20001/chrome/browser/resource...
chrome/browser/resources/extensions/extensions.js:287: updateListVisibility_:
function() {
On 2015/04/27 16:17:05, Dan Beam wrote:
> make this static

Now, since this is a delegate method, I don't think this makes as much sense. 
Lemme know if you still want me to.

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

https://codereview.chromium.org/1105683003/diff/40001/chrome/browser/resource...
chrome/browser/resources/extensions/extension_list.js:167: * @param
{ExtensionList.Delegate} delegate
This causes a Closure Compiler error (ExtensionList.Delegate is an unknown
type).  Easy workaround?

[Dan Beam, 2015-04-27 18:48:00]

lookin' much better

https://codereview.chromium.org/1105683003/diff/20001/chrome/browser/resource...
File chrome/browser/resources/extensions/extensions.js (right):

https://codereview.chromium.org/1105683003/diff/20001/chrome/browser/resource...
chrome/browser/resources/extensions/extensions.js:287: updateListVisibility_:
function() {
On 2015/04/27 18:28:30, Devlin wrote:
> On 2015/04/27 16:17:05, Dan Beam wrote:
> > make this static
> 
> Now, since this is a delegate method, I don't think this makes as much sense. 
> Lemme know if you still want me to.

fine as is

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

https://codereview.chromium.org/1105683003/diff/40001/chrome/browser/resource...
chrome/browser/resources/extensions/extension_list.js:167: * @param
{ExtensionList.Delegate} delegate
On 2015/04/27 18:28:30, Devlin wrote:
> This causes a Closure Compiler error (ExtensionList.Delegate is an unknown
> type).  Easy workaround?

ExtensionList.Delegate -> ExtensionListDelegate (that occurs before this @param)

https://codereview.chromium.org/1105683003/diff/40001/chrome/browser/resource...
chrome/browser/resources/extensions/extension_list.js:288: eventData.event_type
== EventType.UNINSTALLED)
curlies

[Devlin, 2015-04-27 20:02:03]

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

https://codereview.chromium.org/1105683003/diff/40001/chrome/browser/resource...
chrome/browser/resources/extensions/extension_list.js:288: eventData.event_type
== EventType.UNINSTALLED)
On 2015/04/27 18:47:59, Dan Beam wrote:
> curlies

Done.

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

https://codereview.chromium.org/1105683003/diff/60001/chrome/browser/resource...
chrome/browser/resources/extensions/extension_list.js:177: * @param
{ExtensionListDelegate} delegate
Strangely, this (and also lines 244 and 250, as well as the references in
extensions.js) is still producing a warning for an "Unknown type
ExtensionListDelegate"... (making it extensions.ExtensionListDelegate didn't
help.  All seems well when running the page.)

Any ideas?

[Dan Beam, 2015-04-27 21:02:27]

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

https://codereview.chromium.org/1105683003/diff/60001/chrome/browser/resource...
chrome/browser/resources/extensions/extension_list.js:177: * @param
{ExtensionListDelegate} delegate
On 2015/04/27 20:02:03, Devlin wrote:
> Strangely, this (and also lines 244 and 250, as well as the references in
> extensions.js) is still producing a warning for an "Unknown type
> ExtensionListDelegate"... (making it extensions.ExtensionListDelegate didn't
> help.  All seems well when running the page.)
> 
> Any ideas?

this will work:

extensions.ExtensionListDelegate = function() {};
extensions.ExtensionListDelegate.prototype = {...};

pretty huge bummer that our compiler pass doesn't pick up your version, though
:(

[Devlin, 2015-04-27 21:11:54]

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

https://codereview.chromium.org/1105683003/diff/60001/chrome/browser/resource...
chrome/browser/resources/extensions/extension_list.js:177: * @param
{ExtensionListDelegate} delegate
On 2015/04/27 21:02:27, Dan Beam wrote:
> On 2015/04/27 20:02:03, Devlin wrote:
> > Strangely, this (and also lines 244 and 250, as well as the references in
> > extensions.js) is still producing a warning for an "Unknown type
> > ExtensionListDelegate"... (making it extensions.ExtensionListDelegate didn't
> > help.  All seems well when running the page.)
> > 
> > Any ideas?
> 
> this will work:
> 
> extensions.ExtensionListDelegate = function() {};
> extensions.ExtensionListDelegate.prototype = {...};
> 
> pretty huge bummer that our compiler pass doesn't pick up your version, though
> :(

Bummer indeed.  Odd that it worked fine when putting it on ExtensionList, minus
the first reference (i.e., there was 1 error, not 5).

Anyway, fixed.

[Devlin, 2015-04-27 22:19:47]

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

https://codereview.chromium.org/1105683003/diff/60001/chrome/browser/resource...
chrome/browser/resources/extensions/extension_list.js:177: * @param
{ExtensionListDelegate} delegate
On 2015/04/27 21:11:54, Devlin wrote:
> On 2015/04/27 21:02:27, Dan Beam wrote:
> > On 2015/04/27 20:02:03, Devlin wrote:
> > > Strangely, this (and also lines 244 and 250, as well as the references in
> > > extensions.js) is still producing a warning for an "Unknown type
> > > ExtensionListDelegate"... (making it extensions.ExtensionListDelegate
didn't
> > > help.  All seems well when running the page.)
> > > 
> > > Any ideas?
> > 
> > this will work:
> > 
> > extensions.ExtensionListDelegate = function() {};
> > extensions.ExtensionListDelegate.prototype = {...};
> > 
> > pretty huge bummer that our compiler pass doesn't pick up your version,
though
> > :(
> 
> Bummer indeed.  Odd that it worked fine when putting it on ExtensionList,
minus
> the first reference (i.e., there was 1 error, not 5).
> 
> Anyway, fixed.

Fixed better, from offline conversation.

[Dan Beam, 2015-04-27 22:59:38]

lgtm++

[Devlin, 2015-04-27 23:00:27]

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

[commit-bot: I haz the power, 2015-04-27 23:02:14]

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

[commit-bot: I haz the power, 2015-04-28 00:51:45]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-04-28 00:53:22]

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