Skip to content

GitLab

T
tangled
Commit ccba8cdb authored by Nate Chapin's avatar Nate Chapin Committed by Commit Bot
Browse files
Options

Ensure orientation changes propagate to a grandchild even if the child's... 

Ensure orientation changes propagate to a grandchild even if the child's controller is not initialized

When an orientation change occurs, the ScreenOrientationController
of the local frame root propagates the orientation change event to
subframes. However, if the frame doesn't have a
ScreenOrientationController initialized, it skips that frame. However,
it also incorrectly skips any children of that frame, so if a child
does not a ScreenOrientationController, but one if its children does,
that grandchild will not receive the orientation change event like
it should.

Do a full traverse of the frame tree, rather than a layer-by-layer one.

Bug: 1121482
Test: ScreenOrientationControllerTest.OrientationChangePropagationToGrandchild
Change-Id: If9c9e8ef9aa3ad2047aa97778dbfb503d1d827ab
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2399008Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Commit-Queue: Nate Chapin <japhet@chromium.org>
Cr-Commit-Position: refs/heads/master@{#805337}
parent 92403630
Hide whitespace changes
Inline Side-by-side
Showing with 77 additions and 41 deletions
third_party/blink/renderer/core/testing/data/page_with_grandchild.html 0 → 100644
View file @ ccba8cdb
<html>
<body>
<iframe src="single_iframe.html"></iframe>
</body>
</html>
third_party/blink/renderer/modules/screen_orientation/screen_orientation_controller.cc
View file @ ccba8cdb
......@@ -117,16 +117,9 @@ void ScreenOrientationController::UpdateOrientation() {
orientation_->SetAngle(screen_info.orientation_angle);
}
bool ScreenOrientationController::IsActive() const {
return orientation_ && screen_orientation_service_.is_bound();
}
bool ScreenOrientationController::IsVisible() const {
return GetPage() && GetPage()->IsPageVisible();
}
bool ScreenOrientationController::IsActiveAndVisible() const {
return IsActive() && IsVisible();
return orientation_ && screen_orientation_service_.is_bound() && GetPage() &&
GetPage()->IsPageVisible();
}
void ScreenOrientationController::PageVisibilityChanged() {
......@@ -151,42 +144,37 @@ void ScreenOrientationController::PageVisibilityChanged() {
}
void ScreenOrientationController::NotifyOrientationChanged() {
if (!IsVisible() || !GetFrame())
return;
if (IsActive())
UpdateOrientation();
// Keep track of the frames that need to be notified before notifying the
// current frame as it will prevent side effects from the change event
// handlers.
HeapVector<Member<LocalFrame>> child_frames;
for (Frame* child = GetFrame()->Tree().FirstChild(); child;
child = child->Tree().NextSibling()) {
if (auto* child_local_frame = DynamicTo<LocalFrame>(child))
child_frames.push_back(child_local_frame);
HeapVector<Member<LocalFrame>> frames;
for (Frame* frame = GetFrame(); frame;
frame = frame->Tree().TraverseNext(GetFrame())) {
if (auto* local_frame = DynamicTo<LocalFrame>(frame))
frames.push_back(local_frame);
}
// Notify current orientation object.
if (IsActive() && orientation_) {
GetExecutionContext()
->GetTaskRunner(TaskType::kMiscPlatformAPI)
->PostTask(FROM_HERE,
WTF::Bind(
[](ScreenOrientation* orientation) {
ScopedAllowFullscreen allow_fullscreen(
ScopedAllowFullscreen::kOrientationChange);
orientation->DispatchEvent(
*Event::Create(event_type_names::kChange));
},
WrapPersistent(orientation_.Get())));
for (LocalFrame* frame : frames) {
if (auto* controller = FromIfExists(*frame->DomWindow()))
controller->NotifyOrientationChangedInternal();
}
}
// ... and child frames.
for (LocalFrame* child_frame : child_frames) {
if (auto* controller = FromIfExists(*child_frame->DomWindow()))
controller->NotifyOrientationChanged();
}
void ScreenOrientationController::NotifyOrientationChangedInternal() {
if (!IsActiveAndVisible())
return;
UpdateOrientation();
GetExecutionContext()
->GetTaskRunner(TaskType::kMiscPlatformAPI)
->PostTask(FROM_HERE,
WTF::Bind(
[](ScreenOrientation* orientation) {
ScopedAllowFullscreen allow_fullscreen(
ScopedAllowFullscreen::kOrientationChange);
orientation->DispatchEvent(
*Event::Create(event_type_names::kChange));
},
WrapPersistent(orientation_.Get())));
}
void ScreenOrientationController::SetOrientation(
......
third_party/blink/renderer/modules/screen_orientation/screen_orientation_controller.h
View file @ ccba8cdb
......@@ -56,6 +56,7 @@ class MODULES_EXPORT ScreenOrientationController final
static mojom::blink::ScreenOrientation ComputeOrientation(const gfx::Rect&,
uint16_t);
void NotifyOrientationChangedInternal();
// Inherited from ExecutionContextLifecycleObserver and
// PageVisibilityObserver.
......@@ -64,8 +65,6 @@ class MODULES_EXPORT ScreenOrientationController final
void UpdateOrientation();
bool IsActive() const;
bool IsVisible() const;
bool IsActiveAndVisible() const;
void OnLockOrientationResult(int, ScreenOrientationLockResult);
......
third_party/blink/renderer/modules/screen_orientation/screen_orientation_controller_test.cc
View file @ ccba8cdb
......@@ -11,6 +11,7 @@
#include "third_party/blink/public/web/web_local_frame.h"
#include "third_party/blink/renderer/core/frame/frame_test_helpers.h"
#include "third_party/blink/renderer/core/frame/local_dom_window.h"
#include "third_party/blink/renderer/core/frame/web_frame_widget_base.h"
#include "third_party/blink/renderer/core/testing/page_test_base.h"
#include "third_party/blink/renderer/modules/screen_orientation/screen_orientation.h"
#include "third_party/blink/renderer/modules/screen_orientation/web_lock_orientation_callback.h"
......@@ -254,6 +255,49 @@ TEST_F(ScreenOrientationControllerTest, PageVisibilityCrash) {
To<LocalFrame>(frame->Tree().FirstChild())->DomWindow());
EXPECT_EQ(child_orientation->angle(), 1234);
url_test_helpers::UnregisterAllURLsAndClearMemoryCache();
web_view_helper.Reset();
}
TEST_F(ScreenOrientationControllerTest,
OrientationChangePropagationToGrandchild) {
std::string base_url("http://internal.test/");
std::string test_url("page_with_grandchild.html");
url_test_helpers::RegisterMockedURLLoadFromBase(
WebString::FromUTF8(base_url), test::CoreTestDataPath(),
WebString::FromUTF8(test_url));
url_test_helpers::RegisterMockedURLLoadFromBase(
WebString::FromUTF8(base_url), test::CoreTestDataPath(),
WebString::FromUTF8("single_iframe.html"));
url_test_helpers::RegisterMockedURLLoadFromBase(
WebString::FromUTF8(base_url), test::CoreTestDataPath(),
WebString::FromUTF8("visible_iframe.html"));
frame_test_helpers::WebViewHelper web_view_helper;
ScreenInfoWebWidgetClient client;
client.SetAngle(1234);
web_view_helper.InitializeAndLoad(base_url + test_url, nullptr, nullptr,
&client);
Page* page = web_view_helper.GetWebView()->GetPage();
LocalFrame* frame = To<LocalFrame>(page->MainFrame());
// Fully set up on an orientation and a controller in the main frame and
// the grandchild, but not the child.
ScreenOrientation::Create(frame->DomWindow());
Frame* grandchild = frame->Tree().FirstChild()->Tree().FirstChild();
auto* grandchild_orientation =
ScreenOrientation::Create(To<LocalFrame>(grandchild)->DomWindow());
// Update the screen info and ensure it propagated to the grandchild.
blink::ScreenInfo screen_info;
screen_info.orientation_angle = 90;
auto* web_frame_widget_base =
static_cast<WebFrameWidgetBase*>(frame->GetWidgetForLocalRoot());
web_frame_widget_base->UpdateScreenInfo(screen_info);
EXPECT_EQ(grandchild_orientation->angle(), 90);
url_test_helpers::UnregisterAllURLsAndClearMemoryCache();
web_view_helper.Reset();
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment