Skip to content

Commit

Permalink
Revert "Improve iOS PlatformViews to better handle thread merging. (f…
Browse files Browse the repository at this point in the history
…lutter#16935)" (flutter#17600)

This reverts commit f6b8eda.
  • Loading branch information
Chris Yang committed Apr 9, 2020
1 parent 6245149 commit caebc93
Show file tree
Hide file tree
Showing 20 changed files with 31 additions and 342 deletions.
2 changes: 0 additions & 2 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -592,8 +592,6 @@ FILE: ../../../flutter/shell/common/shell_io_manager.cc
FILE: ../../../flutter/shell/common/shell_io_manager.h
FILE: ../../../flutter/shell/common/shell_test.cc
FILE: ../../../flutter/shell/common/shell_test.h
FILE: ../../../flutter/shell/common/shell_test_external_view_embedder.cc
FILE: ../../../flutter/shell/common/shell_test_external_view_embedder.h
FILE: ../../../flutter/shell/common/shell_test_platform_view.cc
FILE: ../../../flutter/shell/common/shell_test_platform_view.h
FILE: ../../../flutter/shell/common/shell_test_platform_view_gl.cc
Expand Down
12 changes: 0 additions & 12 deletions flow/embedded_views.h
Original file line number Diff line number Diff line change
Expand Up @@ -253,18 +253,6 @@ class ExternalViewEmbedder {
// This is called after submitting the embedder frame and the surface frame.
virtual void FinishFrame();

// This should only be called after |SubmitFrame|.
// This method provides the embedder a way to do additional tasks after
// |SubmitFrame|. After invoking this method, the current task on the
// TaskRunner should end immediately.
//
// For example on the iOS embedder, threads are merged in this call.
// A new frame on the platform thread starts immediately. If the GPU thread
// still has some task running, there could be two frames being rendered
// concurrently, which causes undefined behaviors.
virtual void EndFrame(
fml::RefPtr<fml::RasterThreadMerger> raster_thread_merger) {}

FML_DISALLOW_COPY_AND_ASSIGN(ExternalViewEmbedder);

}; // ExternalViewEmbedder
Expand Down
2 changes: 0 additions & 2 deletions shell/common/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -191,8 +191,6 @@ if (enable_unittests) {
"pipeline_unittests.cc",
"shell_test.cc",
"shell_test.h",
"shell_test_external_view_embedder.cc",
"shell_test_external_view_embedder.h",
"shell_test_platform_view.cc",
"shell_test_platform_view.h",
"shell_unittests.cc",
Expand Down
2 changes: 1 addition & 1 deletion shell/common/animator_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ TEST_F(ShellTest, VSyncTargetTime) {
return ShellTestPlatformView::Create(
shell, shell.GetTaskRunners(), vsync_clock,
std::move(create_vsync_waiter),
ShellTestPlatformView::BackendType::kDefaultBackend, nullptr);
ShellTestPlatformView::BackendType::kDefaultBackend);
},
[](Shell& shell) {
return std::make_unique<Rasterizer>(shell, shell.GetTaskRunners());
Expand Down
10 changes: 0 additions & 10 deletions shell/common/rasterizer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -136,16 +136,6 @@ void Rasterizer::Draw(fml::RefPtr<Pipeline<flutter::LayerTree>> pipeline) {
consume_result = PipelineConsumeResult::MoreAvailable;
}

// Merging the thread as we know the next `Draw` should be run on the platform
// thread.
if (raster_status == RasterStatus::kResubmit) {
auto* external_view_embedder = surface_->GetExternalViewEmbedder();
// We know only the `external_view_embedder` can
// causes|RasterStatus::kResubmit|. Check to make sure.
FML_DCHECK(external_view_embedder != nullptr);
external_view_embedder->EndFrame(raster_thread_merger_);
}

// Consume as many pipeline items as possible. But yield the event loop
// between successive tries.
switch (consume_result) {
Expand Down
16 changes: 6 additions & 10 deletions shell/common/shell_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -260,12 +260,9 @@ std::unique_ptr<Shell> ShellTest::CreateShell(Settings settings,
simulate_vsync);
}

std::unique_ptr<Shell> ShellTest::CreateShell(
Settings settings,
TaskRunners task_runners,
bool simulate_vsync,
std::shared_ptr<ShellTestExternalViewEmbedder>
shell_test_external_view_embedder) {
std::unique_ptr<Shell> ShellTest::CreateShell(Settings settings,
TaskRunners task_runners,
bool simulate_vsync) {
const auto vsync_clock = std::make_shared<ShellTestVsyncClock>();
CreateVsyncWaiter create_vsync_waiter = [&]() {
if (simulate_vsync) {
Expand All @@ -278,18 +275,17 @@ std::unique_ptr<Shell> ShellTest::CreateShell(
};
return Shell::Create(
task_runners, settings,
[vsync_clock, &create_vsync_waiter,
shell_test_external_view_embedder](Shell& shell) {
[vsync_clock, &create_vsync_waiter](Shell& shell) {
return ShellTestPlatformView::Create(
shell, shell.GetTaskRunners(), vsync_clock,
std::move(create_vsync_waiter),
ShellTestPlatformView::BackendType::kDefaultBackend,
shell_test_external_view_embedder);
ShellTestPlatformView::BackendType::kDefaultBackend);
},
[](Shell& shell) {
return std::make_unique<Rasterizer>(shell, shell.GetTaskRunners());
});
}

void ShellTest::DestroyShell(std::unique_ptr<Shell> shell) {
DestroyShell(std::move(shell), GetTaskRunnersForFixture());
}
Expand Down
10 changes: 3 additions & 7 deletions shell/common/shell_test.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
#include "flutter/lib/ui/window/platform_message.h"
#include "flutter/shell/common/run_configuration.h"
#include "flutter/shell/common/shell.h"
#include "flutter/shell/common/shell_test_external_view_embedder.h"
#include "flutter/shell/common/thread_host.h"
#include "flutter/shell/common/vsync_waiters_test.h"
#include "flutter/testing/elf_loader.h"
Expand All @@ -32,12 +31,9 @@ class ShellTest : public ThreadTest {
Settings CreateSettingsForFixture();
std::unique_ptr<Shell> CreateShell(Settings settings,
bool simulate_vsync = false);
std::unique_ptr<Shell> CreateShell(
Settings settings,
TaskRunners task_runners,
bool simulate_vsync = false,
std::shared_ptr<ShellTestExternalViewEmbedder>
shell_test_external_view_embedder = nullptr);
std::unique_ptr<Shell> CreateShell(Settings settings,
TaskRunners task_runners,
bool simulate_vsync = false);
void DestroyShell(std::unique_ptr<Shell> shell);
void DestroyShell(std::unique_ptr<Shell> shell, TaskRunners task_runners);
TaskRunners GetTaskRunnersForFixture();
Expand Down
55 changes: 0 additions & 55 deletions shell/common/shell_test_external_view_embedder.cc

This file was deleted.

72 changes: 0 additions & 72 deletions shell/common/shell_test_external_view_embedder.h

This file was deleted.

10 changes: 3 additions & 7 deletions shell/common/shell_test_platform_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,24 +19,20 @@ std::unique_ptr<ShellTestPlatformView> ShellTestPlatformView::Create(
TaskRunners task_runners,
std::shared_ptr<ShellTestVsyncClock> vsync_clock,
CreateVsyncWaiter create_vsync_waiter,
BackendType backend,
std::shared_ptr<ShellTestExternalViewEmbedder>
shell_test_external_view_embedder) {
BackendType backend) {
// TODO(gw280): https://github.com/flutter/flutter/issues/50298
// Make this fully runtime configurable
switch (backend) {
case BackendType::kDefaultBackend:
#ifdef SHELL_ENABLE_GL
case BackendType::kGLBackend:
return std::make_unique<ShellTestPlatformViewGL>(
delegate, task_runners, vsync_clock, create_vsync_waiter,
shell_test_external_view_embedder);
delegate, task_runners, vsync_clock, create_vsync_waiter);
#endif // SHELL_ENABLE_GL
#ifdef SHELL_ENABLE_VULKAN
case BackendType::kVulkanBackend:
return std::make_unique<ShellTestPlatformViewVulkan>(
delegate, task_runners, vsync_clock, create_vsync_waiter,
shell_test_external_view_embedder);
delegate, task_runners, vsync_clock, create_vsync_waiter);
#endif // SHELL_ENABLE_VULKAN
default:
FML_LOG(FATAL) << "No backends supported for ShellTestPlatformView";
Expand Down
5 changes: 1 addition & 4 deletions shell/common/shell_test_platform_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
#define FLUTTER_SHELL_COMMON_SHELL_TEST_PLATFORM_VIEW_H_

#include "flutter/shell/common/platform_view.h"
#include "flutter/shell/common/shell_test_external_view_embedder.h"
#include "flutter/shell/common/vsync_waiters_test.h"

namespace flutter {
Expand All @@ -25,9 +24,7 @@ class ShellTestPlatformView : public PlatformView {
TaskRunners task_runners,
std::shared_ptr<ShellTestVsyncClock> vsync_clock,
CreateVsyncWaiter create_vsync_waiter,
BackendType backend,
std::shared_ptr<ShellTestExternalViewEmbedder>
shell_test_external_view_embedder);
BackendType backend);

virtual void SimulateVSync() = 0;

Expand Down
9 changes: 3 additions & 6 deletions shell/common/shell_test_platform_view_gl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,11 @@ ShellTestPlatformViewGL::ShellTestPlatformViewGL(
PlatformView::Delegate& delegate,
TaskRunners task_runners,
std::shared_ptr<ShellTestVsyncClock> vsync_clock,
CreateVsyncWaiter create_vsync_waiter,
std::shared_ptr<ShellTestExternalViewEmbedder>
shell_test_external_view_embedder)
CreateVsyncWaiter create_vsync_waiter)
: ShellTestPlatformView(delegate, std::move(task_runners)),
gl_surface_(SkISize::Make(800, 600)),
create_vsync_waiter_(std::move(create_vsync_waiter)),
vsync_clock_(vsync_clock),
shell_test_external_view_embedder_(shell_test_external_view_embedder) {}
vsync_clock_(vsync_clock) {}

ShellTestPlatformViewGL::~ShellTestPlatformViewGL() = default;

Expand Down Expand Up @@ -73,7 +70,7 @@ ShellTestPlatformViewGL::GetGLProcResolver() const {

// |GPUSurfaceGLDelegate|
ExternalViewEmbedder* ShellTestPlatformViewGL::GetExternalViewEmbedder() {
return shell_test_external_view_embedder_.get();
return nullptr;
}

} // namespace testing
Expand Down
8 changes: 1 addition & 7 deletions shell/common/shell_test_platform_view_gl.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
#ifndef FLUTTER_SHELL_COMMON_SHELL_TEST_PLATFORM_VIEW_GL_H_
#define FLUTTER_SHELL_COMMON_SHELL_TEST_PLATFORM_VIEW_GL_H_

#include "flutter/shell/common/shell_test_external_view_embedder.h"
#include "flutter/shell/common/shell_test_platform_view.h"
#include "flutter/shell/gpu/gpu_surface_gl_delegate.h"
#include "flutter/testing/test_gl_surface.h"
Expand All @@ -19,9 +18,7 @@ class ShellTestPlatformViewGL : public ShellTestPlatformView,
ShellTestPlatformViewGL(PlatformView::Delegate& delegate,
TaskRunners task_runners,
std::shared_ptr<ShellTestVsyncClock> vsync_clock,
CreateVsyncWaiter create_vsync_waiter,
std::shared_ptr<ShellTestExternalViewEmbedder>
shell_test_external_view_embedder);
CreateVsyncWaiter create_vsync_waiter);

virtual ~ShellTestPlatformViewGL() override;

Expand All @@ -34,9 +31,6 @@ class ShellTestPlatformViewGL : public ShellTestPlatformView,

std::shared_ptr<ShellTestVsyncClock> vsync_clock_;

std::shared_ptr<ShellTestExternalViewEmbedder>
shell_test_external_view_embedder_;

// |PlatformView|
std::unique_ptr<Surface> CreateRenderingSurface() override;

Expand Down

0 comments on commit caebc93

Please sign in to comment.