Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Robust-fy the LayerWebKitThread ownership with InRegionScroller

https://bugs.webkit.org/show_bug.cgi?id=93983
PR #191737

Reviewed by Yong Li.
Patch by Antonio Gomes <agomes@rim.com>

Patch changes the way we currently keep track of the active scrollable area
objects: before, we acquired the scrollable areas and just passed them in a vector up
to the client, copying it over and over again. Also, it was a client responsability to
delete stuff (BAD!).
Now, we keep track of vector within InRegionScroller, as a class member, which allows us to
avoid copies (in follow up patch), and control until when these objects outlive.

Patch also changes InRegionScrollableArea to "retptr" the composited layer
associated to it (if any). This ensure we have a non-null scrollable element always.

As mentioned, InRegionScroller is now responsible for deleting and vector of scrollable areas.

Internally reviewed by Arvid Nilsson.

* Api/InRegionScroller.cpp:
(WebKit):
(BlackBerry::WebKit::InRegionScrollerPrivate::reset): Method is now responsible for
deleting the tracked scrollable areas.
(BlackBerry::WebKit::InRegionScrollerPrivate::calculateInRegionScrollableAreasForPoint):
Renamed from 'inRegionScrollableAreasForPoint'. It was changed in order to store the
scrollable area objects instead of just pass a copy of them up to the client.
(BlackBerry::WebKit::InRegionScrollerPrivate::activeInRegionScrollableAreas): Getter.
(BlackBerry::WebKit::InRegionScrollerPrivate::pushBackInRegionScrollable): It was
promoted to a class method instead of a local helper.
* Api/InRegionScroller_p.h:
(WebKit):
(InRegionScrollerPrivate):
* Api/WebPage.cpp:
(BlackBerry::WebKit::WebPagePrivate::setScrollOriginPoint): Adjustments needed due to the
above changed.
* WebKitSupport/InRegionScrollableArea.cpp:
(BlackBerry::WebKit::InRegionScrollableArea::~InRegionScrollableArea): Clear up the cached layer.
(WebKit):
(BlackBerry::WebKit::InRegionScrollableArea::InRegionScrollableArea):
* WebKitSupport/InRegionScrollableArea.h:
(InRegionScrollableArea):

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@125678 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information...
commit 606a83370fbc195926719f35c6514775676775f6 1 parent 022aa52
Antonio Gomes authored
48 Source/WebKit/blackberry/Api/InRegionScroller.cpp
View
@@ -45,7 +45,6 @@ static bool canScrollRenderBox(RenderBox*);
static RenderLayer* parentLayer(RenderLayer*);
static Node* enclosingLayerNode(RenderLayer*);
static bool isNonRenderViewFixedPositionedContainer(RenderLayer*);
-static void pushBackInRegionScrollable(std::vector<Platform::ScrollViewBase*>&, InRegionScrollableArea*, InRegionScrollerPrivate*);
InRegionScroller::InRegionScroller(WebPagePrivate* webPagePrivate)
: d(new InRegionScrollerPrivate(webPagePrivate))
@@ -92,6 +91,10 @@ WebCore::Node* InRegionScrollerPrivate::node() const
void InRegionScrollerPrivate::reset()
{
setNode(0);
+
+ for (size_t i = 0; i < m_activeInRegionScrollableAreas.size(); ++i)
+ delete m_activeInRegionScrollableAreas[i];
+ m_activeInRegionScrollableAreas.clear();
}
bool InRegionScrollerPrivate::hasNode() const
@@ -143,15 +146,14 @@ bool InRegionScrollerPrivate::scrollBy(const Platform::IntSize& delta)
return scrollNodeRecursively(node(), delta);
}
-std::vector<Platform::ScrollViewBase*> InRegionScrollerPrivate::inRegionScrollableAreasForPoint(const WebCore::IntPoint& point)
+void InRegionScrollerPrivate::calculateInRegionScrollableAreasForPoint(const WebCore::IntPoint& point)
{
- std::vector<Platform::ScrollViewBase*> validReturn;
- std::vector<Platform::ScrollViewBase*> emptyReturn;
+ ASSERT(m_activeInRegionScrollableAreas.empty());
HitTestResult result = m_webPage->m_mainFrame->eventHandler()->hitTestResultAtPoint(m_webPage->mapFromViewportToContents(point), false /*allowShadowContent*/);
Node* node = result.innerNonSharedNode();
if (!node || !node->renderer())
- return emptyReturn;
+ return;
RenderLayer* layer = node->renderer()->enclosingLayer();
do {
@@ -160,37 +162,39 @@ std::vector<Platform::ScrollViewBase*> InRegionScrollerPrivate::inRegionScrollab
if (renderer->isRenderView()) {
if (RenderView* renderView = toRenderView(renderer)) {
FrameView* view = renderView->frameView();
- if (!view)
- return emptyReturn;
+ if (!view) {
+ reset();
+ return;
+ }
if (canScrollInnerFrame(view->frame())) {
- pushBackInRegionScrollable(validReturn, new InRegionScrollableArea(m_webPage, layer), this);
+ pushBackInRegionScrollable(new InRegionScrollableArea(m_webPage, layer));
continue;
}
}
} else if (canScrollRenderBox(layer->renderBox())) {
- pushBackInRegionScrollable(validReturn, new InRegionScrollableArea(m_webPage, layer), this);
+ pushBackInRegionScrollable(new InRegionScrollableArea(m_webPage, layer));
continue;
}
// If we run into a fix positioned layer, set the last scrollable in-region object
// as not able to propagate scroll to its parent scrollable.
- if (isNonRenderViewFixedPositionedContainer(layer) && validReturn.size()) {
- Platform::ScrollViewBase* end = validReturn.back();
+ if (isNonRenderViewFixedPositionedContainer(layer) && m_activeInRegionScrollableAreas.size()) {
+ Platform::ScrollViewBase* end = m_activeInRegionScrollableAreas.back();
end->setCanPropagateScrollingToEnclosingScrollable(false);
}
} while (layer = parentLayer(layer));
- if (validReturn.empty())
- return emptyReturn;
+ if (m_activeInRegionScrollableAreas.empty())
+ return;
// Post-calculate the visible window rects in reverse hit test order so
// we account for all and any clipping rects.
WebCore::IntRect recursiveClippingRect(WebCore::IntPoint::zero(), m_webPage->transformedViewportSize());
- std::vector<Platform::ScrollViewBase*>::reverse_iterator rend = validReturn.rend();
- for (std::vector<Platform::ScrollViewBase*>::reverse_iterator rit = validReturn.rbegin(); rit != rend; ++rit) {
+ std::vector<Platform::ScrollViewBase*>::reverse_iterator rend = m_activeInRegionScrollableAreas.rend();
+ for (std::vector<Platform::ScrollViewBase*>::reverse_iterator rit = m_activeInRegionScrollableAreas.rbegin(); rit != rend; ++rit) {
InRegionScrollableArea* curr = static_cast<InRegionScrollableArea*>(*rit);
RenderLayer* layer = curr->layer();
@@ -220,8 +224,11 @@ std::vector<Platform::ScrollViewBase*> InRegionScrollerPrivate::inRegionScrollab
recursiveClippingRect = visibleWindowRect;
}
}
+}
- return validReturn;
+const std::vector<Platform::ScrollViewBase*>& InRegionScrollerPrivate::activeInRegionScrollableAreas() const
+{
+ return m_activeInRegionScrollableAreas;
}
bool InRegionScrollerPrivate::setLayerScrollPosition(RenderLayer* layer, const IntPoint& scrollPosition)
@@ -431,16 +438,15 @@ static bool isNonRenderViewFixedPositionedContainer(RenderLayer* layer)
return o->isOutOfFlowPositioned() && o->style()->position() == FixedPosition;
}
-static void pushBackInRegionScrollable(std::vector<Platform::ScrollViewBase*>& vector, InRegionScrollableArea* scrollableArea, InRegionScrollerPrivate* scroller)
+void InRegionScrollerPrivate::pushBackInRegionScrollable(InRegionScrollableArea* scrollableArea)
{
- ASSERT(scroller);
ASSERT(!scrollableArea->isNull());
scrollableArea->setCanPropagateScrollingToEnclosingScrollable(!isNonRenderViewFixedPositionedContainer(scrollableArea->layer()));
- vector.push_back(scrollableArea);
- if (vector.size() == 1) {
+ m_activeInRegionScrollableAreas.push_back(scrollableArea);
+ if (m_activeInRegionScrollableAreas.size() == 1) {
// FIXME: Use RenderLayer::renderBox()->node() instead?
- scroller->setNode(enclosingLayerNode(scrollableArea->layer()));
+ setNode(enclosingLayerNode(scrollableArea->layer()));
}
}
7 Source/WebKit/blackberry/Api/InRegionScroller_p.h
View
@@ -35,6 +35,7 @@ class RenderLayer;
namespace BlackBerry {
namespace WebKit {
+class InRegionScrollableArea;
class WebPagePrivate;
class InRegionScrollerPrivate {
@@ -53,19 +54,23 @@ class InRegionScrollerPrivate {
bool setScrollPositionCompositingThread(unsigned camouflagedLayer, const WebCore::IntPoint& scrollPosition);
bool setScrollPositionWebKitThread(unsigned camouflagedLayer, const WebCore::IntPoint& scrollPosition);
- std::vector<Platform::ScrollViewBase*> inRegionScrollableAreasForPoint(const WebCore::IntPoint&);
+ void calculateInRegionScrollableAreasForPoint(const WebCore::IntPoint&);
+ const std::vector<Platform::ScrollViewBase*>& activeInRegionScrollableAreas() const;
WebPagePrivate* m_webPage;
private:
bool setLayerScrollPosition(WebCore::RenderLayer*, const WebCore::IntPoint& scrollPosition);
+ void pushBackInRegionScrollable(InRegionScrollableArea*);
+
// Obsolete codepath.
bool scrollNodeRecursively(WebCore::Node*, const WebCore::IntSize& delta);
bool scrollRenderer(WebCore::RenderObject*, const WebCore::IntSize& delta);
void adjustScrollDelta(const WebCore::IntPoint& maxOffset, const WebCore::IntPoint& currentOffset, WebCore::IntSize& delta) const;
RefPtr<WebCore::Node> m_inRegionScrollStartingNode;
+ std::vector<Platform::ScrollViewBase*> m_activeInRegionScrollableAreas;
};
}
4 Source/WebKit/blackberry/Api/WebPage.cpp
View
@@ -4177,7 +4177,9 @@ void WebPagePrivate::setScrollOriginPoint(const Platform::IntPoint& point)
if (!m_hasInRegionScrollableAreas)
return;
- m_client->notifyInRegionScrollingStartingPointChanged(m_inRegionScroller->d->inRegionScrollableAreasForPoint(point));
+ m_inRegionScroller->d->calculateInRegionScrollableAreasForPoint(point);
+ if (!m_inRegionScroller->d->activeInRegionScrollableAreas().empty())
+ m_client->notifyInRegionScrollingStartingPointChanged(m_inRegionScroller->d->activeInRegionScrollableAreas());
}
void WebPage::setScrollOriginPoint(const Platform::IntPoint& point)
1  Source/WebKit/blackberry/Api/WebPageClient.h
View
@@ -100,7 +100,6 @@ class BLACKBERRY_EXPORT WebPageClient {
virtual void notifyRunLayoutTestsFinished() = 0;
- // Client is responsible for deleting the vector elements.
virtual void notifyInRegionScrollingStartingPointChanged(std::vector<Platform::ScrollViewBase*>) = 0;
virtual void notifyDocumentOnLoad() = 0;
45 Source/WebKit/blackberry/ChangeLog
View
@@ -1,3 +1,48 @@
+2012-08-14 Antonio Gomes <agomes@rim.com>
+
+ [BlackBerry] Robust-fy the LayerWebKitThread ownership with InRegionScroller
+ https://bugs.webkit.org/show_bug.cgi?id=93983
+ PR #191737
+
+ Reviewed by Yong Li.
+
+ Patch changes the way we currently keep track of the active scrollable area
+ objects: before, we acquired the scrollable areas and just passed them in a vector up
+ to the client, copying it over and over again. Also, it was a client responsability to
+ delete stuff (BAD!).
+ Now, we keep track of vector within InRegionScroller, as a class member, which allows us to
+ avoid copies (in follow up patch), and control until when these objects outlive.
+
+ Patch also changes InRegionScrollableArea to "retptr" the composited layer
+ associated to it (if any). This ensure we have a non-null scrollable element always.
+
+ As mentioned, InRegionScroller is now responsible for deleting and vector of scrollable areas.
+
+ Internally reviewed by Arvid Nilsson.
+
+ * Api/InRegionScroller.cpp:
+ (WebKit):
+ (BlackBerry::WebKit::InRegionScrollerPrivate::reset): Method is now responsible for
+ deleting the tracked scrollable areas.
+ (BlackBerry::WebKit::InRegionScrollerPrivate::calculateInRegionScrollableAreasForPoint):
+ Renamed from 'inRegionScrollableAreasForPoint'. It was changed in order to store the
+ scrollable area objects instead of just pass a copy of them up to the client.
+ (BlackBerry::WebKit::InRegionScrollerPrivate::activeInRegionScrollableAreas): Getter.
+ (BlackBerry::WebKit::InRegionScrollerPrivate::pushBackInRegionScrollable): It was
+ promoted to a class method instead of a local helper.
+ * Api/InRegionScroller_p.h:
+ (WebKit):
+ (InRegionScrollerPrivate):
+ * Api/WebPage.cpp:
+ (BlackBerry::WebKit::WebPagePrivate::setScrollOriginPoint): Adjustments needed due to the
+ above changed.
+ * WebKitSupport/InRegionScrollableArea.cpp:
+ (BlackBerry::WebKit::InRegionScrollableArea::~InRegionScrollableArea): Clear up the cached layer.
+ (WebKit):
+ (BlackBerry::WebKit::InRegionScrollableArea::InRegionScrollableArea):
+ * WebKitSupport/InRegionScrollableArea.h:
+ (InRegionScrollableArea):
+
2012-08-15 Nima Ghanavatian <nghanavatian@rim.com>
[BlackBerry] Check for valid field focus before processing a spellcheck request
7 Source/WebKit/blackberry/WebKitSupport/InRegionScrollableArea.cpp
View
@@ -40,6 +40,10 @@ InRegionScrollableArea::InRegionScrollableArea()
{
}
+InRegionScrollableArea::~InRegionScrollableArea()
+{
+}
+
InRegionScrollableArea::InRegionScrollableArea(WebPagePrivate* webPage, RenderLayer* layer)
: m_webPage(webPage)
, m_layer(layer)
@@ -89,7 +93,8 @@ InRegionScrollableArea::InRegionScrollableArea(WebPagePrivate* webPage, RenderLa
if (m_layer->usesCompositedScrolling()) {
m_supportsCompositedScrolling = true;
ASSERT(m_layer->backing()->hasScrollingLayer());
- m_cachedCompositedScrollableLayer = reinterpret_cast<unsigned>(m_layer->backing()->scrollingLayer()->platformLayer());
+ m_camouflagedCompositedScrollableLayer = reinterpret_cast<unsigned>(m_layer->backing()->scrollingLayer()->platformLayer());
+ m_cachedCompositedScrollableLayer = m_layer->backing()->scrollingLayer()->platformLayer();
}
m_overscrollLimitFactor = 0.0;
5 Source/WebKit/blackberry/WebKitSupport/InRegionScrollableArea.h
View
@@ -24,6 +24,7 @@
#include <interaction/ScrollViewBase.h>
namespace WebCore {
+class LayerWebKitThread;
class RenderLayer;
}
@@ -37,6 +38,7 @@ class InRegionScrollableArea : public Platform::ScrollViewBase {
InRegionScrollableArea();
InRegionScrollableArea(WebPagePrivate*, WebCore::RenderLayer*);
+ virtual ~InRegionScrollableArea();
void setVisibleWindowRect(const WebCore::IntRect&);
Platform::IntRect visibleWindowRect() const;
@@ -46,6 +48,9 @@ class InRegionScrollableArea : public Platform::ScrollViewBase {
private:
WebPagePrivate* m_webPage;
WebCore::RenderLayer* m_layer;
+
+ RefPtr<WebCore::LayerWebKitThread> m_cachedCompositedScrollableLayer;
+
bool m_hasWindowVisibleRectCalculated;
};
Please sign in to comment.
Something went wrong with that request. Please try again.