Title: [154997] trunk/Source
Revision
154997
Author
akl...@apple.com
Date
2013-09-03 12:30:20 -0700 (Tue, 03 Sep 2013)

Log Message

Support Vector<Ref<T>>.
<https://webkit.org/b/120637>

Reviewed by Antti Koivisto.

Source/WebCore:

Use Vector<Ref<T>> internally in Page.

* page/Page.cpp:
(WebCore::networkStateChanged):
(WebCore::Page::refreshPlugins):

    Clean up these functions and use Vector<Ref<Frame>> to store pointers to frames
    since we know they are not going to be null.

(WebCore::Page::pluginViews):

    Changed this to return a Vector<Ref<PluginView>> by value instead of passing a
    writable vector as an argument. Clean up loops with 'auto'.

(WebCore::Page::storageBlockingStateChanged):
(WebCore::Page::privateBrowsingStateChanged):

    Tweaked for pluginViews() returning a Vector now.

(WebCore::Page::setVisibilityState):

    Store Documents that need a visibilitychange event in a Vector<Ref<Document>>.

Source/WTF:

Add a Ref(const T&) constructor to enable Vector<Ref<T>>. This looks a bit awkward
but is necessary for Vector::append(const T&) to find a constructor.

An alternative would be to add something like std::vector::emplace_back, but I can't
think of a good name for that, and it'd be nice if append() would "just work."

Also add operator->(). I initially excluded this because I felt it made for
unsafe-looking code. Things quickly got out of hand with .get() everywhere though.

IMO -> looks OK as long as it's only used on the first link in a dereference chain,
as that variable and its type will be "in context."

* wtf/Ref.h:
(WTF::Ref::Ref):
(WTF::Ref::~Ref):
(WTF::Ref::operator->):
(WTF::Ref::get):
* wtf/VectorTraits.h:

    Add simple traits for Ref<T> so it can be moved around freely by Vector.

Modified Paths

Diff

Modified: trunk/Source/WTF/ChangeLog (154996 => 154997)


--- trunk/Source/WTF/ChangeLog	2013-09-03 19:14:41 UTC (rev 154996)
+++ trunk/Source/WTF/ChangeLog	2013-09-03 19:30:20 UTC (rev 154997)
@@ -1,3 +1,31 @@
+2013-09-03  Andreas Kling  <akl...@apple.com>
+
+        Support Vector<Ref<T>>.
+        <https://webkit.org/b/120637>
+
+        Reviewed by Antti Koivisto.
+
+        Add a Ref(const T&) constructor to enable Vector<Ref<T>>. This looks a bit awkward
+        but is necessary for Vector::append(const T&) to find a constructor.
+
+        An alternative would be to add something like std::vector::emplace_back, but I can't
+        think of a good name for that, and it'd be nice if append() would "just work."
+
+        Also add operator->(). I initially excluded this because I felt it made for
+        unsafe-looking code. Things quickly got out of hand with .get() everywhere though.
+
+        IMO -> looks OK as long as it's only used on the first link in a dereference chain,
+        as that variable and its type will be "in context."
+
+        * wtf/Ref.h:
+        (WTF::Ref::Ref):
+        (WTF::Ref::~Ref):
+        (WTF::Ref::operator->):
+        (WTF::Ref::get):
+        * wtf/VectorTraits.h:
+
+            Add simple traits for Ref<T> so it can be moved around freely by Vector.
+
 2013-09-03  Mikhail Pozdnyakov  <mikhail.pozdnya...@intel.com>
 
         Check WTF::VectorFiller template argument type size in compile time

Modified: trunk/Source/WTF/wtf/Ref.h (154996 => 154997)


