Skip to content

Commit

Permalink
refactor: remove WebContents::CreateFrom
Browse files Browse the repository at this point in the history
  • Loading branch information
@zcbenz
zcbenz committed Oct 19, 2018
1 parent 8bc0c34 commit f9c20ff
Show file tree
Hide file tree
Showing 6 changed files with 89 additions and 51 deletions.
15 changes: 9 additions & 6 deletions atom/browser/api/atom_api_app.cc
View file
Expand Up @@ -683,7 +683,7 @@ void App::OnLogin(scoped_refptr<LoginHandler> login_handler,
content::WebContents* web_contents = login_handler->GetWebContents();
if (web_contents) {
prevent_default = Emit(
"login", WebContents::CreateFrom(isolate(), web_contents),
"login", WebContents::FromOrCreate(isolate(), web_contents),
request_details, login_handler->auth_info(),
base::Bind(&PassLoginInformation, base::RetainedRef(login_handler)));
}
Expand Down Expand Up @@ -714,9 +714,12 @@ bool App::CanCreateWindow(
content::WebContents* web_contents =
content::WebContents::FromRenderFrameHost(opener);
if (web_contents) {
auto api_web_contents = WebContents::CreateFrom(isolate(), web_contents);
api_web_contents->OnCreateWindow(target_url, referrer, frame_name,
disposition, additional_features, body);
auto api_web_contents = WebContents::From(isolate(), web_contents);
// No need to emit any event if the WebContents is not available in JS.
if (!api_web_contents.IsEmpty()) {
api_web_contents->OnCreateWindow(target_url, referrer, frame_name,
disposition, additional_features, body);
}
}

return false;
Expand All @@ -735,7 +738,7 @@ void App::AllowCertificateError(
v8::Locker locker(isolate());
v8::HandleScope handle_scope(isolate());
bool prevent_default = Emit(
"certificate-error", WebContents::CreateFrom(isolate(), web_contents),
"certificate-error", WebContents::FromOrCreate(isolate(), web_contents),
request_url, net::ErrorToString(cert_error), ssl_info.cert, callback);

// Deny the certificate by default.
Expand All @@ -762,7 +765,7 @@ void App::SelectClientCertificate(

bool prevent_default =
Emit("select-client-certificate",
WebContents::CreateFrom(isolate(), web_contents),
WebContents::FromOrCreate(isolate(), web_contents),
cert_request_info->host_and_port.ToString(), std::move(client_certs),
base::Bind(&OnClientCertificateSelected, isolate(), shared_delegate,
shared_identities));
Expand Down
6 changes: 5 additions & 1 deletion atom/browser/api/atom_api_render_process_preferences.cc
View file
Expand Up @@ -26,7 +26,11 @@ bool IsWebContents(v8::Isolate* isolate, content::RenderProcessHost* process) {
if (!web_contents)
return false;

auto api_web_contents = WebContents::CreateFrom(isolate, web_contents);
// No need to process if the WebContents is not managed by us.
auto api_web_contents = WebContents::From(isolate, web_contents);
if (api_web_contents.IsEmpty())
return false;

auto type = api_web_contents->GetType();
return type == WebContents::Type::BROWSER_WINDOW ||
type == WebContents::Type::WEB_VIEW;
Expand Down
82 changes: 49 additions & 33 deletions atom/browser/api/atom_api_web_contents.cc
View file
Expand Up @@ -309,22 +309,25 @@ struct WebContents::FrameDispatchHelper {
}
};

WebContents::WebContents(v8::Isolate* isolate,
content::WebContents* web_contents)
: content::WebContentsObserver(web_contents), type_(REMOTE) {
web_contents->SetUserAgentOverride(GetBrowserContext()->GetUserAgent(),
false);
Init(isolate);
AttachAsUserData(web_contents);
InitZoomController(web_contents, mate::Dictionary::CreateEmpty(isolate));
}

WebContents::WebContents(v8::Isolate* isolate,
content::WebContents* web_contents,
Type type)
: content::WebContentsObserver(web_contents), type_(type) {
const mate::Dictionary options = mate::Dictionary::CreateEmpty(isolate);
if (type == REMOTE) {
web_contents->SetUserAgentOverride(GetBrowserContext()->GetUserAgent(),
false);
Init(isolate);
AttachAsUserData(web_contents);
InitZoomController(web_contents, options);
} else {
auto session = Session::CreateFrom(isolate, GetBrowserContext());
session_.Reset(isolate, session.ToV8());
InitWithSessionAndOptions(isolate, web_contents, session, options);
}
DCHECK(type != REMOTE) << "Can't take ownership of a remote WebContents";
auto session = Session::CreateFrom(isolate, GetBrowserContext());
session_.Reset(isolate, session.ToV8());
InitWithSessionAndOptions(isolate, web_contents, session,
mate::Dictionary::CreateEmpty(isolate));
}

WebContents::WebContents(v8::Isolate* isolate,
Expand Down Expand Up @@ -538,8 +541,9 @@ void WebContents::WebContentsCreated(content::WebContents* source_contents,
content::WebContents* new_contents) {
v8::Locker locker(isolate());
v8::HandleScope handle_scope(isolate());
auto api_web_contents = CreateFrom(isolate(), new_contents, BROWSER_WINDOW);
Emit("-web-contents-created", api_web_contents, target_url, frame_name);
// Create V8 wrapper for the |new_contents|.
auto wrapper = CreateAndTake(isolate(), new_contents, BROWSER_WINDOW);
Emit("-web-contents-created", wrapper, target_url, frame_name);
}

void WebContents::AddNewContents(
Expand All @@ -552,7 +556,11 @@ void WebContents::AddNewContents(
new ChildWebContentsTracker(new_contents.get());
v8::Locker locker(isolate());
v8::HandleScope handle_scope(isolate());
auto api_web_contents = CreateFrom(isolate(), new_contents.release());
// Note that the ownership of |new_contents| has already been claimed by
// the WebContentsCreated method, the release call here completes
// the ownership transfer.
auto api_web_contents = From(isolate(), new_contents.release());
DCHECK(!api_web_contents.IsEmpty());
if (Emit("-add-new-contents", api_web_contents, disposition, user_gesture,
initial_rect.x(), initial_rect.y(), initial_rect.width(),
initial_rect.height())) {
Expand Down Expand Up @@ -979,8 +987,8 @@ void WebContents::DevToolsFocused() {
void WebContents::DevToolsOpened() {
v8::Locker locker(isolate());
v8::HandleScope handle_scope(isolate());
auto handle = WebContents::CreateFrom(
isolate(), managed_web_contents()->GetDevToolsWebContents());
auto handle =
FromOrCreate(isolate(), managed_web_contents()->GetDevToolsWebContents());
devtools_web_contents_.Reset(isolate(), handle.ToV8());

// Set inspected tabID.
Expand Down Expand Up @@ -2180,32 +2188,40 @@ void WebContents::OnRendererMessageTo(content::RenderFrameHost* frame_host,
}

// static
mate::Handle<WebContents> WebContents::CreateFrom(
v8::Isolate* isolate,
content::WebContents* web_contents) {
// We have an existing WebContents object in JS.
auto* existing = TrackableObject::FromWrappedClass(isolate, web_contents);
if (existing)
return mate::CreateHandle(isolate, static_cast<WebContents*>(existing));

// Otherwise create a new WebContents wrapper object.
return mate::CreateHandle(isolate,
new WebContents(isolate, web_contents, REMOTE));
mate::Handle<WebContents> WebContents::Create(v8::Isolate* isolate,
const mate::Dictionary& options) {
return mate::CreateHandle(isolate, new WebContents(isolate, options));
}

mate::Handle<WebContents> WebContents::CreateFrom(
// static
mate::Handle<WebContents> WebContents::CreateAndTake(
v8::Isolate* isolate,
content::WebContents* web_contents,
Type type) {
// Otherwise create a new WebContents wrapper object.
return mate::CreateHandle(isolate,
new WebContents(isolate, web_contents, type));
}

// static
mate::Handle<WebContents> WebContents::Create(v8::Isolate* isolate,
const mate::Dictionary& options) {
return mate::CreateHandle(isolate, new WebContents(isolate, options));
mate::Handle<WebContents> WebContents::From(
v8::Isolate* isolate,
content::WebContents* web_contents) {
auto* existing = TrackableObject::FromWrappedClass(isolate, web_contents);
if (existing)
return mate::CreateHandle(isolate, static_cast<WebContents*>(existing));
else
return mate::Handle<WebContents>();
}

// static
mate::Handle<WebContents> WebContents::FromOrCreate(
v8::Isolate* isolate,
content::WebContents* web_contents) {
auto existing = From(isolate, web_contents);
if (!existing.IsEmpty())
return existing;
else
return mate::CreateHandle(isolate, new WebContents(isolate, web_contents));
}

} // namespace api
Expand Down
31 changes: 23 additions & 8 deletions atom/browser/api/atom_api_web_contents.h
View file
Expand Up @@ -82,18 +82,29 @@ class WebContents : public mate::TrackableObject<WebContents>,
using PrintToPDFCallback =
base::Callback<void(v8::Local<v8::Value>, v8::Local<v8::Value>)>;

// Create from an existing WebContents.
static mate::Handle<WebContents> CreateFrom(
v8::Isolate* isolate,
content::WebContents* web_contents);
static mate::Handle<WebContents> CreateFrom(
// Create a new WebContents and return the V8 wrapper of it.
static mate::Handle<WebContents> Create(v8::Isolate* isolate,
const mate::Dictionary& options);

// Create a new V8 wrapper for an existing |web_content|.
//
// The lifetime of |web_contents| will be managed by this class.
static mate::Handle<WebContents> CreateAndTake(
v8::Isolate* isolate,
content::WebContents* web_contents,
Type type);

// Create a new WebContents.
static mate::Handle<WebContents> Create(v8::Isolate* isolate,
const mate::Dictionary& options);
// Get the V8 wrapper of |web_content|, return empty handle if not wrapped.
static mate::Handle<WebContents> From(v8::Isolate* isolate,
content::WebContents* web_content);

// Get the V8 wrapper of the |web_contents|, or create one if not existed.
//
// The lifetime of |web_contents| is NOT managed by this class, and the type
// of this wrapper is always REMOTE.
static mate::Handle<WebContents> FromOrCreate(
v8::Isolate* isolate,
content::WebContents* web_contents);

static void BuildPrototype(v8::Isolate* isolate,
v8::Local<v8::FunctionTemplate> prototype);
Expand Down Expand Up @@ -281,9 +292,13 @@ class WebContents : public mate::TrackableObject<WebContents>,
content::NavigationHandle* navigation_handle);

protected:
// Does not manage lifetime of |web_contents|.
WebContents(v8::Isolate* isolate, content::WebContents* web_contents);
// Takes over ownership of |web_contents|.
WebContents(v8::Isolate* isolate,
content::WebContents* web_contents,
Type type);
// Creates a new content::WebContents.
WebContents(v8::Isolate* isolate, const mate::Dictionary& options);
~WebContents() override;

Expand Down
4 changes: 2 additions & 2 deletions atom/browser/atom_navigation_throttle.cc
View file
Expand Up @@ -29,9 +29,9 @@ AtomNavigationThrottle::WillRedirectRequest() {
}

auto api_contents =
atom::api::WebContents::CreateFrom(v8::Isolate::GetCurrent(), contents);
atom::api::WebContents::From(v8::Isolate::GetCurrent(), contents);
if (api_contents.IsEmpty()) {
NOTREACHED();
// No need to emit any event if the WebContents is not available in JS.
return PROCEED;
}

Expand Down
2 changes: 1 addition & 1 deletion atom/common/native_mate_converters/content_converter.cc
View file
Expand Up @@ -206,7 +206,7 @@ v8::Local<v8::Value> Converter<content::WebContents*>::ToV8(
content::WebContents* val) {
if (!val)
return v8::Null(isolate);
return atom::api::WebContents::CreateFrom(isolate, val).ToV8();
return atom::api::WebContents::FromOrCreate(isolate, val).ToV8();
}

// static
Expand Down

0 comments on commit f9c20ff

Please sign in to comment.