Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(167)

Unified Diff: third_party/WebKit/Source/core/frame/FrameView.cpp

Issue 2845583002: Remove FrameViewBase as base class of RemoteFrameView. (Closed)
Patch Set: ConvertSelfToChild take FrameOrPlugin as arg Created 3 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: third_party/WebKit/Source/core/frame/FrameView.cpp
diff --git a/third_party/WebKit/Source/core/frame/FrameView.cpp b/third_party/WebKit/Source/core/frame/FrameView.cpp
index ae3818d3c8d36c02f6bc7066a16ebf685155cf5d..5ce5ef9fbd434ec8f144ce61ae6373758ba79ce9 100644
--- a/third_party/WebKit/Source/core/frame/FrameView.cpp
+++ b/third_party/WebKit/Source/core/frame/FrameView.cpp
@@ -245,7 +245,6 @@ DEFINE_TRACE(FrameView) {
visitor->Trace(animating_scrollable_areas_);
visitor->Trace(auto_size_info_);
visitor->Trace(children_);
- visitor->Trace(plugins_);
visitor->Trace(scrollbars_);
visitor->Trace(viewport_scrollable_area_);
visitor->Trace(visibility_observer_);
@@ -253,7 +252,6 @@ DEFINE_TRACE(FrameView) {
visitor->Trace(anchoring_adjustment_queue_);
visitor->Trace(scrollbar_manager_);
visitor->Trace(print_context_);
- FrameViewBase::Trace(visitor);
ScrollableArea::Trace(visitor);
}
@@ -382,10 +380,10 @@ void FrameView::Dispose() {
// FIXME: Do we need to do something here for OOPI?
HTMLFrameOwnerElement* owner_element = frame_->DeprecatedLocalOwner();
// TODO(dcheng): It seems buggy that we can have an owner element that points
- // to another FrameViewBase. This can happen when a plugin element loads a
- // frame (FrameViewBase A of type FrameView) and then loads a plugin
- // (FrameViewBase B of type WebPluginContainerImpl). In this case, the frame's
- // view is A and the frame element's owned FrameViewBase is B. See
+ // to another FrameOrPlugin. This can happen when a plugin element loads a
+ // frame (FrameOrPlugin A of type FrameView) and then loads a plugin
+ // (FrameOrPlugin B of type WebPluginContainerImpl). In this case, the frame's
+ // view is A and the frame element's OwnedWidget is B. See
// https://crbug.com/673170 for an example.
if (owner_element && owner_element->OwnedWidget() == this)
owner_element->SetWidget(nullptr);
@@ -506,16 +504,12 @@ void FrameView::InvalidateAllCustomScrollbarsOnActiveChanged() {
bool uses_window_inactive_selector =
frame_->GetDocument()->GetStyleEngine().UsesWindowInactiveSelector();
- const ChildrenSet* view_children = Children();
- for (const Member<FrameViewBase>& child : *view_children) {
- FrameViewBase* frame_view_base = child.Get();
- if (frame_view_base->IsFrameView()) {
- ToFrameView(frame_view_base)
- ->InvalidateAllCustomScrollbarsOnActiveChanged();
- }
+ for (const auto& child : children_) {
+ if (child->IsFrameView())
+ ToFrameView(child)->InvalidateAllCustomScrollbarsOnActiveChanged();
}
- for (const Member<Scrollbar>& scrollbar : *Scrollbars()) {
+ for (const auto& scrollbar : scrollbars_) {
if (uses_window_inactive_selector && scrollbar->IsCustomScrollbar())
scrollbar->StyleChanged();
}
@@ -1169,7 +1163,7 @@ void FrameView::ScheduleOrPerformPostLayoutTasks() {
if (!post_layout_tasks_timer_.IsActive() &&
(NeedsLayout() || in_synchronous_post_layout_)) {
// If we need layout or are already in a synchronous call to
- // postLayoutTasks(), defer FrameViewBase updates and event dispatch until
+ // postLayoutTasks(), defer FrameView updates and event dispatch until
// after we return. postLayoutTasks() can make us need to update again, and
// we can get stuck in a nasty cycle unless we call it through the timer
// here.
@@ -1182,7 +1176,7 @@ void FrameView::ScheduleOrPerformPostLayoutTasks() {
void FrameView::UpdateLayout() {
// We should never layout a Document which is not in a LocalFrame.
DCHECK(frame_);
- ASSERT(frame_->View() == this);
+ DCHECK(frame_->View() == this);
DCHECK(frame_->GetPage());
ScriptForbiddenScope forbid_script;
@@ -1527,7 +1521,7 @@ void FrameView::UpdateGeometries() {
void FrameView::AddPartToUpdate(LayoutEmbeddedObject& object) {
DCHECK(IsInPerformLayout());
- // Tell the DOM element that it needs a FrameViewBase update.
+ // Tell the DOM element that it needs a Plugin update.
Node* node = object.GetNode();
DCHECK(node);
if (isHTMLObjectElement(*node) || isHTMLEmbedElement(*node))
@@ -2057,7 +2051,7 @@ void FrameView::UpdateLayersAndCompositingAfterScrollIfNeeded() {
}
// If there fixed position elements, scrolling may cause compositing layers to
- // change. Update FrameViewBase and layer positions after scrolling, but only
+ // change. Update FrameView and layer positions after scrolling, but only
// if we're not inside of layout.
if (!nested_layout_count_) {
UpdateGeometries();
@@ -2485,8 +2479,8 @@ void FrameView::ScrollToFragmentAnchor() {
}
bool FrameView::UpdatePlugins() {
- // This is always called from updatePluginsTimerFired.
- // m_updatePluginsTimer should only be scheduled if we have FrameViewBases to
+ // This is always called from UpdatePluginsTimerFired.
+ // update_plugins_timer should only be scheduled if we have FrameViews to
// update. Thus I believe we can stop checking isEmpty here, and just ASSERT
// isEmpty:
// FIXME: This assert has been temporarily removed due to
@@ -2755,7 +2749,7 @@ void FrameView::UpdateParentScrollableAreaSet() {
return;
// That ensures that only inner frames are cached.
- FrameView* parent_frame_view = this->ParentFrameView();
+ FrameView* parent_frame_view = ParentFrameView();
if (!parent_frame_view)
return;
@@ -3397,8 +3391,9 @@ void FrameView::UpdateStyleAndLayoutIfNeededRecursiveInternal() {
// TODO(leviw): This currently runs the entire lifecycle on plugin WebViews.
// We should have a way to only run these other Documents to the same
// lifecycle stage as this frame.
- for (const Member<PluginView>& plugin : *Plugins()) {
- plugin->UpdateAllLifecyclePhases();
+ for (const auto& child : children_) {
+ if (child->IsPluginView())
+ ToPluginView(child)->UpdateAllLifecyclePhases();
}
CheckDoesNotNeedLayout();
@@ -3650,11 +3645,11 @@ IntPoint FrameView::ConvertToLayoutItem(const LayoutItem& layout_item,
return RoundedIntPoint(layout_item.AbsoluteToLocal(point, kUseTransforms));
}
-IntPoint FrameView::ConvertSelfToChild(const FrameViewBase* child,
+IntPoint FrameView::ConvertSelfToChild(const FrameOrPlugin& child,
const IntPoint& point) const {
IntPoint new_point = point;
new_point = FrameToContents(point);
- new_point.MoveBy(-child->Location());
+ new_point.MoveBy(child.FrameRect().Location());
return new_point;
}
@@ -3681,7 +3676,7 @@ IntRect FrameView::ConvertFromContainingFrameViewBase(
if (parent_) {
IntRect local_rect = parent_rect;
local_rect.SetLocation(
- parent_->ConvertSelfToChild(this, local_rect.Location()));
+ parent_->ConvertSelfToChild(*this, local_rect.Location()));
return local_rect;
}
@@ -3845,8 +3840,7 @@ void FrameView::RemoveAnimatingScrollableArea(ScrollableArea* scrollable_area) {
animating_scrollable_areas_->erase(scrollable_area);
}
-void FrameView::SetParent(FrameViewBase* parent_frame_view_base) {
- FrameView* parent = ToFrameView(parent_frame_view_base);
+void FrameView::SetParent(FrameView* parent) {
if (parent == parent_)
return;
@@ -3864,7 +3858,13 @@ void FrameView::SetParent(FrameViewBase* parent_frame_view_base) {
subtree_throttled_ = ParentFrameView()->CanThrottleRendering();
}
-void FrameView::RemoveChild(FrameViewBase* child) {
+void FrameView::AddChild(FrameOrPlugin* child) {
+ DCHECK(child != this && !child->Parent());
+ child->SetParent(this);
+ children_.insert(child);
+}
+
+void FrameView::RemoveChild(FrameOrPlugin* child) {
DCHECK(child->Parent() == this);
if (child->IsFrameView() &&
@@ -3875,20 +3875,6 @@ void FrameView::RemoveChild(FrameViewBase* child) {
children_.erase(child);
}
-void FrameView::RemovePlugin(PluginView* plugin) {
- DCHECK(plugin->Parent() == this);
- DCHECK(plugins_.Contains(plugin));
- plugin->SetParent(nullptr);
- plugins_.erase(plugin);
-}
-
-void FrameView::AddPlugin(PluginView* plugin) {
- DCHECK(!plugin->Parent());
- DCHECK(!plugins_.Contains(plugin));
- plugin->SetParent(this);
- plugins_.insert(plugin);
-}
-
void FrameView::RemoveScrollbar(Scrollbar* scrollbar) {
DCHECK(scrollbars_.Contains(scrollbar));
scrollbar->SetParent(nullptr);
@@ -3937,9 +3923,6 @@ void FrameView::FrameRectsChanged() {
for (const auto& child : children_)
child->FrameRectsChanged();
-
- for (const auto& plugin : plugins_)
- plugin->FrameRectsChanged();
}
void FrameView::SetLayoutSizeInternal(const IntSize& size) {
@@ -3989,12 +3972,6 @@ IntSize FrameView::MaximumScrollOffsetInt() const {
return maximum_offset.ExpandedTo(MinimumScrollOffsetInt());
}
-void FrameView::AddChild(FrameViewBase* child) {
- DCHECK(child != this && !child->Parent());
- child->SetParent(this);
- children_.insert(child);
-}
-
void FrameView::SetScrollbarModes(ScrollbarMode horizontal_mode,
ScrollbarMode vertical_mode,
bool horizontal_lock,
@@ -4443,7 +4420,7 @@ void FrameView::ScrollContents(const IntSize& scroll_delta) {
SetNeedsPaintPropertyUpdate();
}
- // This call will move children with native FrameViewBases (plugins) and
+ // This call will move children with native FrameViews (plugins) and
// invalidate them as well.
FrameRectsChanged();
}
@@ -4790,9 +4767,6 @@ void FrameView::SetParentVisible(bool visible) {
for (const auto& child : children_)
child->SetParentVisible(visible);
-
- for (const auto& plugin : plugins_)
- plugin->SetParentVisible(visible);
}
void FrameView::Show() {
@@ -4813,9 +4787,6 @@ void FrameView::Show() {
if (IsParentVisible()) {
for (const auto& child : children_)
child->SetParentVisible(true);
-
- for (const auto& plugin : plugins_)
- plugin->SetParentVisible(true);
}
}
}
@@ -4825,9 +4796,6 @@ void FrameView::Hide() {
if (IsParentVisible()) {
for (const auto& child : children_)
child->SetParentVisible(false);
-
- for (const auto& plugin : plugins_)
- plugin->SetParentVisible(false);
}
SetSelfVisible(false);
if (ScrollingCoordinator* scrolling_coordinator =
@@ -4894,11 +4862,11 @@ void FrameView::CollectAnnotatedRegions(
void FrameView::UpdateViewportIntersectionsForSubtree(
DocumentLifecycle::LifecycleState target_state) {
- // TODO(dcheng): Since FrameViewBase tree updates are deferred, FrameViews
- // might still be in the FrameViewBase hierarchy even though the associated
+ // TODO(dcheng): Since FrameView tree updates are deferred, FrameViews
+ // might still be in the FrameView hierarchy even though the associated
// Document is already detached. Investigate if this check and a similar check
// in lifecycle updates are still needed when there are no more deferred
- // FrameViewBase updates: https://crbug.com/561683
+ // FrameView updates: https://crbug.com/561683
if (!GetFrame().GetDocument()->IsActive())
return;
@@ -4979,11 +4947,10 @@ void FrameView::UpdateRenderThrottlingStatus(
(was_throttled != is_throttled ||
force_throttling_invalidation_behavior ==
kForceThrottlingInvalidation)) {
- for (const Member<FrameViewBase>& child : *Children()) {
+ for (const auto& child : children_) {
if (child->IsFrameView()) {
- FrameView* child_view = ToFrameView(child);
- child_view->UpdateRenderThrottlingStatus(
- child_view->hidden_for_throttling_, is_throttled);
+ ToFrameView(child)->UpdateRenderThrottlingStatus(
+ ToFrameView(child)->hidden_for_throttling_, is_throttled);
}
}
}

Powered by Google App Engine
This is Rietveld 408576698