--- trunk/Source/WTF/wtf/Ref.h	2013-09-03 19:14:41 UTC (rev 154996)
+++ trunk/Source/WTF/wtf/Ref.h	2013-09-03 19:30:20 UTC (rev 154997)
@@ -32,12 +32,18 @@
 
 template<typename T> class Ref {
 public:
-    ALWAYS_INLINE Ref(T& object) : m_ptr(&object) { m_ptr->ref(); }
-    ALWAYS_INLINE ~Ref() { m_ptr->deref(); }
+    // Ref(const T&) looks awkward but makes Vector<Ref<T>> possible.
+    Ref(const T& object) : m_ptr(&const_cast<T&>(object)) { m_ptr->ref(); }
+    Ref(T& object) : m_ptr(&object) { m_ptr->ref(); }
 
-    ALWAYS_INLINE const T& get() const { return *m_ptr; }
-    ALWAYS_INLINE T& get() { return *m_ptr; }
+    ~Ref() { m_ptr->deref(); }
 
+    const T* operator->() const { return m_ptr; }
+    T* operator->() { return m_ptr; }
+
+    const T& get() const { return *m_ptr; }
+    T& get() { return *m_ptr; }
+
 private:
     Ref(const Ref&);
     Ref& operator=(const Ref<T>&);

Modified: trunk/Source/WTF/wtf/VectorTraits.h (154996 => 154997)


--- trunk/Source/WTF/wtf/VectorTraits.h	2013-09-03 19:14:41 UTC (rev 154996)
+++ trunk/Source/WTF/wtf/VectorTraits.h	2013-09-03 19:30:20 UTC (rev 154997)
@@ -23,6 +23,7 @@
 
 #include <wtf/OwnArrayPtr.h>
 #include <wtf/OwnPtr.h>
+#include <wtf/Ref.h>
 #include <wtf/RefPtr.h>
 #include <wtf/TypeTraits.h>
 #include <utility>
@@ -82,6 +83,9 @@
     template<typename P>
     struct VectorTraits<OwnArrayPtr<P> > : SimpleClassVectorTraits { };
 
+    template<typename P>
+    struct VectorTraits<Ref<P> > : SimpleClassVectorTraits { };
+
     template<>
     struct VectorTraits<AtomicString> : SimpleClassVectorTraits { };
 

Modified: trunk/Source/WebCore/ChangeLog (154996 => 154997)


--- trunk/Source/WebCore/ChangeLog	2013-09-03 19:14:41 UTC (rev 154996)
+++ trunk/Source/WebCore/ChangeLog	2013-09-03 19:30:20 UTC (rev 154997)
@@ -1,3 +1,33 @@
+2013-09-03  Andreas Kling  <akl...@apple.com>
+
+        Support Vector<Ref<T>>.
+        <https://webkit.org/b/120637>
+
+        Reviewed by Antti Koivisto.
+
+        Use Vector<Ref<T>> internally in Page.
+
+        * page/Page.cpp:
+        (WebCore::networkStateChanged):
+        (WebCore::Page::refreshPlugins):
+
+            Clean up these functions and use Vector<Ref<Frame>> to store pointers to frames
+            since we know they are not going to be null.
+
+        (WebCore::Page::pluginViews):
+
+            Changed this to return a Vector<Ref<PluginView>> by value instead of passing a
+            writable vector as an argument. Clean up loops with 'auto'.
+
+        (WebCore::Page::storageBlockingStateChanged):
+        (WebCore::Page::privateBrowsingStateChanged):
+
+            Tweaked for pluginViews() returning a Vector now.
+
+        (WebCore::Page::setVisibilityState):
+
+            Store Documents that need a visibilitychange event in a Vector<Ref<Document>>.
+
 2013-08-28  Sergio Villar Senin  <svil...@igalia.com>
 
         [CSS Grid Layout] Add parsing for named grid lines

Modified: trunk/Source/WebCore/page/Page.cpp (154996 => 154997)


--- trunk/Source/WebCore/page/Page.cpp	2013-09-03 19:14:41 UTC (rev 154996)
+++ trunk/Source/WebCore/page/Page.cpp	2013-09-03 19:30:20 UTC (rev 154997)
@@ -98,13 +98,12 @@
 
 static void networkStateChanged(bool isOnLine)
 {
-    Vector<RefPtr<Frame> > frames;
+    Vector<Ref<Frame>> frames;
     
     // Get all the frames of all the pages in all the page groups
-    HashSet<Page*>::iterator end = allPages->end();
-    for (HashSet<Page*>::iterator it = allPages->begin(); it != end; ++it) {
+    for (auto it = allPages->begin(), end = allPages->end(); it != end; ++it) {
         for (Frame* frame = &(*it)->mainFrame(); frame; frame = frame->tree().traverseNext())
-            frames.append(frame);
+            frames.append(*frame);
         InspectorInstrumentation::networkStateChanged(*it);
     }
 
@@ -492,22 +491,18 @@
 
     PluginData::refresh();
 
-    Vector<RefPtr<Frame> > framesNeedingReload;
+    Vector<Ref<Frame>> framesNeedingReload;
 
-    HashSet<Page*>::iterator end = allPages->end();
-    for (HashSet<Page*>::iterator it = allPages->begin(); it != end; ++it) {
-        Page* page = *it;
-        
-        // Clear out the page's plug-in data.
-        if (page->m_pluginData)
-            page->m_pluginData = 0;
+    for (auto it = allPages->begin(), end = allPages->end(); it != end; ++it) {
+        Page& page = **it;
+        page.m_pluginData.clear();
 
         if (!reload)
             continue;
         
-        for (Frame* frame = &(*it)->mainFrame(); frame; frame = frame->tree().traverseNext()) {
+        for (Frame* frame = &page.mainFrame(); frame; frame = frame->tree().traverseNext()) {
             if (frame->loader().subframeLoader()->containsPlugins())
-                framesNeedingReload.append(frame);
+                framesNeedingReload.append(*frame);
         }
     }
 
@@ -1183,23 +1178,26 @@
         frame->document()->initDNSPrefetch();
 }
 
-void Page::collectPluginViews(Vector<RefPtr<PluginViewBase>, 32>& pluginViewBases)
+Vector<Ref<PluginViewBase>> Page::pluginViews()
 {
+    Vector<Ref<PluginViewBase>> views;
+
     for (Frame* frame = &mainFrame(); frame; frame = frame->tree().traverseNext()) {
         FrameView* view = frame->view();
         if (!view)
-            return;
+            break;
 
-        const HashSet<RefPtr<Widget> >* children = view->children();
+        auto children = view->children();
         ASSERT(children);
 
-        HashSet<RefPtr<Widget> >::const_iterator end = children->end();
-        for (HashSet<RefPtr<Widget> >::const_iterator it = children->begin(); it != end; ++it) {
+        for (auto it = children->begin(), end = children->end(); it != end; ++it) {
             Widget* widget = (*it).get();
             if (widget->isPluginViewBase())
-                pluginViewBases.append(toPluginViewBase(widget));
+                views.append(*toPluginViewBase(widget));
         }
     }
+
+    return views;
 }
 
 void Page::storageBlockingStateChanged()
@@ -1209,11 +1207,10 @@
 
     // Collect the PluginViews in to a vector to ensure that action the plug-in takes
     // from below storageBlockingStateChanged does not affect their lifetime.
-    Vector<RefPtr<PluginViewBase>, 32> pluginViewBases;
-    collectPluginViews(pluginViewBases);
+    auto views = pluginViews();
 
-    for (size_t i = 0; i < pluginViewBases.size(); ++i)
-        pluginViewBases[i]->storageBlockingStateChanged();
+    for (unsigned i = 0; i < views.size(); ++i)
+        views[i]->storageBlockingStateChanged();
 }
 
 void Page::privateBrowsingStateChanged()
@@ -1225,11 +1222,10 @@
 
     // Collect the PluginViews in to a vector to ensure that action the plug-in takes
     // from below privateBrowsingStateChanged does not affect their lifetime.
-    Vector<RefPtr<PluginViewBase>, 32> pluginViewBases;
-    collectPluginViews(pluginViewBases);
+    auto views = pluginViews();
 
-    for (size_t i = 0; i < pluginViewBases.size(); ++i)
-        pluginViewBases[i]->privateBrowsingStateChanged(privateBrowsingEnabled);
+    for (unsigned i = 0; i < views.size(); ++i)
+        views[i]->privateBrowsingStateChanged(privateBrowsingEnabled);
 }
 
 #if !ASSERT_DISABLED
@@ -1275,9 +1271,10 @@
     m_visibilityState = visibilityState;
 
     if (!isInitialState) {
-        Vector<RefPtr<Document>> documents;
+        Vector<Ref<Document>> documents;
         for (Frame* frame = m_mainFrame.get(); frame; frame = frame->tree().traverseNext())
-            documents.append(frame->document());
+            documents.append(*frame->document());
+
         for (size_t i = 0, size = documents.size(); i < size; ++i)
             documents[i]->dispatchEvent(Event::create(eventNames().visibilitychangeEvent, false, false));
     }

Modified: trunk/Source/WebCore/page/Page.h (154996 => 154997)


--- trunk/Source/WebCore/page/Page.h	2013-09-03 19:14:41 UTC (rev 154996)
+++ trunk/Source/WebCore/page/Page.h	2013-09-03 19:30:20 UTC (rev 154997)
@@ -36,6 +36,7 @@
 #include <wtf/HashMap.h>
 #include <wtf/HashSet.h>
 #include <wtf/Noncopyable.h>
+#include <wtf/Ref.h>
 #include <wtf/RefCounted.h>
 #include <wtf/text/WTFString.h>
 
@@ -436,7 +437,7 @@
     void setTimerAlignmentInterval(double);
     double timerAlignmentInterval() const;
 
-    void collectPluginViews(Vector<RefPtr<PluginViewBase>, 32>& pluginViewBases);
+    Vector<Ref<PluginViewBase>> pluginViews();
 
     void throttleTimers();
     void unthrottleTimers();
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to