Implement FrameLifecycle |
||||
Issue descriptionCurrently we're checking client(), host(), page(), etc in WebViewImpl et al to check if a frame is attached, so there's a lot of random null checks. Make a lifecycle like DocumentLifecycle, e.g. FrameLifecycle, for a Frame and assert you are at the correct stage of the lifecycle in each method. Add methods like isAttached(), etc which callers can use if they are unsure, but change client(), host(), etc to return references instead of pointers and assert the state is correct when they are called. Any errors that this causes should be fixed since they expose a path where the Frame has gotten into an inconsistent/invalid state.
,
Mar 15 2017
,
Mar 27 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/382a2483590ea3a82082d78c36b1f10a65ac7501 commit 382a2483590ea3a82082d78c36b1f10a65ac7501 Author: sashab <sashab@chromium.org> Date: Mon Mar 27 06:58:44 2017 Add a lifecycle state machine for Frame This CL adds a linear state machine to the Frame to ensure that it follows an orderly progression through its state machine. The plan is to use this state machine to make an isAttached() method which can check this state, and later make page(), client(), host() etc return references that DCHECK on the correct state. Also removed m_isDetaching and replaced it with checks on the lifecycle. Based on abarth's CL for adding a state machine for Document in: https://chromiumcodereview.appspot.com/25036004 BUG=700783 Review-Url: https://codereview.chromium.org/2748603002 Cr-Commit-Position: refs/heads/master@{#459710} [modify] https://crrev.com/382a2483590ea3a82082d78c36b1f10a65ac7501/third_party/WebKit/Source/core/frame/BUILD.gn [modify] https://crrev.com/382a2483590ea3a82082d78c36b1f10a65ac7501/third_party/WebKit/Source/core/frame/Frame.cpp [modify] https://crrev.com/382a2483590ea3a82082d78c36b1f10a65ac7501/third_party/WebKit/Source/core/frame/Frame.h [add] https://crrev.com/382a2483590ea3a82082d78c36b1f10a65ac7501/third_party/WebKit/Source/core/frame/FrameLifecycle.cpp [add] https://crrev.com/382a2483590ea3a82082d78c36b1f10a65ac7501/third_party/WebKit/Source/core/frame/FrameLifecycle.h [modify] https://crrev.com/382a2483590ea3a82082d78c36b1f10a65ac7501/third_party/WebKit/Source/core/frame/LocalFrame.cpp [modify] https://crrev.com/382a2483590ea3a82082d78c36b1f10a65ac7501/third_party/WebKit/Source/core/frame/RemoteFrame.cpp [modify] https://crrev.com/382a2483590ea3a82082d78c36b1f10a65ac7501/third_party/WebKit/Source/core/loader/FrameLoader.cpp [modify] https://crrev.com/382a2483590ea3a82082d78c36b1f10a65ac7501/third_party/WebKit/Source/core/page/Page.cpp [modify] https://crrev.com/382a2483590ea3a82082d78c36b1f10a65ac7501/third_party/WebKit/Source/web/WebFrame.cpp
,
Apr 4 2017
,
Feb 2 2018
Shifting focus to work on ChromeOS.
,
May 4 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3bd782b461d57a14f9067412b0cde1eda20ac720 commit 3bd782b461d57a14f9067412b0cde1eda20ac720 Author: Daniel Cheng <dcheng@chromium.org> Date: Fri May 04 18:47:24 2018 Unconditionally detach in Page::WillDetachFrame() ElementVisibilityObserverTest was using PageTestBase and would have been tricky to fix properly; instead just migrate it over to use FrameTestHelpers::WebViewHelper. PresentationAvailabilityTest is left as a V8TestingScope for now but should ultimately be converted as well. Bug: 700783, 734748 Change-Id: Ic73f187a2a1082891168143e37675a763f80db2c Reviewed-on: https://chromium-review.googlesource.com/1038142 Reviewed-by: Mounir Lamouri <mlamouri@chromium.org> Commit-Queue: Daniel Cheng <dcheng@chromium.org> Cr-Commit-Position: refs/heads/master@{#556134} [modify] https://crrev.com/3bd782b461d57a14f9067412b0cde1eda20ac720/third_party/blink/renderer/core/dom/element_visibility_observer_test.cc [modify] https://crrev.com/3bd782b461d57a14f9067412b0cde1eda20ac720/third_party/blink/renderer/core/frame/local_frame.cc [modify] https://crrev.com/3bd782b461d57a14f9067412b0cde1eda20ac720/third_party/blink/renderer/core/page/page.cc [modify] https://crrev.com/3bd782b461d57a14f9067412b0cde1eda20ac720/third_party/blink/renderer/modules/presentation/presentation_availability_test.cc |
||||
►
Sign in to add a comment |
||||
Comment 1 by sashab@chromium.org
, Mar 15 2017