Skip to content

Commit a961e6f

Browse files
zetafunctionzetafunction
committed
Revert of Remove default args to several PageWidgetDelegate members, try 2. (patchset #2 id:20001 of https://codereview.chromium.org/604463004/)
Reason for revert: Causing crashes on canary. Original issue's description: > Remove default args to several PageWidgetDelegate members, try 2. > > BUG=411993 > > Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=182737 TBR=japhet@chromium.org,kenrb@chromium.org NOTREECHECKS=true NOTRY=true BUG=411993 Review URL: https://codereview.chromium.org/598503003 git-svn-id: svn://svn.chromium.org/blink/trunk@182873 bbb929c8-8fbe-4397-9dbb-9b2b20218538
1 parent a8ecc88 commit a961e6f

File tree

4 files changed

+51
-35
lines changed

4 files changed

+51
-35
lines changed

‎Source/web/PageWidgetDelegate.cpp

+39-21
Original file line numberDiff line numberDiff line change
@@ -47,24 +47,41 @@
4747

4848
namespace blink {
4949

50-
void PageWidgetDelegate::animate(Page* page, double monotonicFrameBeginTime, LocalFrame* root)
50+
static inline FrameView* rootFrameView(Page* page, LocalFrame* rootFrame)
5151
{
52-
RefPtr<FrameView> view = root->view();
52+
if (rootFrame)
53+
return rootFrame->view();
54+
if (!page)
55+
return 0;
56+
if (!page->mainFrame()->isLocalFrame())
57+
return 0;
58+
return toLocalFrame(page->mainFrame())->view();
59+
}
60+
61+
void PageWidgetDelegate::animate(Page* page, double monotonicFrameBeginTime, LocalFrame* rootFrame)
62+
{
63+
RefPtr<FrameView> view = rootFrameView(page, rootFrame);
5364
if (!view)
5465
return;
5566
page->autoscrollController().animate(monotonicFrameBeginTime);
5667
page->animator().serviceScriptedAnimations(monotonicFrameBeginTime);
5768
}
5869

59-
void PageWidgetDelegate::layout(Page* page, LocalFrame* root)
70+
void PageWidgetDelegate::layout(Page* page, LocalFrame* rootFrame)
6071
{
6172
if (!page)
6273
return;
6374

64-
page->animator().updateLayoutAndStyleForPainting(root);
75+
if (!rootFrame) {
76+
if (!page->mainFrame() || !page->mainFrame()->isLocalFrame())
77+
return;
78+
rootFrame = toLocalFrame(page->mainFrame());
79+
}
80+
81+
page->animator().updateLayoutAndStyleForPainting(rootFrame);
6582
}
6683

67-
void PageWidgetDelegate::paint(Page* page, PageOverlayList* overlays, WebCanvas* canvas, const WebRect& rect, CanvasBackground background, LocalFrame* root)
84+
void PageWidgetDelegate::paint(Page* page, PageOverlayList* overlays, WebCanvas* canvas, const WebRect& rect, CanvasBackground background, LocalFrame* rootFrame)
6885
{
6986
if (rect.isEmpty())
7087
return;
@@ -74,7 +91,7 @@ void PageWidgetDelegate::paint(Page* page, PageOverlayList* overlays, WebCanvas*
7491
gc.setDeviceScaleFactor(page->deviceScaleFactor());
7592
IntRect dirtyRect(rect);
7693
gc.save(); // Needed to save the canvas, not the GraphicsContext.
77-
FrameView* view = root->view();
94+
FrameView* view = rootFrameView(page, rootFrame);
7895
if (view) {
7996
gc.clip(dirtyRect);
8097
view->paint(&gc, dirtyRect);
@@ -86,40 +103,41 @@ void PageWidgetDelegate::paint(Page* page, PageOverlayList* overlays, WebCanvas*
86103
gc.restore();
87104
}
88105

89-
bool PageWidgetDelegate::handleInputEvent(Page* page, PageWidgetEventHandler& handler, const WebInputEvent& event, LocalFrame* root)
106+
bool PageWidgetDelegate::handleInputEvent(Page* page, PageWidgetEventHandler& handler, const WebInputEvent& event, LocalFrame* rootFrame)
90107
{
108+
LocalFrame* frame = rootFrame;
109+
if (!frame)
110+
frame = page && page->mainFrame()->isLocalFrame() ? toLocalFrame(page->mainFrame()) : 0;
91111
switch (event.type) {
92112

93113
// FIXME: WebKit seems to always return false on mouse events processing
94114
// methods. For now we'll assume it has processed them (as we are only
95115
// interested in whether keyboard events are processed).
96-
// FIXME: Why do we return true when there is no root or the root is
97-
// detached?
98116
case WebInputEvent::MouseMove:
99-
if (!root || !root->view())
117+
if (!frame || !frame->view())
100118
return true;
101-
handler.handleMouseMove(*root, static_cast<const WebMouseEvent&>(event));
119+
handler.handleMouseMove(*frame, static_cast<const WebMouseEvent&>(event));
102120
return true;
103121
case WebInputEvent::MouseLeave:
104-
if (!root || !root->view())
122+
if (!frame || !frame->view())
105123
return true;
106-
handler.handleMouseLeave(*root, static_cast<const WebMouseEvent&>(event));
124+
handler.handleMouseLeave(*frame, static_cast<const WebMouseEvent&>(event));
107125
return true;
108126
case WebInputEvent::MouseDown:
109-
if (!root || !root->view())
127+
if (!frame || !frame->view())
110128
return true;
111-
handler.handleMouseDown(*root, static_cast<const WebMouseEvent&>(event));
129+
handler.handleMouseDown(*frame, static_cast<const WebMouseEvent&>(event));
112130
return true;
113131
case WebInputEvent::MouseUp:
114-
if (!root || !root->view())
132+
if (!frame || !frame->view())
115133
return true;
116-
handler.handleMouseUp(*root, static_cast<const WebMouseEvent&>(event));
134+
handler.handleMouseUp(*frame, static_cast<const WebMouseEvent&>(event));
117135
return true;
118136

119137
case WebInputEvent::MouseWheel:
120-
if (!root || !root->view())
138+
if (!frame || !frame->view())
121139
return false;
122-
return handler.handleMouseWheel(*root, static_cast<const WebMouseWheelEvent&>(event));
140+
return handler.handleMouseWheel(*frame, static_cast<const WebMouseWheelEvent&>(event));
123141

124142
case WebInputEvent::RawKeyDown:
125143
case WebInputEvent::KeyDown:
@@ -149,9 +167,9 @@ bool PageWidgetDelegate::handleInputEvent(Page* page, PageWidgetEventHandler& ha
149167
case WebInputEvent::TouchMove:
150168
case WebInputEvent::TouchEnd:
151169
case WebInputEvent::TouchCancel:
152-
if (!root || !root->view())
170+
if (!frame || !frame->view())
153171
return false;
154-
return handler.handleTouchEvent(*root, static_cast<const WebTouchEvent&>(event));
172+
return handler.handleTouchEvent(*frame, static_cast<const WebTouchEvent&>(event));
155173

156174
case WebInputEvent::GesturePinchBegin:
157175
case WebInputEvent::GesturePinchEnd:

‎Source/web/PageWidgetDelegate.h

+4-4
Original file line numberDiff line numberDiff line change
@@ -72,10 +72,10 @@ class PageWidgetDelegate {
7272
// rootFrame arguments indicate a root localFrame from which to start performing the
7373
// specified operation. If rootFrame is 0, these methods will attempt to use the
7474
// Page's mainFrame(), if it is a LocalFrame.
75-
static void animate(Page*, double monotonicFrameBeginTime, LocalFrame* root);
76-
static void layout(Page*, LocalFrame* root);
77-
static void paint(Page*, PageOverlayList*, WebCanvas*, const WebRect&, CanvasBackground, LocalFrame* root);
78-
static bool handleInputEvent(Page*, PageWidgetEventHandler&, const WebInputEvent&, LocalFrame* root);
75+
static void animate(Page*, double monotonicFrameBeginTime, LocalFrame* rootFrame = 0);
76+
static void layout(Page*, LocalFrame* rootFrame = 0);
77+
static void paint(Page*, PageOverlayList*, WebCanvas*, const WebRect&, CanvasBackground, LocalFrame* rootFrame = 0);
78+
static bool handleInputEvent(Page*, PageWidgetEventHandler&, const WebInputEvent&, LocalFrame* rootFrame = 0);
7979

8080
private:
8181
PageWidgetDelegate() { }

‎Source/web/WebPagePopupImpl.cpp

+4-4
Original file line numberDiff line numberDiff line change
@@ -323,7 +323,7 @@ void WebPagePopupImpl::beginFrame(const WebBeginFrameArgs& frameTime)
323323
{
324324
// FIXME: This should use frameTime.lastFrameTimeMonotonic but doing so
325325
// breaks tests.
326-
PageWidgetDelegate::animate(m_page.get(), monotonicallyIncreasingTime(), m_page->deprecatedLocalMainFrame());
326+
PageWidgetDelegate::animate(m_page.get(), monotonicallyIncreasingTime());
327327
}
328328

329329
void WebPagePopupImpl::willCloseLayerTreeView()
@@ -334,13 +334,13 @@ void WebPagePopupImpl::willCloseLayerTreeView()
334334

335335
void WebPagePopupImpl::layout()
336336
{
337-
PageWidgetDelegate::layout(m_page.get(), m_page->deprecatedLocalMainFrame());
337+
PageWidgetDelegate::layout(m_page.get());
338338
}
339339

340340
void WebPagePopupImpl::paint(WebCanvas* canvas, const WebRect& rect)
341341
{
342342
if (!m_closing)
343-
PageWidgetDelegate::paint(m_page.get(), 0, canvas, rect, PageWidgetDelegate::Opaque, m_page->deprecatedLocalMainFrame());
343+
PageWidgetDelegate::paint(m_page.get(), 0, canvas, rect, PageWidgetDelegate::Opaque);
344344
}
345345

346346
void WebPagePopupImpl::resize(const WebSize& newSize)
@@ -379,7 +379,7 @@ bool WebPagePopupImpl::handleInputEvent(const WebInputEvent& event)
379379
{
380380
if (m_closing)
381381
return false;
382-
return PageWidgetDelegate::handleInputEvent(m_page.get(), *this, event, m_page->deprecatedLocalMainFrame());
382+
return PageWidgetDelegate::handleInputEvent(m_page.get(), *this, event);
383383
}
384384

385385
bool WebPagePopupImpl::handleKeyEvent(const PlatformKeyboardEvent& event)

‎Source/web/WebViewImpl.cpp

+4-6
Original file line numberDiff line numberDiff line change
@@ -1839,9 +1839,7 @@ void WebViewImpl::beginFrame(const WebBeginFrameArgs& frameTime)
18391839
if (!m_page)
18401840
return;
18411841

1842-
// FIXME: This should probably be using the local root?
1843-
if (m_page->mainFrame()->isLocalFrame())
1844-
PageWidgetDelegate::animate(m_page.get(), validFrameTime.lastFrameTimeMonotonic, m_page->deprecatedLocalMainFrame());
1842+
PageWidgetDelegate::animate(m_page.get(), validFrameTime.lastFrameTimeMonotonic);
18451843

18461844
if (m_continuousPaintingEnabled) {
18471845
ContinuousPainter::setNeedsDisplayRecursive(m_rootGraphicsLayer, m_pageOverlays.get());
@@ -1877,7 +1875,7 @@ void WebViewImpl::paint(WebCanvas* canvas, const WebRect& rect)
18771875
ASSERT(!isAcceleratedCompositingActive());
18781876

18791877
double paintStart = currentTime();
1880-
PageWidgetDelegate::paint(m_page.get(), pageOverlays(), canvas, rect, isTransparent() ? PageWidgetDelegate::Translucent : PageWidgetDelegate::Opaque, m_page->deprecatedLocalMainFrame());
1878+
PageWidgetDelegate::paint(m_page.get(), pageOverlays(), canvas, rect, isTransparent() ? PageWidgetDelegate::Translucent : PageWidgetDelegate::Opaque);
18811879
double paintEnd = currentTime();
18821880
double pixelsPerSec = (rect.width * rect.height) / (paintEnd - paintStart);
18831881
Platform::current()->histogramCustomCounts("Renderer4.SoftwarePaintDurationMS", (paintEnd - paintStart) * 1000, 0, 120, 30);
@@ -1896,7 +1894,7 @@ void WebViewImpl::paintCompositedDeprecated(WebCanvas* canvas, const WebRect& re
18961894
PaintBehavior oldPaintBehavior = view->paintBehavior();
18971895
view->setPaintBehavior(oldPaintBehavior | PaintBehaviorFlattenCompositingLayers);
18981896

1899-
PageWidgetDelegate::paint(m_page.get(), pageOverlays(), canvas, rect, isTransparent() ? PageWidgetDelegate::Translucent : PageWidgetDelegate::Opaque, m_page->deprecatedLocalMainFrame());
1897+
PageWidgetDelegate::paint(m_page.get(), pageOverlays(), canvas, rect, isTransparent() ? PageWidgetDelegate::Translucent : PageWidgetDelegate::Opaque);
19001898

19011899
view->setPaintBehavior(oldPaintBehavior);
19021900
}
@@ -2086,7 +2084,7 @@ bool WebViewImpl::handleInputEvent(const WebInputEvent& inputEvent)
20862084
return true;
20872085
}
20882086

2089-
return PageWidgetDelegate::handleInputEvent(m_page.get(), *this, inputEvent, m_page->deprecatedLocalMainFrame());
2087+
return PageWidgetDelegate::handleInputEvent(m_page.get(), *this, inputEvent);
20902088
}
20912089

20922090
void WebViewImpl::setCursorVisibilityState(bool isVisible)

0 commit comments

Comments
 (0)