Initial ResizeObserver implementation

third_party/WebKit/Source/core/observer/ResizeObservation.h

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef ResizeObservation_h
+#define ResizeObservation_h
+
+#include "bindings/core/v8/ScriptWrappable.h"

[szager1, 2016/05/23 19:49:10]

I don't think you use this.

[atotic1, 2016/05/25 00:15:52]

Done.

+#include "core/dom/Element.h"

[szager1, 2016/05/23 19:49:10]

You shouldn't need this, the forward class declaration is sufficient.

[atotic1, 2016/05/25 00:15:52]

Done.

+#include "core/observer/ResizeObserverEntry.h"
+#include "platform/heap/Handle.h"
+
+namespace blink {
+
+class Element;
+class ResizeObserver;
+
+// ResizeObservation represents an element that is being observed.
+class ResizeObservation : public GarbageCollectedFinalized<ResizeObservation> {

[szager1, 2016/05/23 19:49:11]

Too many blank lines.

[atotic1, 2016/05/25 00:15:53]

Done.

+
+public:
+
+    ResizeObservation(Element * target, ResizeObserver *);

[szager1, 2016/05/23 19:49:11]

No space between "Element" and "*".  Here and everywhere in this patch.

[atotic1, 2016/05/25 00:15:53]

Done.

+
+    virtual ~ResizeObservation() {};

[szager1, 2016/05/23 19:49:11]

No spurious destructors: get rid of this and change class declaration to us
GarbageCollected<ResizeObservation>.

[atotic1, 2016/05/25 00:15:52]

My understanding was that unless class is GarbageCollectedFinalized, its
destructor will not be called.

This class does need a destructor, because it has a member whose destructor
should be called:
HeapHashSet<Member<ResizeObservation>> m_observations
Who will be calling HeapHashSet's destructor?

[szager1, 2016/06/02 20:52:40]

HeapHashSet does not have a destructor.

+
+    // True if target was resized since last broadcast
+    bool hasResized() const;
+
+    Element * target() const { return m_target; }

[szager1, 2016/05/23 19:49:11]

No space between "Element" and "*"

[atotic1, 2016/05/25 00:15:53]

Done.

+
+    int broadcastWidth() const { return m_broadcastWidth; }
+
+    int broadcastHeight() const { return m_broadcastHeight; }
+
+    void setBroadcastSize(int width, int height);
+
+    DECLARE_TRACE();
+
+private:
+
+    WeakMember<Element> m_target;
+    Member<ResizeObserver> m_observer; // Used for GC only.

[szager1, 2016/05/23 19:49:10]

I think this comment is confusing; it doesn't give enough information to really
be useful.  I suggest getting rid of it.  If you want to provide an explanation
of gc semantics for this class (which I don't think is necessary, but feel
free), then it should go in a README.md.

[atotic1, 2016/05/25 00:15:53]

If I was refactoring a class, and saw a class member that was not used anywhere,

I'd be tempted to get rid of it.

This comment clarifies that in spite of being unused, the variable needs to
stay.
What's the harm in keeping in?

+    int m_broadcastWidth; // Last broadcast clientWidth.

[szager1, 2016/05/23 19:49:11]

We need to use LayoutUnit everywhere, not int.  For javascript interfaces,
convert to/from float.

To be concise, use LayoutSize:

  LayoutSize broadcastSize() const { return m_broadcastSize; }
  void setBroadcastSize(LayoutSize size) { m_broadcastSize = size; }
private:
  LayoutSize m_broadcastSize;

[szager1, 2016/05/23 19:49:11]

Instead of the comment, just rename the field to m_lastBroadcastSize.

[atotic1, 2016/05/25 00:15:52]

LayoutUnit vs int: current code returns an int, that's why I am holding onto an
int.

The size code is still in flux. Added a TODO, will take under advice in next
rewrite.

[atotic1, 2016/05/25 00:15:53]

Acknowledged.

+    int m_broadcastHeight; // Last broadcast clientHeight.
+};
+
+class ResizeObservationSet : public GarbageCollectedFinalized<ResizeObservationSet> {

[szager1, 2016/05/23 19:49:10]

We don't really use custom collection wrapper classes in blink; just use
HeapHashSet<Member<ResizeObservation>> everywhere.

[atotic1, 2016/05/25 00:15:53]

That's makes for some ugly looking declarations, and makes the collection
type harder to change. Done.

+
+public:
+
+    virtual ~ResizeObservationSet() {};
+
+    void add(ResizeObservation*);
+    void remove(ResizeObservation*);
+    DECLARE_TRACE();
+
+private:
+
+    HeapHashSet<Member<ResizeObservation>> m_observations;
+
+};
+
+} // namespace blink
+
+#endif // ResizeObservation_h