Skip to content

Commit

Permalink
Revert of Make the compositing assert disabler for paint invalidation…
Browse files Browse the repository at this point in the history
… more targeted. (patchset #4 of https://codereview.chromium.org/506553004/)

Reason for revert:
This seems to be asserting in many PPAPI tests. For example:
http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=@ToT%20Blink&tests=OutOfProcessPPAPITest.FlashFullscreen&testType=browser_tests

Original issue's description:
> Make the compositing assert disabler for paint invalidation more targeted.
> 
> BUG=360286
> 
> Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=180997

[email protected],[email protected]
NOTREECHECKS=true
NOTRY=true
BUG=360286

Review URL: https://codereview.chromium.org/516603003

git-svn-id: svn://svn.chromium.org/blink/trunk@181025 bbb929c8-8fbe-4397-9dbb-9b2b20218538
  • Loading branch information
mounirlamouri committed Aug 28, 2014
1 parent 2956428 commit 1feb64e
Show file tree
Hide file tree
Showing 9 changed files with 5 additions and 41 deletions.
7 changes: 1 addition & 6 deletions Source/core/editing/Caret.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
* PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE COMPUTER, INC. OR
* CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
* CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
* EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
* PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
* PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
Expand All @@ -33,7 +33,6 @@
#include "core/frame/Settings.h"
#include "core/html/HTMLTextFormControlElement.h"
#include "core/rendering/RenderBlock.h"
#include "core/rendering/RenderLayer.h"
#include "core/rendering/RenderView.h"
#include "platform/graphics/GraphicsContext.h"

Expand Down Expand Up @@ -62,10 +61,6 @@ bool DragCaretController::isContentRichlyEditable() const

void DragCaretController::setCaretPosition(const VisiblePosition& position)
{
// for querying RenderLayer::compositingState()
// This code is probably correct, since it doesn't occur in a stack that involves updating compositing state.
DisableCompositingQueryAsserts disabler;

if (Node* node = m_position.deepEquivalent().deprecatedNode())
invalidateCaretRect(node);
m_position = position;
Expand Down
3 changes: 0 additions & 3 deletions Source/core/frame/FrameView.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -294,9 +294,6 @@ bool FrameView::didFirstLayout() const

void FrameView::invalidateRect(const IntRect& rect)
{
// For querying RenderLayer::compositingState() when invalidating scrollbars.
// FIXME: do all scrollbar invalidations after layout of all frames is complete. It's currently not recursively true.
DisableCompositingQueryAsserts disabler;
if (!parent()) {
if (HostWindow* window = hostWindow())
window->invalidateContentsAndRootView(rect);
Expand Down
4 changes: 0 additions & 4 deletions Source/core/html/HTMLCanvasElement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@
#include "core/html/canvas/WebGLContextEvent.h"
#include "core/html/canvas/WebGLRenderingContext.h"
#include "core/rendering/RenderHTMLCanvas.h"
#include "core/rendering/RenderLayer.h"
#include "platform/MIMETypeRegistry.h"
#include "platform/RuntimeEnabledFeatures.h"
#include "platform/graphics/Canvas2DImageBufferSurface.h"
Expand Down Expand Up @@ -213,9 +212,6 @@ void HTMLCanvasElement::didFinalizeFrame()
m_dirtyRect.intersect(srcRect);
if (RenderBox* ro = renderBox()) {
FloatRect mappedDirtyRect = mapRect(m_dirtyRect, srcRect, ro->contentBoxRect());
// For querying RenderLayer::compositingState()
// FIXME: is this invalidation using the correct compositing state?
DisableCompositingQueryAsserts disabler;
ro->invalidatePaintRectangle(enclosingIntRect(mappedDirtyRect));
}
notifyObserversCanvasChanged(m_dirtyRect);
Expand Down
4 changes: 1 addition & 3 deletions Source/core/rendering/RenderImage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@
#include "core/inspector/InspectorTraceEvents.h"
#include "core/rendering/HitTestResult.h"
#include "core/rendering/PaintInfo.h"
#include "core/rendering/RenderLayer.h"
#include "core/rendering/RenderView.h"
#include "core/rendering/TextRunConstructor.h"
#include "core/svg/graphics/SVGImage.h"
Expand Down Expand Up @@ -252,7 +251,6 @@ void RenderImage::paintInvalidationOrMarkForLayout(bool imageSizeChangedToAccomo
{
// FIXME: We should not be allowing paint invalidations during layout. crbug.com/339584
AllowPaintInvalidationScope scoper(frameView());
DisableCompositingQueryAsserts disabler;
invalidatePaintRectangle(paintInvalidationRect);
}

Expand Down Expand Up @@ -437,7 +435,7 @@ void RenderImage::areaElementFocusChanged(HTMLAreaElement* areaElement)
paintInvalidationRect.moveBy(-absoluteContentBox().location());
paintInvalidationRect.inflate(outlineWidth);

paintInvalidationOrMarkForLayout(false, &paintInvalidationRect);
invalidatePaintRectangle(paintInvalidationRect);
}

void RenderImage::paintIntoRect(GraphicsContext* context, const LayoutRect& rect)
Expand Down
5 changes: 0 additions & 5 deletions Source/core/rendering/RenderLayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1295,11 +1295,6 @@ void RenderLayer::removeOnlyThisLayer()
return;

m_clipper.clearClipRectsIncludingDescendants();

// For querying RenderLayer::compositingState()
// Eager invalidation here is correct, since we are invalidating with respect to the previous frame's
// compositing state when removing the layer.
DisableCompositingQueryAsserts disabler;
paintInvalidator().paintInvalidationIncludingNonCompositingDescendants();

RenderLayer* nextSib = nextSibling();
Expand Down
3 changes: 0 additions & 3 deletions Source/core/rendering/RenderLayerScrollableArea.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -403,9 +403,6 @@ void RenderLayerScrollableArea::setScrollOffset(const IntPoint& newScrollOffset)

// Just schedule a full paint invalidation of our object.
if (requiresPaintInvalidation) {
// For querying RenderLayer::compositingState()
// This code appears correct, since scrolling outside of layout happens during activities that do not dirty compositing state.
DisableCompositingQueryAsserts disabler;
if (box().frameView()->isInPerformLayout())
box().setShouldDoFullPaintInvalidation(true);
else
Expand Down
3 changes: 3 additions & 0 deletions Source/core/rendering/RenderObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1518,6 +1518,9 @@ void RenderObject::invalidatePaintUsingContainer(const RenderLayerModelObject* p
"object", this->debugName().ascii(),
"info", jsonObjectForPaintInvalidationInfo(r, invalidationReasonToString(invalidationReason)));

// For querying RenderLayer::compositingState()
DisableCompositingQueryAsserts disabler;

if (paintInvalidationContainer->isRenderFlowThread()) {
toRenderFlowThread(paintInvalidationContainer)->paintInvalidationRectangleInRegions(r);
return;
Expand Down
12 changes: 0 additions & 12 deletions Source/core/rendering/RenderView.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -589,10 +589,6 @@ void RenderView::invalidatePaintForSelection() const
{
HashSet<RenderBlock*> processedBlocks;

// For querying RenderLayer::compositingState()
// FIXME: this may be wrong. crbug.com/407416
DisableCompositingQueryAsserts disabler;

RenderObject* end = rendererAfterPosition(m_selectionEnd, m_selectionEndPos);
for (RenderObject* o = m_selectionStart; o && o != end; o = o->nextInPreOrder()) {
if (!o->canBeSelectionLeaf() && o != m_selectionStart && o != m_selectionEnd)
Expand Down Expand Up @@ -746,10 +742,6 @@ void RenderView::setSelection(RenderObject* start, int startPos, RenderObject* e
if (!m_frameView || blockPaintInvalidationMode == PaintInvalidationNothing)
return;

// For querying RenderLayer::compositingState()
// FIXME: this is wrong, selection should not cause eager invalidation. crbug.com/407416
DisableCompositingQueryAsserts disabler;

// Have any of the old selected objects changed compared to the new selection?
for (SelectedObjectMap::iterator i = oldSelectedObjects.begin(); i != oldObjectsEnd; ++i) {
RenderObject* obj = i->key;
Expand Down Expand Up @@ -802,10 +794,6 @@ void RenderView::getSelection(RenderObject*& startRenderer, int& startOffset, Re

void RenderView::clearSelection()
{
// For querying RenderLayer::compositingState()
// This is correct, since destroying render objects needs to cause eager paint invalidations.
DisableCompositingQueryAsserts disabler;

layer()->invalidatePaintForBlockSelectionGaps();
setSelection(0, -1, 0, -1, PaintInvalidationNewMinusOld);
}
Expand Down
5 changes: 0 additions & 5 deletions Source/core/rendering/compositing/RenderLayerCompositor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -474,11 +474,6 @@ void RenderLayerCompositor::paintInvalidationOnCompositingChange(RenderLayer* la
if (layer->renderer() != &m_renderView && !layer->renderer()->parent())
return;

// For querying RenderLayer::compositingState()
// Eager invalidation here is correct, since we are invalidating with respect to the previous frame's
// compositing state when changing the compositing backing of the layer.
DisableCompositingQueryAsserts disabler;

layer->paintInvalidator().paintInvalidationIncludingNonCompositingDescendants();
}

Expand Down

0 comments on commit 1feb64e

Please sign in to comment.