Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor: remove api::WebContents::CreateFrom #15241

Merged
merged 1 commit into from Oct 19, 2018
+85 −51
Merged

refactor: remove api::WebContents::CreateFrom #15241

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 9 additions & 6 deletions atom/browser/api/atom_api_app.cc
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion atom/browser/api/atom_api_render_process_preferences.cc
View file
Open in desktop
Expand Up @@ -26,7 +26,7 @@ bool IsWebContents(v8::Isolate* isolate, content::RenderProcessHost* process) {
if (!web_contents)
return false;

auto api_web_contents = WebContents::CreateFrom(isolate, web_contents);
auto api_web_contents = WebContents::FromOrCreate(isolate, web_contents);
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
Open in desktop
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
Open in desktop
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,
deepak1556 marked this conversation as resolved.
Show resolved Hide resolved
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
Open in desktop
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
Open in desktop
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