Skip to content

Commit

Permalink
Revert "[Chromecast] Refactor and Simplify CastWebView"
Browse files Browse the repository at this point in the history
This reverts commit 6d51408.

Reason for revert: b/149569123

Original change's description:
> [Chromecast] Refactor and Simplify CastWebView
> 
> CastWebView forces too much indirection from the underlying page/window
> functionality, so some forwarding methods have been removed in favor of
> calling the members directly. This is presently not ideal, but will be a
> great improvement once the CastWebContents and CastContentWindow are
> surfaced as mojo interfaces via CastWebService and their lifetimes can
> be tracked independently.
> 
> The CastWebService API has been simplified so that clients no longer
> need to provide a SiteInstance when creating CastWebViews. The renderer
> preloading logic, which was previously disjoint from CastWebView, has
> now been incorporated.
> 
> Merge-With: eureka-internal/331288
> 
> Bug: internal 77879457
> Test: CQ, manually verify on display assistant device.
> Change-Id: I72fc57f0669783f61076626962d47b72d93463f6
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1968257
> Reviewed-by: Yuchen Liu <yucliu@chromium.org>
> Commit-Queue: Sean Topping <seantopping@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#740143}

TBR=sanfin@chromium.org,seantopping@chromium.org,yucliu@chromium.org,lijiawei@chromium.org,mdellaquila@google.com,zxliang@google.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: internal 77879457
Change-Id: I9a6445b7a76e681e2f9c7e155c18058481e88684
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2065850
Reviewed-by: Sean Topping <seantopping@chromium.org>
Reviewed-by: Yuchen Liu <yucliu@chromium.org>
Commit-Queue: Sean Topping <seantopping@chromium.org>
Cr-Commit-Position: refs/heads/master@{#742893}
  • Loading branch information
Sean Topping authored and Commit Bot committed Feb 20, 2020
1 parent 718a6f6 commit cafbb26
Show file tree
Hide file tree
Showing 19 changed files with 157 additions and 247 deletions.
2 changes: 0 additions & 2 deletions chromecast/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,6 @@ cast_source_set("browser") {
"cast_web_contents_impl.h",
"cast_web_service.cc",
"cast_web_service.h",
"cast_web_view_base.cc",
"cast_web_view_base.h",
"cast_web_view_default.cc",
"cast_web_view_default.h",
"cast_web_view_factory.cc",
Expand Down
24 changes: 1 addition & 23 deletions chromecast/browser/cast_content_window_aura.cc
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,7 @@ CastContentWindowAura::CastContentWindowAura(
gesture_priority_(params.gesture_priority),
is_touch_enabled_(params.enable_touch_input),
window_(nullptr),
has_screen_access_(false),
resize_window_when_navigation_starts_(true) {}
has_screen_access_(false) {}

CastContentWindowAura::~CastContentWindowAura() {
CastWebContents::Observer::Observe(nullptr);
Expand All @@ -100,7 +99,6 @@ void CastContentWindowAura::CreateWindowForWebContents(
DCHECK(window_manager_) << "A CastWindowManager must be provided before "
<< "creating a window for WebContents.";
CastWebContents::Observer::Observe(cast_web_contents);
content::WebContentsObserver::Observe(cast_web_contents->web_contents());
window_ = cast_web_contents->web_contents()->GetNativeView();
if (!window_->HasObserver(this)) {
window_->AddObserver(this);
Expand Down Expand Up @@ -133,7 +131,6 @@ void CastContentWindowAura::GrantScreenAccess() {

void CastContentWindowAura::RevokeScreenAccess() {
has_screen_access_ = false;
resize_window_when_navigation_starts_ = false;
if (window_) {
window_->Hide();
// Because rendering a larger window may require more system resources,
Expand Down Expand Up @@ -191,23 +188,4 @@ void CastContentWindowAura::OnWindowDestroyed(aura::Window* window) {
window_ = nullptr;
}

void CastContentWindowAura::DidStartNavigation(
content::NavigationHandle* navigation_handle) {
if (!resize_window_when_navigation_starts_) {
return;
}
resize_window_when_navigation_starts_ = false;

// Resize window
gfx::Size display_size =
display::Screen::GetScreen()->GetPrimaryDisplay().size();
aura::Window* content_window = web_contents()->GetNativeView();
content_window->SetBounds(
gfx::Rect(display_size.width(), display_size.height()));
}

void CastContentWindowAura::WebContentsDestroyed() {
content::WebContentsObserver::Observe(nullptr);
}

} // namespace chromecast
9 changes: 0 additions & 9 deletions chromecast/browser/cast_content_window_aura.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@
#include "chromecast/browser/cast_content_gesture_handler.h"
#include "chromecast/browser/cast_content_window.h"
#include "chromecast/ui/media_control_ui.h"
#include "content/public/browser/web_contents.h"
#include "content/public/browser/web_contents_observer.h"
#include "ui/aura/window_observer.h"

namespace aura {
Expand All @@ -23,7 +21,6 @@ class TouchBlocker;

class CastContentWindowAura : public CastContentWindow,
public CastWebContents::Observer,
private content::WebContentsObserver,
public aura::WindowObserver {
public:
CastContentWindowAura(const CastContentWindow::CreateParams& params,
Expand Down Expand Up @@ -53,11 +50,6 @@ class CastContentWindowAura : public CastContentWindow,
void OnWindowDestroyed(aura::Window* window) override;

private:
// WebContentsObserver implementation:
void DidStartNavigation(
content::NavigationHandle* navigation_handle) override;
void WebContentsDestroyed() override;

CastWindowManager* const window_manager_;

// Utility class for detecting and dispatching gestures to delegates.
Expand All @@ -71,7 +63,6 @@ class CastContentWindowAura : public CastContentWindow,

aura::Window* window_;
bool has_screen_access_;
bool resize_window_when_navigation_starts_;

DISALLOW_COPY_AND_ASSIGN(CastContentWindowAura);
};
Expand Down
12 changes: 0 additions & 12 deletions chromecast/browser/cast_web_contents.h
Original file line number Diff line number Diff line change
Expand Up @@ -295,18 +295,6 @@ class CastWebContents {
// page.
virtual void Stop(int error_code) = 0;

// ===========================================================================
// Visibility
// ===========================================================================

// Specify if the WebContents should be treated as visible. This triggers a
// document "visibilitychange" change event, and will paint the WebContents
// quad if |visible| is true (otherwise it will be blank). Note that this does
// *not* guarantee the page is visible on the screen, as that depends on if
// the WebContents quad is present in the screen layout and isn't obscured by
// another window.
virtual void SetWebVisibilityAndPaint(bool visible) = 0;

// ===========================================================================
// Media Management
// ===========================================================================
Expand Down
15 changes: 0 additions & 15 deletions chromecast/browser/cast_web_contents_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -290,21 +290,6 @@ void CastWebContentsImpl::Stop(int error_code) {
NotifyPageState();
}

void CastWebContentsImpl::SetWebVisibilityAndPaint(bool visible) {
if (!web_contents_)
return;
if (visible) {
web_contents_->WasShown();
} else {
web_contents_->WasHidden();
}
if (web_contents_->GetVisibility() != content::Visibility::VISIBLE) {
// Since we are managing the visibility, we need to ensure pages are
// unfrozen in the event this occurred while in the background.
web_contents_->SetPageFrozen(false);
}
}

void CastWebContentsImpl::BlockMediaLoading(bool blocked) {
if (media_blocker_)
media_blocker_->BlockMediaLoading(blocked);
Expand Down
1 change: 0 additions & 1 deletion chromecast/browser/cast_web_contents_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ class CastWebContentsImpl : public CastWebContents,
void LoadUrl(const GURL& url) override;
void ClosePage() override;
void Stop(int error_code) override;
void SetWebVisibilityAndPaint(bool visible) override;
void RegisterInterfaceProvider(
const InterfaceSet& interface_set,
service_manager::InterfaceProvider* interface_provider) override;
Expand Down
23 changes: 19 additions & 4 deletions chromecast/browser/cast_web_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@
#include "base/time/time.h"
#include "chromecast/browser/cast_web_view_default.h"
#include "chromecast/browser/cast_web_view_factory.h"
#include "chromecast/browser/lru_renderer_cache.h"
#include "chromecast/chromecast_buildflags.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/gpu_utils.h"
#include "content/public/browser/media_session.h"
#include "content/public/browser/site_instance.h"
#include "content/public/browser/storage_partition.h"
#include "content/public/browser/web_contents.h"
#include "services/network/public/mojom/cookie_manager.mojom.h"
Expand All @@ -46,8 +46,6 @@ CastWebService::CastWebService(content::BrowserContext* browser_context,
: browser_context_(browser_context),
web_view_factory_(web_view_factory),
window_manager_(window_manager),
overlay_renderer_cache_(
std::make_unique<LRURendererCache>(browser_context_, 1)),
task_runner_(base::SequencedTaskRunnerHandle::Get()),
weak_factory_(this) {
DCHECK(browser_context_);
Expand All @@ -60,9 +58,26 @@ CastWebService::~CastWebService() = default;

CastWebView::Scoped CastWebService::CreateWebView(
const CastWebView::CreateParams& params,
scoped_refptr<content::SiteInstance> site_instance,
const GURL& initial_url) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
auto web_view = web_view_factory_->CreateWebView(params, this, initial_url);
auto web_view = web_view_factory_->CreateWebView(
params, this, std::move(site_instance), initial_url);
CastWebView::Scoped scoped(web_view.get(), [this](CastWebView* web_view) {
OwnerDestroyed(web_view);
});
web_views_.insert(std::move(web_view));
return scoped;
}

CastWebView::Scoped CastWebService::CreateWebView(
const CastWebView::CreateParams& params,
const GURL& initial_url) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
auto web_view = web_view_factory_->CreateWebView(
params, this,
content::SiteInstance::CreateForURL(browser_context_, initial_url),
initial_url);
CastWebView::Scoped scoped(web_view.get(), [this](CastWebView* web_view) {
OwnerDestroyed(web_view);
});
Expand Down
13 changes: 5 additions & 8 deletions chromecast/browser/cast_web_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ namespace chromecast {

class CastWebViewFactory;
class CastWindowManager;
class LRURendererCache;

// This class dispenses CastWebView objects which are used to wrap WebContents
// in cast_shell. This class temporarily takes ownership of CastWebViews when
Expand All @@ -42,17 +41,17 @@ class CastWebService {
CastWindowManager* window_manager);
~CastWebService();

CastWebView::Scoped CreateWebView(
const CastWebView::CreateParams& params,
scoped_refptr<content::SiteInstance> site_instance,
const GURL& initial_url);

CastWebView::Scoped CreateWebView(const CastWebView::CreateParams& params,
const GURL& initial_url);

std::unique_ptr<CastContentWindow> CreateWindow(
const CastContentWindow::CreateParams& params);

content::BrowserContext* browser_context() { return browser_context_; }
LRURendererCache* overlay_renderer_cache() {
return overlay_renderer_cache_.get();
}

void FlushDomLocalStorage();

// |callback| is called when data deletion is done or at least the deletion
Expand All @@ -69,8 +68,6 @@ class CastWebService {
CastWindowManager* const window_manager_;
base::flat_set<std::unique_ptr<CastWebView>> web_views_;

const std::unique_ptr<LRURendererCache> overlay_renderer_cache_;

const scoped_refptr<base::SequencedTaskRunner> task_runner_;
base::WeakPtr<CastWebService> weak_ptr_;

Expand Down
25 changes: 23 additions & 2 deletions chromecast/browser/cast_web_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,31 @@ CastWebView::Delegate::RunBluetoothChooser(
return nullptr;
}

CastWebView::CreateParams::CreateParams() = default;
CastWebView::CastWebView(const CreateParams& create_params)
: delegate_(create_params.delegate),
shutdown_delay_(create_params.shutdown_delay) {}

CastWebView::CreateParams::CreateParams(const CreateParams& other) = default;
CastWebView::~CastWebView() {
for (Observer& observer : observer_list_) {
observer.OnPageDestroyed(this);
}
}

void CastWebView::ForceClose() {
shutdown_delay_ = base::TimeDelta();
ClosePage();
}

void CastWebView::AddObserver(CastWebView::Observer* observer) {
observer_list_.AddObserver(observer);
}

void CastWebView::RemoveObserver(CastWebView::Observer* observer) {
observer_list_.RemoveObserver(observer);
}

CastWebView::CreateParams::CreateParams() = default;
CastWebView::CreateParams::CreateParams(const CreateParams& other) = default;
CastWebView::CreateParams::~CreateParams() = default;

} // namespace chromecast
58 changes: 34 additions & 24 deletions chromecast/browser/cast_web_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@
#include <string>

#include "base/macros.h"
#include "base/observer_list.h"
#include "base/strings/string16.h"
#include "base/time/time.h"
#include "base/values.h"
#include "chromecast/browser/cast_content_window.h"
#include "chromecast/browser/cast_web_contents.h"
#include "chromecast/ui/mojom/ui_service.mojom.h"
Expand All @@ -20,8 +22,6 @@

namespace chromecast {

class CastWebService;

// A simplified interface for loading and displaying WebContents in cast_shell.
class CastWebView {
public:
Expand Down Expand Up @@ -55,14 +55,6 @@ class CastWebView {
using Scoped =
std::unique_ptr<CastWebView, std::function<void(CastWebView*)>>;

enum class RendererPool {
// Don't use a renderer pool for prelaunching. This means launching the
// render process eagerly is un-restricted and will always succeed.
NONE,
// Pool for overlay apps, which allows up to one pre-cached site.
OVERLAY,
};

// The parameters used to create a CastWebView instance. Passed to
// CastWebService::CreateWebView().
struct CreateParams {
Expand All @@ -79,8 +71,6 @@ class CastWebView {
// Parameters for creating the content window for this CastWebView.
CastContentWindow::CreateParams window_params;

CastWebService* web_service = nullptr;

// Identifies the activity that is hosted by this CastWebView.
std::string activity_id = "";

Expand All @@ -105,42 +95,62 @@ class CastWebView {
// immediately and synchronously.
base::TimeDelta shutdown_delay = base::TimeDelta();

// Pool for pre-launched renderers.
RendererPool renderer_pool = RendererPool::NONE;

// Eagerly pre-launches a render process for |prelaunch_url| if it is valid.
GURL prelaunch_url;

CreateParams();
CreateParams(const CreateParams& other);
~CreateParams();
};

CastWebView() = default;
virtual ~CastWebView() = default;
explicit CastWebView(const CreateParams& create_params);
virtual ~CastWebView();

virtual CastContentWindow* window() const = 0;

virtual content::WebContents* web_contents() const = 0;

virtual CastWebContents* cast_web_contents() = 0;

virtual base::TimeDelta shutdown_delay() const = 0;
base::TimeDelta shutdown_delay() const { return shutdown_delay_; }

// Navigates to |url|. The loaded page will be preloaded if MakeVisible has
// not been called on the object.
virtual void LoadUrl(GURL url) = 0;

// Begins the close process for this page (ie. triggering document.onunload).
// A consumer of the class can be notified when the process has been finished
// via Delegate::OnPageStopped(). The page will be torn down after
// |CreateParams::shutdown_delay| has elapsed, or immediately if the browser
// is shutting down.
virtual void ClosePage() = 0;

// Closes the page immediately, ignoring |CreateParams::shutdown_delay|.
virtual void ForceClose() = 0;
void ForceClose();

// Adds the page to the window manager and makes it visible to the user if
// |is_visible| is true. |z_order| determines how this window is layered in
// relationt other windows (higher value == more foreground).
virtual void InitializeWindow(mojom::ZOrder z_order,
VisibilityPriority initial_priority) = 0;

// Allows the page to be shown on the screen. The page cannot be shown on the
// screen until this is called.
virtual void GrantScreenAccess() = 0;

// Prevents the page from being shown on the screen until GrantScreenAccess()
// is called.
virtual void RevokeScreenAccess() = 0;

// Observer interface:
virtual void AddObserver(Observer* observer) = 0;
virtual void RemoveObserver(Observer* observer) = 0;
void AddObserver(Observer* observer);
void RemoveObserver(Observer* observer);

protected:
base::WeakPtr<Delegate> delegate_;

private:
base::TimeDelta shutdown_delay_;

base::ObserverList<Observer>::Unchecked observer_list_;

DISALLOW_COPY_AND_ASSIGN(CastWebView);
};

Expand Down
Loading

0 comments on commit cafbb26

Please sign in to comment.