Ah, the joys of JS. If you don't want to clean this up in your current CL, please go ahead and assign this to me.

Definitely a bad part of JS, or to be more precise a bad API by W3C. addEventListener() should return an ID that you pass to removeEventListener(), similar to setTimeout() and clearTimeout() work. 

Anyway, I am trying to better understand the remaining responsibilities of CrScrollableBehavior, besides the updateScrollContents() method, before making any improvements/fixes in it. At first glance, the fact we call offsetHeight, scrollTop etc, on every 'scroll' event seems a bit heavy performance-wise, so following the trail of what do the CSS classes we toggle do, and why they are useful.

Assigning to dpapad for now to decide who can take on this bug.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fdb78fe88a06d28e76bf0e46346c19d2578c871a

commit fdb78fe88a06d28e76bf0e46346c19d2578c871a
Author: dpapad <dpapad@chromium.org>
Date: Wed Oct 26 23:06:12 2016

Tweak 'scroll' listener registration in CrScrollableBehavior.

Previously listeners were added in attached() and were unsuccessfully
removed in detached(). Unsuccessfully because removeEventListener() was
called with a different function reference than the one used in
addEventListener().

Now listeners are added in ready() which is guaranteed to execute once
regardless of how many times an element is attached/detached. Removing
the listeners is unncessary since a detached element can't be scrolled
anyway (and most likely will get GCed).

BUG= 658450 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2453923002
Cr-Commit-Position: refs/heads/master@{#427861}

[modify] https://crrev.com/fdb78fe88a06d28e76bf0e46346c19d2578c871a/ui/webui/resources/cr_elements/cr_scrollable_behavior.js