Merge pull request electron#11370 from electron/fix_dchecks

John Kleinschmidt · web-flow · commit f10c279322fe · 2017-12-07T15:55:33.000-05:00
Fix: Re-enable some dchecks
diff --git a/atom/browser/api/atom_api_app.cc b/atom/browser/api/atom_api_app.cc
@@ -667,25 +667,33 @@ void App::OnLogin(LoginHandler* login_handler,
     login_handler->CancelAuth();
 }
 
-void App::OnCreateWindow(
+bool App::CanCreateWindow(
+    content::RenderFrameHost* opener,
+    const GURL& opener_url,
+    const GURL& opener_top_level_frame_url,
+    const GURL& source_origin,
+    content::mojom::WindowContainerType container_type,
     const GURL& target_url,
+    const content::Referrer& referrer,
     const std::string& frame_name,
     WindowOpenDisposition disposition,
-    const std::vector<std::string>& features,
+    const blink::mojom::WindowFeatures& features,
+    const std::vector<std::string>& additional_features,
     const scoped_refptr<content::ResourceRequestBody>& body,
-    content::RenderFrameHost* opener) {
+    bool user_gesture,
+    bool opener_suppressed,
+    bool* no_javascript_access) {
   v8::Locker locker(isolate());
   v8::HandleScope handle_scope(isolate());
   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,
-                                     frame_name,
-                                     disposition,
-                                     features,
-                                     body);
+    api_web_contents->OnCreateWindow(target_url, frame_name, disposition,
+                                     additional_features, body);
   }
+
+  return false;
 }
 
 void App::AllowCertificateError(
diff --git a/atom/browser/api/atom_api_app.h b/atom/browser/api/atom_api_app.h
@@ -75,15 +75,6 @@ class App : public AtomBrowserClient::Delegate,
   static void BuildPrototype(v8::Isolate* isolate,
                              v8::Local<v8::FunctionTemplate> prototype);
 
-  // Called when window with disposition needs to be created.
-  void OnCreateWindow(
-      const GURL& target_url,
-      const std::string& frame_name,
-      WindowOpenDisposition disposition,
-      const std::vector<std::string>& features,
-      const scoped_refptr<content::ResourceRequestBody>& body,
-      content::RenderFrameHost* opener);
-
 #if defined(USE_NSS_CERTS)
   void OnCertificateManagerModelCreated(
       std::unique_ptr<base::DictionaryValue> options,
@@ -152,6 +143,21 @@ class App : public AtomBrowserClient::Delegate,
       net::SSLCertRequestInfo* cert_request_info,
       net::ClientCertIdentityList client_certs,
       std::unique_ptr<content::ClientCertificateDelegate> delegate) override;
+  bool CanCreateWindow(content::RenderFrameHost* opener,
+                       const GURL& opener_url,
+                       const GURL& opener_top_level_frame_url,
+                       const GURL& source_origin,
+                       content::mojom::WindowContainerType container_type,
+                       const GURL& target_url,
+                       const content::Referrer& referrer,
+                       const std::string& frame_name,
+                       WindowOpenDisposition disposition,
+                       const blink::mojom::WindowFeatures& features,
+                       const std::vector<std::string>& additional_features,
+                       const scoped_refptr<content::ResourceRequestBody>& body,
+                       bool user_gesture,
+                       bool opener_suppressed,
+                       bool* no_javascript_access) override;
 
   // content::GpuDataManagerObserver:
   void OnGpuProcessCrashed(base::TerminationStatus status) override;
diff --git a/atom/browser/atom_browser_client.cc b/atom/browser/atom_browser_client.cc
@@ -351,8 +351,7 @@ bool AtomBrowserClient::CanCreateWindow(
     bool user_gesture,
     bool opener_suppressed,
     bool* no_javascript_access) {
-  // FIXME: Ensure the DCHECK doesn't fail and then re-enable
-  // DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   int opener_render_process_id = opener->GetProcess()->GetID();
 
@@ -373,15 +372,11 @@ bool AtomBrowserClient::CanCreateWindow(
   }
 
   if (delegate_) {
-    content::BrowserThread::PostTask(content::BrowserThread::UI, FROM_HERE,
-        base::Bind(&api::App::OnCreateWindow,
-                   base::Unretained(static_cast<api::App*>(delegate_)),
-                                    target_url,
-                                    frame_name,
-                                    disposition,
-                                    additional_features,
-                                    body,
-                                    opener));
+    return delegate_->CanCreateWindow(
+        opener, opener_url, opener_top_level_frame_url, source_origin,
+        container_type, target_url, referrer, frame_name, disposition, features,
+        additional_features, body, user_gesture, opener_suppressed,
+        no_javascript_access);
   }
 
   return false;
diff --git a/brightray/browser/browser_main_parts.cc b/brightray/browser/browser_main_parts.cc
@@ -18,6 +18,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "brightray/browser/browser_context.h"
 #include "brightray/browser/devtools_manager_delegate.h"
+#include "brightray/browser/media/media_capture_devices_dispatcher.h"
 #include "brightray/browser/web_ui_controller_factory.h"
 #include "brightray/common/application_info.h"
 #include "brightray/common/main_delegate.h"
@@ -268,6 +269,9 @@ int BrowserMainParts::PreCreateThreads() {
 #endif
 #endif
 
+  // Force MediaCaptureDevicesDispatcher to be created on UI thread.
+  MediaCaptureDevicesDispatcher::GetInstance();
+
   if (!views::LayoutProvider::Get())
     layout_provider_.reset(new views::LayoutProvider());
 
diff --git a/brightray/browser/media/media_capture_devices_dispatcher.cc b/brightray/browser/media/media_capture_devices_dispatcher.cc
@@ -44,8 +44,7 @@ MediaCaptureDevicesDispatcher::MediaCaptureDevicesDispatcher()
     : is_device_enumeration_disabled_(false) {
   // MediaCaptureDevicesDispatcher is a singleton. It should be created on
   // UI thread.
-  // FIXME: Ensure the DCHECK doesn't fail and then re-enable
-  // DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 }
 
 MediaCaptureDevicesDispatcher::~MediaCaptureDevicesDispatcher() {}

Original file line number	Diff line number	Diff line change
`@@ -44,8 +44,7 @@ MediaCaptureDevicesDispatcher::MediaCaptureDevicesDispatcher()`
`44`	`44`	`: is_device_enumeration_disabled_(false) {`
`45`	`45`	`// MediaCaptureDevicesDispatcher is a singleton. It should be created on`
`46`	`46`	`// UI thread.`
`47`		`- // FIXME: Ensure the DCHECK doesn't fail and then re-enable`
`48`		`- // DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));`
	`47`	`+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));`
`49`	`48`	`}`
`50`	`49`
`51`	`50`	`MediaCaptureDevicesDispatcher::~MediaCaptureDevicesDispatcher() {}`