From 8dcb5662e3b1cff27f6e1dd8d156ccff4d9be57b Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Sat, 2 Mar 2019 01:53:47 -0800 Subject: [PATCH 01/22] refactor: remove chromium/net_url_request_job.patch (#17174) Does not appear to be used any more --- patches/common/chromium/.patches | 1 - .../common/chromium/net_url_request_job.patch | 18 ------------------ 2 files changed, 19 deletions(-) delete mode 100644 patches/common/chromium/net_url_request_job.patch diff --git a/patches/common/chromium/.patches b/patches/common/chromium/.patches index db4306f2ac398..5228b559cfd7f 100644 --- a/patches/common/chromium/.patches +++ b/patches/common/chromium/.patches @@ -11,7 +11,6 @@ compositor_delegate.patch disable_hidden.patch dom_storage_limits.patch frame_host_manager.patch -net_url_request_job.patch out_of_process_instance.patch render_widget_host_view_base.patch render_widget_host_view_mac.patch diff --git a/patches/common/chromium/net_url_request_job.patch b/patches/common/chromium/net_url_request_job.patch deleted file mode 100644 index 3529d52f7e620..0000000000000 --- a/patches/common/chromium/net_url_request_job.patch +++ /dev/null @@ -1,18 +0,0 @@ -From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 -From: Cheng Zhao -Date: Thu, 20 Sep 2018 17:46:06 -0700 -Subject: net_url_request_job.patch - - -diff --git a/net/url_request/url_request_job.h b/net/url_request/url_request_job.h -index f08e6e890ebc6b7f745b3dfab6415cc9bdce050f..c05a3b2ec3020dff845dbb79737a95bd17a2766f 100644 ---- a/net/url_request/url_request_job.h -+++ b/net/url_request/url_request_job.h -@@ -294,6 +294,7 @@ class NET_EXPORT URLRequestJob : public base::PowerObserver { - void OnCallToDelegate(NetLogEventType type); - void OnCallToDelegateComplete(); - -+ public: - // Called to read raw (pre-filtered) data from this Job. Reads at most - // |buf_size| bytes into |buf|. - // Possible return values: From ed31cfebc9ccdc713290d46ed62a752ddfb361eb Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Mon, 4 Mar 2019 09:47:59 -0600 Subject: [PATCH 02/22] fix: check for pane focus before removing it. (#17164) Fixes #16883. This bug seems to have been introduced in the #15302's menu a11y refactor and is new in 5-0-x. --- atom/browser/ui/views/menu_bar.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/atom/browser/ui/views/menu_bar.cc b/atom/browser/ui/views/menu_bar.cc index 18bb5c23278fd..7207685fd1214 100644 --- a/atom/browser/ui/views/menu_bar.cc +++ b/atom/browser/ui/views/menu_bar.cc @@ -119,7 +119,9 @@ bool MenuBar::GetMenuButtonFromScreenPoint(const gfx::Point& screenPoint, } void MenuBar::OnBeforeExecuteCommand() { - RemovePaneFocus(); + if (GetPaneFocusTraversable() != nullptr) { + RemovePaneFocus(); + } window_->RestoreFocus(); } From 793623767782f14beddf3da6d5a1fbe1bcd26953 Mon Sep 17 00:00:00 2001 From: Robo Date: Tue, 5 Mar 2019 10:38:55 +0530 Subject: [PATCH 03/22] build: enable gn check for //electron:electron_lib (#17100) * build: enable gn check for //electron:electron_lib * ci: add gn check step * ci: set depot_tools path * chrome_key_systems_provider.h nogncheck * chore: fix gn check errors on windows * chore: gn check //electron:electron_app --- .circleci/config.yml | 103 ++++++++++++++++++ BUILD.gn | 48 +++++--- appveyor.yml | 2 + atom/browser/api/atom_api_app.cc | 4 +- atom/browser/api/atom_api_browser_window.cc | 4 +- atom/browser/api/atom_api_session.cc | 2 +- atom/browser/api/atom_api_web_contents.cc | 8 +- .../browser/api/atom_api_web_contents_impl.cc | 6 +- atom/browser/api/gpuinfo_manager.h | 2 +- atom/browser/atom_blob_reader.cc | 2 +- atom/browser/atom_browser_context.cc | 2 +- atom/browser/common_web_contents_delegate.cc | 7 +- .../browser/loader/layered_resource_handler.h | 2 +- .../browser/net/url_request_context_getter.cc | 2 +- .../ui/cocoa/atom_ns_window_delegate.h | 2 +- atom/browser/web_view_guest_delegate.cc | 2 +- atom/renderer/renderer_client_base.cc | 2 +- atom/renderer/renderer_client_base.h | 2 +- chromium_src/BUILD.gn | 18 +-- 19 files changed, 172 insertions(+), 48 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 1eecbec20ef0b..b4d70777cd55e 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -231,6 +231,14 @@ step-gn-gen-default: &step-gn-gen-default cd src gn gen out/Default --args='import("'$GN_CONFIG'") cc_wrapper="'"$SCCACHE_PATH"'"'" $GN_EXTRA_ARGS" +step-gn-check: &step-gn-check + run: + name: GN check + command: | + cd src + gn check out/Default //electron:electron_lib + gn check out/Default //electron:electron_app + step-electron-build: &step-electron-build run: name: Electron build @@ -568,6 +576,15 @@ steps-checkout: &steps-checkout - depot_tools - src +steps-electron-gn-check: &steps-electron-gn-check + steps: + - attach_workspace: + at: . + - *step-depot-tools-add-to-path + - *step-setup-env-for-build + - *step-gn-gen-default + - *step-gn-check + steps-electron-build: &steps-electron-build steps: - attach_workspace: @@ -857,6 +874,13 @@ jobs: <<: *env-enable-sccache <<: *steps-electron-build + linux-x64-debug-gn-check: + <<: *machine-linux-medium + environment: + <<: *env-linux-medium + <<: *env-debug-build + <<: *steps-electron-gn-check + linux-x64-testing: <<: *machine-linux-2xlarge environment: @@ -865,6 +889,13 @@ jobs: <<: *env-enable-sccache <<: *steps-electron-build-for-tests + linux-x64-testing-gn-check: + <<: *machine-linux-medium + environment: + <<: *env-linux-medium + <<: *env-testing-build + <<: *steps-electron-gn-check + linux-x64-chromedriver: <<: *machine-linux-medium environment: @@ -1005,6 +1036,14 @@ jobs: <<: *env-enable-sccache <<: *steps-electron-build + linux-arm64-debug-gn-check: + <<: *machine-linux-medium + environment: + <<: *env-linux-medium + <<: *env-arm64 + <<: *env-debug-build + <<: *steps-electron-gn-check + linux-arm64-testing: <<: *machine-linux-2xlarge environment: @@ -1015,6 +1054,14 @@ jobs: TRIGGER_ARM_TEST: true <<: *steps-electron-build-for-tests + linux-arm64-testing-gn-check: + <<: *machine-linux-medium + environment: + <<: *env-linux-medium + <<: *env-arm64 + <<: *env-testing-build + <<: *steps-electron-gn-check + linux-arm64-chromedriver: <<: *machine-linux-medium environment: @@ -1062,6 +1109,20 @@ jobs: <<: *env-enable-sccache <<: *steps-electron-build-for-tests + osx-debug-gn-check: + <<: *machine-mac + environment: + <<: *env-machine-mac + <<: *env-debug-build + <<: *steps-electron-gn-check + + osx-testing-gn-check: + <<: *machine-mac + environment: + <<: *env-machine-mac + <<: *env-testing-build + <<: *steps-electron-gn-check + osx-chromedriver: <<: *machine-mac environment: @@ -1096,6 +1157,22 @@ jobs: <<: *env-enable-sccache <<: *steps-electron-build-for-tests + mas-debug-gn-check: + <<: *machine-mac + environment: + <<: *env-machine-mac + <<: *env-mas + <<: *env-debug-build + <<: *steps-electron-gn-check + + mas-testing-gn-check: + <<: *machine-mac + environment: + <<: *env-machine-mac + <<: *env-mas + <<: *env-testing-build + <<: *steps-electron-gn-check + mas-chromedriver: <<: *machine-mac environment: @@ -1353,9 +1430,15 @@ workflows: - linux-x64-debug: requires: - linux-checkout + - linux-x64-debug-gn-check: + requires: + - linux-checkout - linux-x64-testing: requires: - linux-checkout + - linux-x64-testing-gn-check: + requires: + - linux-checkout - linux-x64-testing-tests: requires: - linux-x64-testing @@ -1380,9 +1463,15 @@ workflows: - linux-arm64-debug: requires: - linux-checkout + - linux-arm64-debug-gn-check: + requires: + - linux-checkout - linux-arm64-testing: requires: - linux-checkout + - linux-arm64-testing-gn-check: + requires: + - linux-checkout build-mac: jobs: @@ -1391,6 +1480,13 @@ workflows: requires: - mac-checkout + - osx-debug-gn-check: + requires: + - mac-checkout + - osx-testing-gn-check: + requires: + - mac-checkout + - osx-testing-tests: requires: - osx-testing @@ -1399,6 +1495,13 @@ workflows: requires: - mac-checkout + - mas-debug-gn-check: + requires: + - mac-checkout + - mas-testing-gn-check: + requires: + - mac-checkout + - mas-testing-tests: requires: - mas-testing diff --git a/BUILD.gn b/BUILD.gn index 6bd3938a2cd5a..c4f0c3f62b9d3 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -326,13 +326,13 @@ static_library("electron_lib") { deps = [ ":atom_js2c", + ":resources", "buildflags", "chromium_src:chrome", "manifests", "native_mate", - "//base", "//base:base_static", - "//base:i18n", + "//base/allocator:buildflags", "//chrome/app/resources:platform_locale_settings", "//components/certificate_transparency", "//components/net_log", @@ -341,10 +341,12 @@ static_library("electron_lib") { "//components/spellcheck/renderer", "//components/viz/host", "//components/viz/service", - "//content/public/app:both", "//content/public/browser", "//content/public/child", "//content/public/common:service_names", + "//content/public/renderer", + "//content/public/utility", + "//device/bluetooth", "//gin", "//media/capture/mojom:video_capture", "//media/mojo/interfaces", @@ -354,6 +356,7 @@ static_library("electron_lib") { "//ppapi/host", "//ppapi/proxy", "//ppapi/shared_impl", + "//printing/buildflags", "//services/audio/public/mojom:constants", "//services/device/public/mojom", "//services/proxy_resolver:lib", @@ -369,8 +372,17 @@ static_library("electron_lib") { "//third_party/widevine/cdm:headers", "//ui/events:dom_keycode_converter", "//ui/gl", + "//ui/native_theme", + "//ui/shell_dialogs", "//ui/views", "//v8", + "//v8:v8_libplatform", + ] + + public_deps = [ + "//base", + "//base:i18n", + "//content/public/app:both", ] include_dirs = [ @@ -435,6 +447,7 @@ static_library("electron_lib") { "atom/browser/fake_location_provider.cc", "atom/browser/fake_location_provider.h", ] + deps += [ "//services/device/public/cpp/geolocation" ] } if (is_mac) { @@ -485,9 +498,6 @@ static_library("electron_lib") { "//device/bluetooth", "//ui/events/devices/x11", "//ui/events/platform/x11", - "//ui/native_theme", - "//ui/views/controls/webview", - "//ui/wm", ] configs += [ ":gio_unix" ] defines += [ @@ -499,9 +509,20 @@ static_library("electron_lib") { } if (is_win) { libs += [ "dwmapi.lib" ] + deps += [ + "//third_party/breakpad:breakpad_handler", + "//third_party/breakpad:breakpad_sender", + "//ui/native_theme:native_theme_browser", + "//ui/wm/public", + ] + public_deps += [ "//sandbox/win:sandbox" ] } if (is_linux || is_win) { - deps += [ "//third_party/breakpad:client" ] + deps += [ + "//third_party/breakpad:client", + "//ui/views/controls/webview", + "//ui/wm", + ] include_dirs += [ "//third_party/breakpad" ] } @@ -866,6 +887,7 @@ if (is_mac) { ":electron_lib", ":packed_resources", "//content:sandbox_helper_win", + "//electron/buildflags", "//ui/strings", ] @@ -894,17 +916,7 @@ if (is_mac) { # TODO: we should be generating our .rc files more like how chrome does "atom/browser/resources/win/atom.ico", "atom/browser/resources/win/atom.rc", - "atom/browser/resources/win/resources.h", - ] - - deps += [ - "//third_party/breakpad:breakpad_handler", - "//third_party/breakpad:breakpad_sender", - "//ui/native_theme:native_theme_browser", - "//ui/shell_dialogs", - "//ui/views/controls/webview", - "//ui/wm", - "//ui/wm/public", + "atom/browser/resources/win/resource.h", ] libs = [ diff --git a/appveyor.yml b/appveyor.yml index d279c26474648..b04dc0c3318b0 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -56,6 +56,8 @@ build_script: - cd src - ps: $env:BUILD_CONFIG_PATH="//electron/build/args/%GN_CONFIG%.gn" - gn gen out/Default "--args=import(\"%BUILD_CONFIG_PATH%\") %GN_EXTRA_ARGS%" + - gn check out/Default //electron:electron_lib + - gn check out/Default //electron:electron_app - ninja -C out/Default electron:electron_app - gn gen out/ffmpeg "--args=import(\"//electron/build/args/ffmpeg.gn\") %GN_EXTRA_ARGS%" - ninja -C out/ffmpeg electron:electron_ffmpeg_zip diff --git a/atom/browser/api/atom_api_app.cc b/atom/browser/api/atom_api_app.cc index 41bfa5f26ced1..ad07393178cd4 100644 --- a/atom/browser/api/atom_api_app.cc +++ b/atom/browser/api/atom_api_app.cc @@ -34,8 +34,8 @@ #include "chrome/browser/browser_process.h" #include "chrome/browser/icon_manager.h" #include "chrome/common/chrome_paths.h" -#include "content/browser/gpu/compositor_util.h" -#include "content/browser/gpu/gpu_data_manager_impl.h" +#include "content/browser/gpu/compositor_util.h" // nogncheck +#include "content/browser/gpu/gpu_data_manager_impl.h" // nogncheck #include "content/public/browser/browser_accessibility_state.h" #include "content/public/browser/browser_child_process_host.h" #include "content/public/browser/child_process_data.h" diff --git a/atom/browser/api/atom_api_browser_window.cc b/atom/browser/api/atom_api_browser_window.cc index 7c7afe18e5878..80e23ebf76a09 100644 --- a/atom/browser/api/atom_api_browser_window.cc +++ b/atom/browser/api/atom_api_browser_window.cc @@ -17,8 +17,8 @@ #include "atom/common/native_mate_converters/value_converter.h" #include "atom/common/options_switches.h" #include "base/threading/thread_task_runner_handle.h" -#include "content/browser/renderer_host/render_widget_host_impl.h" -#include "content/browser/renderer_host/render_widget_host_owner_delegate.h" +#include "content/browser/renderer_host/render_widget_host_impl.h" // nogncheck +#include "content/browser/renderer_host/render_widget_host_owner_delegate.h" // nogncheck #include "content/public/browser/render_process_host.h" #include "content/public/browser/render_view_host.h" #include "gin/converter.h" diff --git a/atom/browser/api/atom_api_session.cc b/atom/browser/api/atom_api_session.cc index 021c8e862b1c4..e5e3809859475 100644 --- a/atom/browser/api/atom_api_session.cc +++ b/atom/browser/api/atom_api_session.cc @@ -49,7 +49,7 @@ #include "native_mate/object_template_builder.h" #include "net/base/load_flags.h" #include "net/disk_cache/disk_cache.h" -#include "net/dns/host_cache.h" +#include "net/dns/host_cache.h" // nogncheck #include "net/http/http_auth_handler_factory.h" #include "net/http/http_auth_preferences.h" #include "net/http/http_cache.h" diff --git a/atom/browser/api/atom_api_web_contents.cc b/atom/browser/api/atom_api_web_contents.cc index 47c95ca5074c8..fd6eb0d6058d1 100644 --- a/atom/browser/api/atom_api_web_contents.cc +++ b/atom/browser/api/atom_api_web_contents.cc @@ -54,10 +54,10 @@ #include "base/values.h" #include "chrome/browser/browser_process.h" #include "chrome/browser/ssl/security_state_tab_helper.h" -#include "content/browser/frame_host/frame_tree_node.h" -#include "content/browser/frame_host/render_frame_host_manager.h" -#include "content/browser/renderer_host/render_widget_host_impl.h" -#include "content/browser/renderer_host/render_widget_host_view_base.h" +#include "content/browser/frame_host/frame_tree_node.h" // nogncheck +#include "content/browser/frame_host/render_frame_host_manager.h" // nogncheck +#include "content/browser/renderer_host/render_widget_host_impl.h" // nogncheck +#include "content/browser/renderer_host/render_widget_host_view_base.h" // nogncheck #include "content/common/widget_messages.h" #include "content/public/browser/child_process_security_policy.h" #include "content/public/browser/download_request_utils.h" diff --git a/atom/browser/api/atom_api_web_contents_impl.cc b/atom/browser/api/atom_api_web_contents_impl.cc index ccb7086700bf1..30448ff2d5af6 100644 --- a/atom/browser/api/atom_api_web_contents_impl.cc +++ b/atom/browser/api/atom_api_web_contents_impl.cc @@ -4,9 +4,9 @@ #include "atom/browser/api/atom_api_web_contents.h" -#include "content/browser/frame_host/frame_tree.h" -#include "content/browser/frame_host/frame_tree_node.h" -#include "content/browser/web_contents/web_contents_impl.h" +#include "content/browser/frame_host/frame_tree.h" // nogncheck +#include "content/browser/frame_host/frame_tree_node.h" // nogncheck +#include "content/browser/web_contents/web_contents_impl.h" // nogncheck #include "content/public/browser/guest_mode.h" #if BUILDFLAG(ENABLE_OSR) diff --git a/atom/browser/api/gpuinfo_manager.h b/atom/browser/api/gpuinfo_manager.h index 18749f6f562a6..6fcb08a19621a 100644 --- a/atom/browser/api/gpuinfo_manager.h +++ b/atom/browser/api/gpuinfo_manager.h @@ -11,7 +11,7 @@ #include "atom/common/native_mate_converters/value_converter.h" #include "atom/common/promise_util.h" -#include "content/browser/gpu/gpu_data_manager_impl.h" +#include "content/browser/gpu/gpu_data_manager_impl.h" // nogncheck #include "content/public/browser/gpu_data_manager.h" #include "content/public/browser/gpu_data_manager_observer.h" diff --git a/atom/browser/atom_blob_reader.cc b/atom/browser/atom_blob_reader.cc index 1796b81a63274..cfb89bc8de1c0 100644 --- a/atom/browser/atom_blob_reader.cc +++ b/atom/browser/atom_blob_reader.cc @@ -7,7 +7,7 @@ #include #include "base/task/post_task.h" -#include "content/browser/blob_storage/chrome_blob_storage_context.h" +#include "content/browser/blob_storage/chrome_blob_storage_context.h" // nogncheck #include "content/public/browser/browser_task_traits.h" #include "content/public/browser/browser_thread.h" #include "net/base/io_buffer.h" diff --git a/atom/browser/atom_browser_context.cc b/atom/browser/atom_browser_context.cc index d8e805d4cdb06..99c274ecd7bb0 100644 --- a/atom/browser/atom_browser_context.cc +++ b/atom/browser/atom_browser_context.cc @@ -36,7 +36,7 @@ #include "components/prefs/value_map_pref_store.h" #include "components/proxy_config/pref_proxy_config_tracker_impl.h" #include "components/proxy_config/proxy_config_pref_names.h" -#include "content/browser/blob_storage/chrome_blob_storage_context.h" +#include "content/browser/blob_storage/chrome_blob_storage_context.h" // nogncheck #include "content/public/browser/browser_thread.h" #include "content/public/browser/storage_partition.h" #include "net/base/escape.h" diff --git a/atom/browser/common_web_contents_delegate.cc b/atom/browser/common_web_contents_delegate.cc index 112b5a179becd..827bbc3b4c706 100644 --- a/atom/browser/common_web_contents_delegate.cc +++ b/atom/browser/common_web_contents_delegate.cc @@ -24,13 +24,12 @@ #include "base/threading/scoped_blocking_call.h" #include "base/threading/sequenced_task_runner_handle.h" #include "chrome/browser/ssl/security_state_tab_helper.h" -#include "chrome/browser/ui/color_chooser.h" #include "chrome/common/pref_names.h" #include "components/prefs/pref_service.h" #include "components/prefs/scoped_user_pref_update.h" #include "components/security_state/content/content_utils.h" #include "components/security_state/core/security_state.h" -#include "content/browser/renderer_host/render_widget_host_view_base.h" +#include "content/browser/renderer_host/render_widget_host_view_base.h" // nogncheck #include "content/public/browser/browser_thread.h" #include "content/public/browser/child_process_security_policy.h" #include "content/public/browser/file_select_listener.h" @@ -42,6 +41,10 @@ #include "printing/buildflags/buildflags.h" #include "storage/browser/fileapi/isolated_context.h" +#if BUILDFLAG(ENABLE_COLOR_CHOOSER) +#include "chrome/browser/ui/color_chooser.h" +#endif + #if BUILDFLAG(ENABLE_OSR) #include "atom/browser/osr/osr_web_contents_view.h" #endif diff --git a/atom/browser/loader/layered_resource_handler.h b/atom/browser/loader/layered_resource_handler.h index ba1ad16e90d85..78bb275050e2f 100644 --- a/atom/browser/loader/layered_resource_handler.h +++ b/atom/browser/loader/layered_resource_handler.h @@ -7,7 +7,7 @@ #include -#include "content/browser/loader/layered_resource_handler.h" +#include "content/browser/loader/layered_resource_handler.h" // nogncheck #include "services/network/public/cpp/resource_response.h" namespace atom { diff --git a/atom/browser/net/url_request_context_getter.cc b/atom/browser/net/url_request_context_getter.cc index 70d8c8b02a822..705f1f2b85d6c 100644 --- a/atom/browser/net/url_request_context_getter.cc +++ b/atom/browser/net/url_request_context_getter.cc @@ -35,7 +35,7 @@ #include "net/base/host_mapping_rules.h" #include "net/cert/multi_log_ct_verifier.h" #include "net/cookies/cookie_monster.h" -#include "net/dns/mapped_host_resolver.h" +#include "net/dns/mapped_host_resolver.h" // nogncheck #include "net/http/http_auth_handler_factory.h" #include "net/http/http_auth_preferences.h" #include "net/http/http_auth_scheme.h" diff --git a/atom/browser/ui/cocoa/atom_ns_window_delegate.h b/atom/browser/ui/cocoa/atom_ns_window_delegate.h index 32a37d06921b3..c38affd20bc19 100644 --- a/atom/browser/ui/cocoa/atom_ns_window_delegate.h +++ b/atom/browser/ui/cocoa/atom_ns_window_delegate.h @@ -7,7 +7,7 @@ #include -#include "ui/views_bridge_mac/views_nswindow_delegate.h" +#include "ui/views_bridge_mac/views_nswindow_delegate.h" // nogncheck namespace atom { class NativeWindowMac; diff --git a/atom/browser/web_view_guest_delegate.cc b/atom/browser/web_view_guest_delegate.cc index 01cb63ba39c8b..d139e88e69541 100644 --- a/atom/browser/web_view_guest_delegate.cc +++ b/atom/browser/web_view_guest_delegate.cc @@ -8,7 +8,7 @@ #include "atom/browser/api/atom_api_web_contents.h" #include "atom/common/native_mate_converters/gurl_converter.h" -#include "content/browser/web_contents/web_contents_impl.h" +#include "content/browser/web_contents/web_contents_impl.h" // nogncheck #include "content/public/browser/navigation_handle.h" #include "content/public/browser/render_frame_host.h" #include "content/public/browser/render_process_host.h" diff --git a/atom/renderer/renderer_client_base.cc b/atom/renderer/renderer_client_base.cc index d7682a21500ab..037a97b6e061b 100644 --- a/atom/renderer/renderer_client_base.cc +++ b/atom/renderer/renderer_client_base.cc @@ -33,7 +33,7 @@ #include "third_party/blink/public/web/web_script_source.h" #include "third_party/blink/public/web/web_security_policy.h" #include "third_party/blink/public/web/web_view.h" -#include "third_party/blink/renderer/platform/weborigin/scheme_registry.h" +#include "third_party/blink/renderer/platform/weborigin/scheme_registry.h" // nogncheck #if defined(OS_MACOSX) #include "base/strings/sys_string_conversions.h" diff --git a/atom/renderer/renderer_client_base.h b/atom/renderer/renderer_client_base.h index 508ad391ca9df..3ec129a79c362 100644 --- a/atom/renderer/renderer_client_base.h +++ b/atom/renderer/renderer_client_base.h @@ -15,7 +15,7 @@ #include "widevine_cdm_version.h" // NOLINT(build/include) #if defined(WIDEVINE_CDM_AVAILABLE) -#include "chrome/renderer/media/chrome_key_systems_provider.h" +#include "chrome/renderer/media/chrome_key_systems_provider.h" // nogncheck #endif namespace atom { diff --git a/chromium_src/BUILD.gn b/chromium_src/BUILD.gn index cf86fe197478a..732a13b43571c 100644 --- a/chromium_src/BUILD.gn +++ b/chromium_src/BUILD.gn @@ -49,14 +49,15 @@ static_library("chrome") { "//extensions/browser/app_window/size_constraints.h", ] public_deps = [ - "//content/public/browser", - ] - deps = [ "//chrome/common", - "//components/feature_engagement:buildflags", + "//chrome/common:version_header", "//components/keyed_service/content", "//components/proxy_config", "//components/security_state/content", + "//content/public/browser", + ] + deps = [ + "//components/feature_engagement:buildflags", ] if (is_linux) { @@ -85,6 +86,7 @@ static_library("chrome") { "//chrome/browser/platform_util.cc", "//chrome/browser/platform_util.h", "//chrome/browser/ui/browser_dialogs.h", + "//chrome/browser/ui/color_chooser.h", ] if (use_aura) { @@ -156,14 +158,16 @@ static_library("chrome") { "//chrome/browser/printing/printing_message_filter.cc", "//chrome/browser/printing/printing_message_filter.h", ] - deps += [ + public_deps += [ "//chrome/services/printing:lib", "//components/printing/browser", - "//components/printing/common", "//components/printing/renderer", - "//components/services/pdf_compositor", "//components/services/pdf_compositor/public/cpp:factory", "//components/services/pdf_compositor/public/interfaces", + ] + deps += [ + "//components/printing/common", + "//components/services/pdf_compositor", "//printing", ] From e05985145bc8599b439636bce93ffd8be1b9379a Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Tue, 5 Mar 2019 05:54:48 -0800 Subject: [PATCH 04/22] feat: promisify dialog.showOpenDialog() (#16973) * feat: promisify dialog.showOpenDialog() * address feedback from review * address feedback from review --- atom/browser/api/atom_api_dialog.cc | 28 +++--- atom/browser/common_web_contents_delegate.cc | 2 +- atom/browser/ui/file_dialog.h | 18 ++-- atom/browser/ui/file_dialog_gtk.cc | 33 ++++--- atom/browser/ui/file_dialog_mac.mm | 48 +++++----- atom/browser/ui/file_dialog_win.cc | 38 +++++--- atom/browser/web_dialog_helper.cc | 7 +- docs/api/dialog.md | 89 ++++++++++++++++-- docs/api/promisification.md | 2 +- lib/browser/api/dialog.js | 98 +++++++++----------- lib/browser/chrome-devtools.js | 6 +- spec/ts-smoke/electron/main.ts | 6 +- 12 files changed, 228 insertions(+), 147 deletions(-) diff --git a/atom/browser/api/atom_api_dialog.cc b/atom/browser/api/atom_api_dialog.cc index 623f69ae27c4e..2958b7273b1db 100644 --- a/atom/browser/api/atom_api_dialog.cc +++ b/atom/browser/api/atom_api_dialog.cc @@ -16,6 +16,7 @@ #include "atom/common/native_mate_converters/file_path_converter.h" #include "atom/common/native_mate_converters/image_converter.h" #include "atom/common/native_mate_converters/net_converter.h" +#include "atom/common/promise_util.h" #include "native_mate/dictionary.h" #include "atom/common/node_includes.h" @@ -51,18 +52,20 @@ void ShowMessageBox(int type, } } -void ShowOpenDialog(const file_dialog::DialogSettings& settings, - mate::Arguments* args) { - v8::Local peek = args->PeekNext(); - file_dialog::OpenDialogCallback callback; - if (mate::Converter::FromV8( - args->isolate(), peek, &callback)) { - file_dialog::ShowOpenDialog(settings, callback); - } else { - std::vector paths; - if (file_dialog::ShowOpenDialog(settings, &paths)) - args->Return(paths); - } +void ShowOpenDialogSync(const file_dialog::DialogSettings& settings, + mate::Arguments* args) { + std::vector paths; + if (file_dialog::ShowOpenDialogSync(settings, &paths)) + args->Return(paths); +} + +v8::Local ShowOpenDialog( + const file_dialog::DialogSettings& settings, + mate::Arguments* args) { + atom::util::Promise promise(args->isolate()); + v8::Local handle = promise.GetHandle(); + file_dialog::ShowOpenDialog(settings, std::move(promise)); + return handle; } void ShowSaveDialog(const file_dialog::DialogSettings& settings, @@ -86,6 +89,7 @@ void Initialize(v8::Local exports, mate::Dictionary dict(context->GetIsolate(), exports); dict.SetMethod("showMessageBox", &ShowMessageBox); dict.SetMethod("showErrorBox", &atom::ShowErrorBox); + dict.SetMethod("showOpenDialogSync", &ShowOpenDialogSync); dict.SetMethod("showOpenDialog", &ShowOpenDialog); dict.SetMethod("showSaveDialog", &ShowSaveDialog); #if defined(OS_MACOSX) || defined(OS_WIN) diff --git a/atom/browser/common_web_contents_delegate.cc b/atom/browser/common_web_contents_delegate.cc index 827bbc3b4c706..e5e6e196ba109 100644 --- a/atom/browser/common_web_contents_delegate.cc +++ b/atom/browser/common_web_contents_delegate.cc @@ -454,7 +454,7 @@ void CommonWebContentsDelegate::DevToolsAddFileSystem( settings.parent_window = owner_window(); settings.force_detached = offscreen_; settings.properties = file_dialog::FILE_DIALOG_OPEN_DIRECTORY; - if (!file_dialog::ShowOpenDialog(settings, &paths)) + if (!file_dialog::ShowOpenDialogSync(settings, &paths)) return; path = paths[0]; diff --git a/atom/browser/ui/file_dialog.h b/atom/browser/ui/file_dialog.h index becc1806610ae..6666b369c84aa 100644 --- a/atom/browser/ui/file_dialog.h +++ b/atom/browser/ui/file_dialog.h @@ -9,8 +9,11 @@ #include #include +#include "atom/common/native_mate_converters/file_path_converter.h" +#include "atom/common/promise_util.h" #include "base/callback_forward.h" #include "base/files/file_path.h" +#include "native_mate/dictionary.h" namespace atom { class NativeWindow; @@ -34,20 +37,11 @@ enum FileDialogProperty { }; #if defined(MAS_BUILD) -typedef base::Callback& paths, - const std::vector& bookmarkData)> - OpenDialogCallback; - typedef base::Callback SaveDialogCallback; #else -typedef base::Callback& paths)> - OpenDialogCallback; - typedef base::Callback SaveDialogCallback; #endif @@ -70,11 +64,11 @@ struct DialogSettings { ~DialogSettings(); }; -bool ShowOpenDialog(const DialogSettings& settings, - std::vector* paths); +bool ShowOpenDialogSync(const DialogSettings& settings, + std::vector* paths); void ShowOpenDialog(const DialogSettings& settings, - const OpenDialogCallback& callback); + atom::util::Promise promise); bool ShowSaveDialog(const DialogSettings& settings, base::FilePath* path); diff --git a/atom/browser/ui/file_dialog_gtk.cc b/atom/browser/ui/file_dialog_gtk.cc index 20f9d2b38c85b..75447c46e6261 100644 --- a/atom/browser/ui/file_dialog_gtk.cc +++ b/atom/browser/ui/file_dialog_gtk.cc @@ -131,8 +131,8 @@ class FileChooserDialog { RunAsynchronous(); } - void RunOpenAsynchronous(const OpenDialogCallback& callback) { - open_callback_ = callback; + void RunOpenAsynchronous(atom::util::Promise promise) { + open_promise_.reset(new atom::util::Promise(std::move(promise))); RunAsynchronous(); } @@ -174,7 +174,7 @@ class FileChooserDialog { Filters filters_; SaveDialogCallback save_callback_; - OpenDialogCallback open_callback_; + std::unique_ptr open_promise_; // Callback for when we update the preview for the selection. CHROMEG_CALLBACK_0(FileChooserDialog, void, OnUpdatePreview, GtkWidget*); @@ -190,11 +190,17 @@ void FileChooserDialog::OnFileDialogResponse(GtkWidget* widget, int response) { save_callback_.Run(true, GetFileName()); else save_callback_.Run(false, base::FilePath()); - } else if (!open_callback_.is_null()) { - if (response == GTK_RESPONSE_ACCEPT) - open_callback_.Run(true, GetFileNames()); - else - open_callback_.Run(false, std::vector()); + } else if (open_promise_) { + mate::Dictionary dict = + mate::Dictionary::CreateEmpty(open_promise_->isolate()); + if (response == GTK_RESPONSE_ACCEPT) { + dict.Set("canceled", false); + dict.Set("filePaths", GetFileNames()); + } else { + dict.Set("canceled", true); + dict.Set("filePaths", std::vector()); + } + open_promise_->Resolve(dict.GetHandle()); } delete this; } @@ -252,8 +258,8 @@ void FileChooserDialog::OnUpdatePreview(GtkWidget* chooser) { } // namespace -bool ShowOpenDialog(const DialogSettings& settings, - std::vector* paths) { +bool ShowOpenDialogSync(const DialogSettings& settings, + std::vector* paths) { GtkFileChooserAction action = GTK_FILE_CHOOSER_ACTION_OPEN; if (settings.properties & FILE_DIALOG_OPEN_DIRECTORY) action = GTK_FILE_CHOOSER_ACTION_SELECT_FOLDER; @@ -265,19 +271,18 @@ bool ShowOpenDialog(const DialogSettings& settings, if (response == GTK_RESPONSE_ACCEPT) { *paths = open_dialog.GetFileNames(); return true; - } else { - return false; } + return false; } void ShowOpenDialog(const DialogSettings& settings, - const OpenDialogCallback& callback) { + atom::util::Promise promise) { GtkFileChooserAction action = GTK_FILE_CHOOSER_ACTION_OPEN; if (settings.properties & FILE_DIALOG_OPEN_DIRECTORY) action = GTK_FILE_CHOOSER_ACTION_SELECT_FOLDER; FileChooserDialog* open_dialog = new FileChooserDialog(action, settings); open_dialog->SetupProperties(settings.properties); - open_dialog->RunOpenAsynchronous(callback); + open_dialog->RunOpenAsynchronous(std::move(promise)); } bool ShowSaveDialog(const DialogSettings& settings, base::FilePath* path) { diff --git a/atom/browser/ui/file_dialog_mac.mm b/atom/browser/ui/file_dialog_mac.mm index e2cfd96fa6e28..fb0b61368febe 100644 --- a/atom/browser/ui/file_dialog_mac.mm +++ b/atom/browser/ui/file_dialog_mac.mm @@ -268,8 +268,8 @@ void ReadDialogPaths(NSOpenPanel* dialog, std::vector* paths) { } // namespace -bool ShowOpenDialog(const DialogSettings& settings, - std::vector* paths) { +bool ShowOpenDialogSync(const DialogSettings& settings, + std::vector* paths) { DCHECK(paths); NSOpenPanel* dialog = [NSOpenPanel openPanel]; @@ -287,58 +287,62 @@ bool ShowOpenDialog(const DialogSettings& settings, void OpenDialogCompletion(int chosen, NSOpenPanel* dialog, bool security_scoped_bookmarks, - const OpenDialogCallback& callback) { + atom::util::Promise promise) { + mate::Dictionary dict = mate::Dictionary::CreateEmpty(promise.isolate()); if (chosen == NSFileHandlingPanelCancelButton) { + dict.Set("canceled", true); + dict.Set("filePaths", std::vector()); #if defined(MAS_BUILD) - callback.Run(false, std::vector(), - std::vector()); -#else - callback.Run(false, std::vector()); + dict.Set("bookmarks", std::vector()); #endif + promise.Resolve(dict.GetHandle()); } else { std::vector paths; + dict.Set("canceled", false); #if defined(MAS_BUILD) std::vector bookmarks; - if (security_scoped_bookmarks) { + if (security_scoped_bookmarks) ReadDialogPathsWithBookmarks(dialog, &paths, &bookmarks); - } else { + else ReadDialogPaths(dialog, &paths); - } - callback.Run(true, paths, bookmarks); + dict.Set("filePaths", paths); + dict.Set("bookmarks", bookmarks); #else ReadDialogPaths(dialog, &paths); - callback.Run(true, paths); + dict.Set("filePaths", paths); #endif + promise.Resolve(dict.GetHandle()); } } void ShowOpenDialog(const DialogSettings& settings, - const OpenDialogCallback& c) { + atom::util::Promise promise) { NSOpenPanel* dialog = [NSOpenPanel openPanel]; SetupDialog(dialog, settings); SetupDialogForProperties(dialog, settings.properties); - // Duplicate the callback object here since c is a reference and gcd would - // only store the pointer, by duplication we can force gcd to store a copy. - __block OpenDialogCallback callback = c; // Capture the value of the security_scoped_bookmarks settings flag // and pass it to the completion handler. bool security_scoped_bookmarks = settings.security_scoped_bookmarks; + __block atom::util::Promise p = std::move(promise); + if (!settings.parent_window || !settings.parent_window->GetNativeWindow() || settings.force_detached) { [dialog beginWithCompletionHandler:^(NSInteger chosen) { - OpenDialogCompletion(chosen, dialog, security_scoped_bookmarks, callback); + OpenDialogCompletion(chosen, dialog, security_scoped_bookmarks, + std::move(p)); }]; } else { NSWindow* window = settings.parent_window->GetNativeWindow().GetNativeNSWindow(); - [dialog beginSheetModalForWindow:window - completionHandler:^(NSInteger chosen) { - OpenDialogCompletion(chosen, dialog, - security_scoped_bookmarks, callback); - }]; + [dialog + beginSheetModalForWindow:window + completionHandler:^(NSInteger chosen) { + OpenDialogCompletion(chosen, dialog, security_scoped_bookmarks, + std::move(p)); + }]; } } diff --git a/atom/browser/ui/file_dialog_win.cc b/atom/browser/ui/file_dialog_win.cc index ed7641501f1a3..46e9191ab81a9 100644 --- a/atom/browser/ui/file_dialog_win.cc +++ b/atom/browser/ui/file_dialog_win.cc @@ -81,13 +81,23 @@ bool CreateDialogThread(RunState* run_state) { return true; } +void OnDialogOpened(atom::util::Promise promise, + bool canceled, + std::vector paths) { + mate::Dictionary dict = mate::Dictionary::CreateEmpty(promise.isolate()); + dict.Set("canceled", canceled); + dict.Set("filePaths", paths); + promise.Resolve(dict.GetHandle()); +} + void RunOpenDialogInNewThread(const RunState& run_state, const DialogSettings& settings, - const OpenDialogCallback& callback) { + atom::util::Promise promise) { std::vector paths; - bool result = ShowOpenDialog(settings, &paths); - run_state.ui_task_runner->PostTask(FROM_HERE, - base::Bind(callback, result, paths)); + bool result = ShowOpenDialogSync(settings, &paths); + run_state.ui_task_runner->PostTask( + FROM_HERE, + base::BindOnce(&OnDialogOpened, std::move(promise), result, paths)); run_state.ui_task_runner->DeleteSoon(FROM_HERE, run_state.dialog_thread); } @@ -197,8 +207,8 @@ static void ApplySettings(IFileDialog* dialog, const DialogSettings& settings) { } } -bool ShowOpenDialog(const DialogSettings& settings, - std::vector* paths) { +bool ShowOpenDialogSync(const DialogSettings& settings, + std::vector* paths) { ATL::CComPtr file_open_dialog; HRESULT hr = file_open_dialog.CoCreateInstance(CLSID_FileOpenDialog); @@ -251,16 +261,18 @@ bool ShowOpenDialog(const DialogSettings& settings, } void ShowOpenDialog(const DialogSettings& settings, - const OpenDialogCallback& callback) { + atom::util::Promise promise) { + mate::Dictionary dict = mate::Dictionary::CreateEmpty(promise.isolate()); RunState run_state; if (!CreateDialogThread(&run_state)) { - callback.Run(false, std::vector()); - return; + dict.Set("canceled", true); + dict.Set("filePaths", std::vector()); + promise.Resolve(dict.GetHandle()); + } else { + run_state.dialog_thread->task_runner()->PostTask( + FROM_HERE, base::BindOnce(&RunOpenDialogInNewThread, run_state, + settings, std::move(promise))); } - - run_state.dialog_thread->task_runner()->PostTask( - FROM_HERE, - base::Bind(&RunOpenDialogInNewThread, run_state, settings, callback)); } bool ShowSaveDialog(const DialogSettings& settings, base::FilePath* path) { diff --git a/atom/browser/web_dialog_helper.cc b/atom/browser/web_dialog_helper.cc index bd705e8f09af0..b68220b5be57a 100644 --- a/atom/browser/web_dialog_helper.cc +++ b/atom/browser/web_dialog_helper.cc @@ -11,6 +11,7 @@ #include "atom/browser/atom_browser_context.h" #include "atom/browser/native_window.h" #include "atom/browser/ui/file_dialog.h" +#include "atom/common/native_mate_converters/callback.h" #include "base/bind.h" #include "base/files/file_enumerator.h" #include "base/files/file_path.h" @@ -46,8 +47,12 @@ class FileSelectHelper : public base::RefCounted, } void ShowOpenDialog(const file_dialog::DialogSettings& settings) { + v8::Isolate* isolate = v8::Isolate::GetCurrent(); + atom::util::Promise promise(isolate); + + file_dialog::ShowOpenDialog(settings, std::move(promise)); auto callback = base::Bind(&FileSelectHelper::OnOpenDialogDone, this); - file_dialog::ShowOpenDialog(settings, callback); + ignore_result(promise.Then(callback)); } void ShowSaveDialog(const file_dialog::DialogSettings& settings) { diff --git a/docs/api/dialog.md b/docs/api/dialog.md index bc1a0cc99069d..5843e4e11d607 100644 --- a/docs/api/dialog.md +++ b/docs/api/dialog.md @@ -23,7 +23,67 @@ console.log(dialog) The `dialog` module has the following methods: -### `dialog.showOpenDialog([browserWindow, ]options[, callback])` +### `dialog.showOpenDialogSync([browserWindow, ]options)` + +* `browserWindow` [BrowserWindow](browser-window.md) (optional) +* `options` Object + * `title` String (optional) + * `defaultPath` String (optional) + * `buttonLabel` String (optional) - Custom label for the confirmation button, when + left empty the default label will be used. + * `filters` [FileFilter[]](structures/file-filter.md) (optional) + * `properties` String[] (optional) - Contains which features the dialog should + use. The following values are supported: + * `openFile` - Allow files to be selected. + * `openDirectory` - Allow directories to be selected. + * `multiSelections` - Allow multiple paths to be selected. + * `showHiddenFiles` - Show hidden files in dialog. + * `createDirectory` _macOS_ - Allow creating new directories from dialog. + * `promptToCreate` _Windows_ - Prompt for creation if the file path entered + in the dialog does not exist. This does not actually create the file at + the path but allows non-existent paths to be returned that should be + created by the application. + * `noResolveAliases` _macOS_ - Disable the automatic alias (symlink) path + resolution. Selected aliases will now return the alias path instead of + their target path. + * `treatPackageAsDirectory` _macOS_ - Treat packages, such as `.app` folders, + as a directory instead of a file. + * `message` String (optional) _macOS_ - Message to display above input + boxes. + * `securityScopedBookmarks` Boolean (optional) _masOS_ _mas_ - Create [security scoped bookmarks](https://developer.apple.com/library/content/documentation/Security/Conceptual/AppSandboxDesignGuide/AppSandboxInDepth/AppSandboxInDepth.html#//apple_ref/doc/uid/TP40011183-CH3-SW16) when packaged for the Mac App Store. + +The `browserWindow` argument allows the dialog to attach itself to a parent window, making it modal. + +The `filters` specifies an array of file types that can be displayed or +selected when you want to limit the user to a specific type. For example: + +```javascript +{ + filters: [ + { name: 'Images', extensions: ['jpg', 'png', 'gif'] }, + { name: 'Movies', extensions: ['mkv', 'avi', 'mp4'] }, + { name: 'Custom File Type', extensions: ['as'] }, + { name: 'All Files', extensions: ['*'] } + ] +} +``` + +The `extensions` array should contain extensions without wildcards or dots (e.g. +`'png'` is good but `'.png'` and `'*.png'` are bad). To show all files, use the +`'*'` wildcard (no other wildcard is supported). + +**Note:** On Windows and Linux an open dialog can not be both a file selector +and a directory selector, so if you set `properties` to +`['openFile', 'openDirectory']` on these platforms, a directory selector will be +shown. + +```js +dialog.showOpenDialogSync(mainWindow, { + properties: ['openFile', 'openDirectory'] +}) +``` + +### `dialog.showOpenDialog([browserWindow, ]options)` * `browserWindow` [BrowserWindow](browser-window.md) (optional) * `options` Object @@ -52,11 +112,12 @@ The `dialog` module has the following methods: boxes. * `securityScopedBookmarks` Boolean (optional) _masOS_ _mas_ - Create [security scoped bookmarks](https://developer.apple.com/library/content/documentation/Security/Conceptual/AppSandboxDesignGuide/AppSandboxInDepth/AppSandboxInDepth.html#//apple_ref/doc/uid/TP40011183-CH3-SW16) when packaged for the Mac App Store. * `callback` Function (optional) - * `filePaths` String[] (optional) - An array of file paths chosen by the user. If the dialog is cancelled this will be `undefined`. - * `bookmarks` String[] (optional) _macOS_ _mas_ - An array matching the `filePaths` array of base64 encoded strings which contains security scoped bookmark data. `securityScopedBookmarks` must be enabled for this to be populated. -Returns `String[] | undefined`, an array of file paths chosen by the user, -if the callback is provided it returns `undefined`. +Returns `Promise` - Resolve wih an object containing the following: + +* `canceled` - Boolean - whether or not the dialog was canceled. +* `filePaths` String[] (optional) - An array of file paths chosen by the user. If the dialog is cancelled this will be an empty array. +* `bookmarks` String[] (optional) _macOS_ _mas_ - An array matching the `filePaths` array of base64 encoded strings which contains security scoped bookmark data. `securityScopedBookmarks` must be enabled for this to be populated. The `browserWindow` argument allows the dialog to attach itself to a parent window, making it modal. @@ -78,14 +139,22 @@ The `extensions` array should contain extensions without wildcards or dots (e.g. `'png'` is good but `'.png'` and `'*.png'` are bad). To show all files, use the `'*'` wildcard (no other wildcard is supported). -If a `callback` is passed, the API call will be asynchronous and the result -will be passed via `callback(filenames)`. - **Note:** On Windows and Linux an open dialog can not be both a file selector and a directory selector, so if you set `properties` to `['openFile', 'openDirectory']` on these platforms, a directory selector will be shown. +```js +dialog.showOpenDialog(mainWindow, { + properties: ['openFile', 'openDirectory'] +}).then(result => { + console.log(result.canceled) + console.log(result.filePaths) +}).catch(err => { + console.log(err) +}) +``` + ### `dialog.showSaveDialog([browserWindow, ]options[, callback])` * `browserWindow` [BrowserWindow](browser-window.md) (optional) @@ -201,9 +270,9 @@ attached to the parent window, making it modal. On Windows the options are more limited, due to the Win32 APIs used: - - The `message` argument is not used, as the OS provides its own confirmation +* The `message` argument is not used, as the OS provides its own confirmation dialog. - - The `browserWindow` argument is ignored since it is not possible to make +* The `browserWindow` argument is ignored since it is not possible to make this confirmation dialog modal. ## Sheets diff --git a/docs/api/promisification.md b/docs/api/promisification.md index 096f93a26980c..c0d6f30e0fb23 100644 --- a/docs/api/promisification.md +++ b/docs/api/promisification.md @@ -9,7 +9,6 @@ When a majority of affected functions are migrated, this flag will be enabled by ### Candidate Functions - [app.importCertificate(options, callback)](https://github.com/electron/electron/blob/master/docs/api/app.md#importCertificate) -- [dialog.showOpenDialog([browserWindow, ]options[, callback])](https://github.com/electron/electron/blob/master/docs/api/dialog.md#showOpenDialog) - [dialog.showSaveDialog([browserWindow, ]options[, callback])](https://github.com/electron/electron/blob/master/docs/api/dialog.md#showSaveDialog) - [dialog.showMessageBox([browserWindow, ]options[, callback])](https://github.com/electron/electron/blob/master/docs/api/dialog.md#showMessageBox) - [dialog.showCertificateTrustDialog([browserWindow, ]options, callback)](https://github.com/electron/electron/blob/master/docs/api/dialog.md#showCertificateTrustDialog) @@ -46,6 +45,7 @@ When a majority of affected functions are migrated, this flag will be enabled by - [debugger.sendCommand(method[, commandParams, callback])](https://github.com/electron/electron/blob/master/docs/api/debugger.md#sendCommand) - [desktopCapturer.getSources(options, callback)](https://github.com/electron/electron/blob/master/docs/api/desktop-capturer.md#getSources) - [netLog.stopLogging([callback])](https://github.com/electron/electron/blob/master/docs/api/net-log.md#stopLogging) +- [dialog.showOpenDialog([browserWindow, ]options[, callback])](https://github.com/electron/electron/blob/master/docs/api/dialog.md#showOpenDialog) - [protocol.isProtocolHandled(scheme, callback)](https://github.com/electron/electron/blob/master/docs/api/protocol.md#isProtocolHandled) - [shell.openExternal(url[, options, callback])](https://github.com/electron/electron/blob/master/docs/api/shell.md#openExternal) - [webviewTag.capturePage([rect, ]callback)](https://github.com/electron/electron/blob/master/docs/api/webview-tag.md#capturePage) diff --git a/lib/browser/api/dialog.js b/lib/browser/api/dialog.js index 7659800b79b21..4e7032d38525d 100644 --- a/lib/browser/api/dialog.js +++ b/lib/browser/api/dialog.js @@ -1,6 +1,6 @@ 'use strict' -const { app, BrowserWindow } = require('electron') +const { app, BrowserWindow, deprecate } = require('electron') const binding = process.atomBinding('dialog') const v8Util = process.atomBinding('v8_util') @@ -70,70 +70,54 @@ const checkAppInitialized = function () { } } -module.exports = { - showOpenDialog: function (...args) { - checkAppInitialized() - - let [window, options, callback] = parseArgs(...args) - - if (options == null) { - options = { - title: 'Open', - properties: ['openFile'] - } - } +const openDialog = (sync, window, options) => { + checkAppInitialized() - let { buttonLabel, defaultPath, filters, properties, title, message, securityScopedBookmarks = false } = options - - if (properties == null) { - properties = ['openFile'] - } else if (!Array.isArray(properties)) { - throw new TypeError('Properties must be an array') - } - - let dialogProperties = 0 - for (const prop in fileDialogProperties) { - if (properties.includes(prop)) { - dialogProperties |= fileDialogProperties[prop] - } - } - - if (title == null) { - title = '' - } else if (typeof title !== 'string') { - throw new TypeError('Title must be a string') + if (window.constructor !== BrowserWindow) options = window + if (options == null) { + options = { + title: 'Open', + properties: ['openFile'] } + } - if (buttonLabel == null) { - buttonLabel = '' - } else if (typeof buttonLabel !== 'string') { - throw new TypeError('Button label must be a string') + const { + buttonLabel = '', + defaultPath = '', + filters = [], + properties = ['openFile'], + title = '', + message = '', + securityScopedBookmarks = false + } = options + + if (!Array.isArray(properties)) throw new TypeError('Properties must be an array') + + let dialogProperties = 0 + for (const prop in fileDialogProperties) { + if (properties.includes(prop)) { + dialogProperties |= fileDialogProperties[prop] } + } - if (defaultPath == null) { - defaultPath = '' - } else if (typeof defaultPath !== 'string') { - throw new TypeError('Default path must be a string') - } + if (typeof title !== 'string') throw new TypeError('Title must be a string') + if (typeof buttonLabel !== 'string') throw new TypeError('Button label must be a string') + if (typeof defaultPath !== 'string') throw new TypeError('Default path must be a string') + if (typeof message !== 'string') throw new TypeError('Message must be a string') - if (filters == null) { - filters = [] - } + const settings = { title, buttonLabel, defaultPath, filters, message, securityScopedBookmarks, window } + settings.properties = dialogProperties - if (message == null) { - message = '' - } else if (typeof message !== 'string') { - throw new TypeError('Message must be a string') - } + return (sync) ? binding.showOpenDialogSync(settings) : binding.showOpenDialog(settings) +} - const wrappedCallback = typeof callback === 'function' ? function (success, result, bookmarkData) { - return success ? callback(result, bookmarkData) : callback() - } : null - const settings = { title, buttonLabel, defaultPath, filters, message, securityScopedBookmarks, window } - settings.properties = dialogProperties - return binding.showOpenDialog(settings, wrappedCallback) +module.exports = { + showOpenDialog: function (window, options) { + return openDialog(false, window, options) + }, + showOpenDialogSync: function (window, options) { + return openDialog(true, window, options) }, - showSaveDialog: function (...args) { checkAppInitialized() @@ -306,6 +290,8 @@ module.exports = { } } +module.exports.showOpenDialog = deprecate.promisify(module.exports.showOpenDialog) + // Mark standard asynchronous functions. v8Util.setHiddenValue(module.exports.showMessageBox, 'asynchronous', true) v8Util.setHiddenValue(module.exports.showOpenDialog, 'asynchronous', true) diff --git a/lib/browser/chrome-devtools.js b/lib/browser/chrome-devtools.js index f2db3334441c5..b3aa84f0f9493 100644 --- a/lib/browser/chrome-devtools.js +++ b/lib/browser/chrome-devtools.js @@ -86,9 +86,9 @@ ipcMainUtils.handle('ELECTRON_INSPECTOR_SELECT_FILE', function (event) { return new Promise((resolve, reject) => { assertChromeDevTools(event.sender, 'window.UI.createFileSelectorElement()') - dialog.showOpenDialog({}, function (files) { - if (files) { - const path = files[0] + dialog.showOpenDialog({}, function (result) { + if (!result.canceled) { + const path = result.filePaths[0] fs.readFile(path, (error, data) => { if (error) { reject(error) diff --git a/spec/ts-smoke/electron/main.ts b/spec/ts-smoke/electron/main.ts index 378c8bad0c215..20027d0cb83d7 100644 --- a/spec/ts-smoke/electron/main.ts +++ b/spec/ts-smoke/electron/main.ts @@ -479,7 +479,7 @@ contentTracing.startRecording(options, function () { // https://github.com/atom/electron/blob/master/docs/api/dialog.md // variant without browserWindow -let openDialogResult: string[] = dialog.showOpenDialog({ +dialog.showOpenDialogSync({ title: 'Testing showOpenDialog', defaultPath: '/var/log/syslog', filters: [{ name: '', extensions: [''] }], @@ -487,11 +487,13 @@ let openDialogResult: string[] = dialog.showOpenDialog({ }) // variant with browserWindow -openDialogResult = dialog.showOpenDialog(win3, { +dialog.showOpenDialog(win3, { title: 'Testing showOpenDialog', defaultPath: '/var/log/syslog', filters: [{ name: '', extensions: [''] }], properties: ['openFile', 'openDirectory', 'multiSelections'] +}).then(ret => { + console.log(ret) }) // global-shortcut From aa863f3246994bf4d2eec79e7ecf288ad29cdc22 Mon Sep 17 00:00:00 2001 From: "Koen [XII]" Date: Tue, 5 Mar 2019 18:20:52 +0100 Subject: [PATCH 05/22] Fix typo in registerSchemesAsPrivileged reference (#17214) --- docs/api/protocol.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/api/protocol.md b/docs/api/protocol.md index d9804a87d9c17..ca5e041bb95b7 100644 --- a/docs/api/protocol.md +++ b/docs/api/protocol.md @@ -75,7 +75,7 @@ By default web storage apis (localStorage, sessionStorage, webSQL, indexedDB, co are disabled for non standard schemes. So in general if you want to register a custom protocol to replace the `http` protocol, you have to register it as a standard scheme. -`protocol.registerSchemesAsPriviliged` can be used to replicate the functionality of the previous `protocol.registerStandardSchemes`, `webFrame.registerURLSchemeAs*` and `protocol.registerServiceWorkerSchemes` functions that existed prior to Electron 5.0.0, for example: +`protocol.registerSchemesAsPrivileged` can be used to replicate the functionality of the previous `protocol.registerStandardSchemes`, `webFrame.registerURLSchemeAs*` and `protocol.registerServiceWorkerSchemes` functions that existed prior to Electron 5.0.0, for example: **before (<= v4.x)** ```javascript @@ -88,7 +88,7 @@ webFrame.registerURLSchemeAsPrivileged('scheme2', { secure: true }) **after (>= v5.x)** ```javascript -protocol.registerSchemesAsPriviliged([ +protocol.registerSchemesAsPrivileged([ { scheme: 'scheme1', privileges: { standard: true, secure: true } }, { scheme: 'scheme2', privileges: { standard: true, secure: true } } ]) From 5a88d9e6fcb1e14b92635d5f81e0c0b32a7d8cb5 Mon Sep 17 00:00:00 2001 From: Jeremy Apthorp Date: Tue, 5 Mar 2019 10:40:52 -0800 Subject: [PATCH 06/22] docs: add troubleshooting note about pywin32 (#17216) --- docs/development/build-instructions-windows.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/development/build-instructions-windows.md b/docs/development/build-instructions-windows.md index abc45c5ab5de1..62c27a7843ee0 100644 --- a/docs/development/build-instructions-windows.md +++ b/docs/development/build-instructions-windows.md @@ -109,3 +109,7 @@ $ git config --system core.longpaths true ### error: use of undeclared identifier 'DefaultDelegateCheckMode' This can happen during build, when Debugging Tools for Windows has been installed with Windows Driver Kit. Uninstall Windows Driver Kit and install Debugging Tools with steps described above. + +### ImportError: No module named win32file + +Make sure you have installed `pywin32` with `pip install pywin32`. From 92c9dbc179614099d1389c32161b15c178f47e8b Mon Sep 17 00:00:00 2001 From: Roller Bot <42361097+roller-bot@users.noreply.github.com> Date: Tue, 5 Mar 2019 11:01:38 -0800 Subject: [PATCH 07/22] chore: bump chromium in DEPS to 73.0.3683.65 (#17234) --- DEPS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DEPS b/DEPS index 23f4770e13ea4..35cc92e111e34 100644 --- a/DEPS +++ b/DEPS @@ -10,7 +10,7 @@ gclient_gn_args = [ vars = { 'chromium_version': - '73.0.3683.61', + '73.0.3683.65', 'node_version': 'fac6d766c143db8db05bb3b0c0871df8f032363c', From 6cb7b8d3a4b0ec2e6c1edb314dc32f126882cf37 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Tue, 5 Mar 2019 13:48:20 -0800 Subject: [PATCH 08/22] feat: promisify dialog.showSaveDialog() (#17054) * feat: promisify dialog.showSaveDialog() * address some feedback from review * filename => filePath * fix last filename => filePath --- atom/browser/api/atom_api_dialog.cc | 28 +++--- .../browser/atom_download_manager_delegate.cc | 8 +- atom/browser/common_web_contents_delegate.cc | 2 +- atom/browser/ui/file_dialog.h | 14 +-- atom/browser/ui/file_dialog_gtk.cc | 32 ++++--- atom/browser/ui/file_dialog_mac.mm | 39 ++++---- atom/browser/ui/file_dialog_win.cc | 36 +++++--- atom/browser/web_dialog_helper.cc | 12 ++- docs/api/dialog.md | 38 ++++++-- docs/api/promisification.md | 4 +- lib/browser/api/dialog.js | 88 +++++++------------ 11 files changed, 166 insertions(+), 135 deletions(-) diff --git a/atom/browser/api/atom_api_dialog.cc b/atom/browser/api/atom_api_dialog.cc index 2958b7273b1db..197acc3a2ed26 100644 --- a/atom/browser/api/atom_api_dialog.cc +++ b/atom/browser/api/atom_api_dialog.cc @@ -68,18 +68,21 @@ v8::Local ShowOpenDialog( return handle; } -void ShowSaveDialog(const file_dialog::DialogSettings& settings, - mate::Arguments* args) { - v8::Local peek = args->PeekNext(); - file_dialog::SaveDialogCallback callback; - if (mate::Converter::FromV8( - args->isolate(), peek, &callback)) { - file_dialog::ShowSaveDialog(settings, callback); - } else { - base::FilePath path; - if (file_dialog::ShowSaveDialog(settings, &path)) - args->Return(path); - } +void ShowSaveDialogSync(const file_dialog::DialogSettings& settings, + mate::Arguments* args) { + base::FilePath path; + if (file_dialog::ShowSaveDialogSync(settings, &path)) + args->Return(path); +} + +v8::Local ShowSaveDialog( + const file_dialog::DialogSettings& settings, + mate::Arguments* args) { + atom::util::Promise promise(args->isolate()); + v8::Local handle = promise.GetHandle(); + + file_dialog::ShowSaveDialog(settings, std::move(promise)); + return handle; } void Initialize(v8::Local exports, @@ -91,6 +94,7 @@ void Initialize(v8::Local exports, dict.SetMethod("showErrorBox", &atom::ShowErrorBox); dict.SetMethod("showOpenDialogSync", &ShowOpenDialogSync); dict.SetMethod("showOpenDialog", &ShowOpenDialog); + dict.SetMethod("showSaveDialogSync", &ShowSaveDialogSync); dict.SetMethod("showSaveDialog", &ShowSaveDialog); #if defined(OS_MACOSX) || defined(OS_WIN) dict.SetMethod("showCertificateTrustDialog", diff --git a/atom/browser/atom_download_manager_delegate.cc b/atom/browser/atom_download_manager_delegate.cc index 49851db4ffe1d..7fe2292abb263 100644 --- a/atom/browser/atom_download_manager_delegate.cc +++ b/atom/browser/atom_download_manager_delegate.cc @@ -5,12 +5,14 @@ #include "atom/browser/atom_download_manager_delegate.h" #include +#include #include "atom/browser/api/atom_api_download_item.h" #include "atom/browser/atom_browser_context.h" #include "atom/browser/native_window.h" #include "atom/browser/ui/file_dialog.h" #include "atom/browser/web_contents_preferences.h" +#include "atom/common/native_mate_converters/callback.h" #include "atom/common/options_switches.h" #include "base/bind.h" #include "base/files/file_util.h" @@ -119,10 +121,14 @@ void AtomDownloadManagerDelegate::OnDownloadPathGenerated( !web_preferences || web_preferences->IsEnabled(options::kOffscreen); settings.force_detached = offscreen; + v8::Isolate* isolate = v8::Isolate::GetCurrent(); + atom::util::Promise dialog_promise(isolate); auto dialog_callback = base::Bind(&AtomDownloadManagerDelegate::OnDownloadSaveDialogDone, base::Unretained(this), download_id, callback); - file_dialog::ShowSaveDialog(settings, dialog_callback); + + file_dialog::ShowSaveDialog(settings, std::move(dialog_promise)); + ignore_result(dialog_promise.Then(dialog_callback)); } else { callback.Run(path, download::DownloadItem::TARGET_DISPOSITION_PROMPT, download::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, path, diff --git a/atom/browser/common_web_contents_delegate.cc b/atom/browser/common_web_contents_delegate.cc index e5e6e196ba109..dae1ec367d67a 100644 --- a/atom/browser/common_web_contents_delegate.cc +++ b/atom/browser/common_web_contents_delegate.cc @@ -382,7 +382,7 @@ void CommonWebContentsDelegate::DevToolsSaveToFile(const std::string& url, settings.force_detached = offscreen_; settings.title = url; settings.default_path = base::FilePath::FromUTF8Unsafe(url); - if (!file_dialog::ShowSaveDialog(settings, &path)) { + if (!file_dialog::ShowSaveDialogSync(settings, &path)) { base::Value url_value(url); web_contents_->CallClientFunction("DevToolsAPI.canceledSaveURL", &url_value, nullptr, nullptr); diff --git a/atom/browser/ui/file_dialog.h b/atom/browser/ui/file_dialog.h index 6666b369c84aa..ba50bd29bc0c1 100644 --- a/atom/browser/ui/file_dialog.h +++ b/atom/browser/ui/file_dialog.h @@ -36,16 +36,6 @@ enum FileDialogProperty { FILE_DIALOG_TREAT_PACKAGE_APP_AS_DIRECTORY = 1 << 7, }; -#if defined(MAS_BUILD) -typedef base::Callback - SaveDialogCallback; -#else -typedef base::Callback - SaveDialogCallback; -#endif - struct DialogSettings { atom::NativeWindow* parent_window = nullptr; std::string title; @@ -70,10 +60,10 @@ bool ShowOpenDialogSync(const DialogSettings& settings, void ShowOpenDialog(const DialogSettings& settings, atom::util::Promise promise); -bool ShowSaveDialog(const DialogSettings& settings, base::FilePath* path); +bool ShowSaveDialogSync(const DialogSettings& settings, base::FilePath* path); void ShowSaveDialog(const DialogSettings& settings, - const SaveDialogCallback& callback); + atom::util::Promise promise); } // namespace file_dialog diff --git a/atom/browser/ui/file_dialog_gtk.cc b/atom/browser/ui/file_dialog_gtk.cc index 75447c46e6261..3c245e5521972 100644 --- a/atom/browser/ui/file_dialog_gtk.cc +++ b/atom/browser/ui/file_dialog_gtk.cc @@ -126,8 +126,8 @@ class FileChooserDialog { gtk_window_present_with_time(GTK_WINDOW(dialog_), time); } - void RunSaveAsynchronous(const SaveDialogCallback& callback) { - save_callback_ = callback; + void RunSaveAsynchronous(atom::util::Promise promise) { + save_promise_.reset(new atom::util::Promise(std::move(promise))); RunAsynchronous(); } @@ -173,7 +173,7 @@ class FileChooserDialog { GtkWidget* preview_; Filters filters_; - SaveDialogCallback save_callback_; + std::unique_ptr save_promise_; std::unique_ptr open_promise_; // Callback for when we update the preview for the selection. @@ -184,12 +184,17 @@ class FileChooserDialog { void FileChooserDialog::OnFileDialogResponse(GtkWidget* widget, int response) { gtk_widget_hide(dialog_); - - if (!save_callback_.is_null()) { - if (response == GTK_RESPONSE_ACCEPT) - save_callback_.Run(true, GetFileName()); - else - save_callback_.Run(false, base::FilePath()); + if (save_promise_) { + mate::Dictionary dict = + mate::Dictionary::CreateEmpty(save_promise_->isolate()); + if (response == GTK_RESPONSE_ACCEPT) { + dict.Set("canceled", false); + dict.Set("filePath", GetFileName()); + } else { + dict.Set("canceled", true); + dict.Set("filePath", base::FilePath()); + } + save_promise_->Resolve(dict.GetHandle()); } else if (open_promise_) { mate::Dictionary dict = mate::Dictionary::CreateEmpty(open_promise_->isolate()); @@ -285,23 +290,22 @@ void ShowOpenDialog(const DialogSettings& settings, open_dialog->RunOpenAsynchronous(std::move(promise)); } -bool ShowSaveDialog(const DialogSettings& settings, base::FilePath* path) { +bool ShowSaveDialogSync(const DialogSettings& settings, base::FilePath* path) { FileChooserDialog save_dialog(GTK_FILE_CHOOSER_ACTION_SAVE, settings); gtk_widget_show_all(save_dialog.dialog()); int response = gtk_dialog_run(GTK_DIALOG(save_dialog.dialog())); if (response == GTK_RESPONSE_ACCEPT) { *path = save_dialog.GetFileName(); return true; - } else { - return false; } + return false; } void ShowSaveDialog(const DialogSettings& settings, - const SaveDialogCallback& callback) { + atom::util::Promise promise) { FileChooserDialog* save_dialog = new FileChooserDialog(GTK_FILE_CHOOSER_ACTION_SAVE, settings); - save_dialog->RunSaveAsynchronous(callback); + save_dialog->RunSaveAsynchronous(std::move(promise)); } } // namespace file_dialog diff --git a/atom/browser/ui/file_dialog_mac.mm b/atom/browser/ui/file_dialog_mac.mm index fb0b61368febe..d6aaecb93e060 100644 --- a/atom/browser/ui/file_dialog_mac.mm +++ b/atom/browser/ui/file_dialog_mac.mm @@ -346,7 +346,7 @@ void ShowOpenDialog(const DialogSettings& settings, } } -bool ShowSaveDialog(const DialogSettings& settings, base::FilePath* path) { +bool ShowSaveDialogSync(const DialogSettings& settings, base::FilePath* path) { DCHECK(path); NSSavePanel* dialog = [NSSavePanel savePanel]; @@ -363,50 +363,57 @@ bool ShowSaveDialog(const DialogSettings& settings, base::FilePath* path) { void SaveDialogCompletion(int chosen, NSSavePanel* dialog, bool security_scoped_bookmarks, - const SaveDialogCallback& callback) { + atom::util::Promise promise) { + mate::Dictionary dict = mate::Dictionary::CreateEmpty(promise.isolate()); if (chosen == NSFileHandlingPanelCancelButton) { + dict.Set("canceled", true); + dict.Set("filePath", base::FilePath()); #if defined(MAS_BUILD) - callback.Run(false, base::FilePath(), ""); -#else - callback.Run(false, base::FilePath()); + dict.Set("bookmark", ""); #endif } else { std::string path = base::SysNSStringToUTF8([[dialog URL] path]); + dict.Set("filePath", base::FilePath(path)); + dict.Set("canceled", false); #if defined(MAS_BUILD) std::string bookmark; if (security_scoped_bookmarks) { bookmark = GetBookmarkDataFromNSURL([dialog URL]); + dict.Set("bookmark", bookmark); } - callback.Run(true, base::FilePath(path), bookmark); -#else - callback.Run(true, base::FilePath(path)); #endif } + promise.Resolve(dict.GetHandle()); } void ShowSaveDialog(const DialogSettings& settings, - const SaveDialogCallback& c) { + atom::util::Promise promise) { NSSavePanel* dialog = [NSSavePanel savePanel]; SetupDialog(dialog, settings); [dialog setCanSelectHiddenExtension:YES]; - __block SaveDialogCallback callback = c; + // Capture the value of the security_scoped_bookmarks settings flag + // and pass it to the completion handler. bool security_scoped_bookmarks = settings.security_scoped_bookmarks; + __block atom::util::Promise p = std::move(promise); + if (!settings.parent_window || !settings.parent_window->GetNativeWindow() || settings.force_detached) { [dialog beginWithCompletionHandler:^(NSInteger chosen) { - SaveDialogCompletion(chosen, dialog, security_scoped_bookmarks, callback); + SaveDialogCompletion(chosen, dialog, security_scoped_bookmarks, + std::move(p)); }]; } else { NSWindow* window = settings.parent_window->GetNativeWindow().GetNativeNSWindow(); - [dialog beginSheetModalForWindow:window - completionHandler:^(NSInteger chosen) { - SaveDialogCompletion(chosen, dialog, - security_scoped_bookmarks, callback); - }]; + [dialog + beginSheetModalForWindow:window + completionHandler:^(NSInteger chosen) { + SaveDialogCompletion(chosen, dialog, security_scoped_bookmarks, + std::move(p)); + }]; } } diff --git a/atom/browser/ui/file_dialog_win.cc b/atom/browser/ui/file_dialog_win.cc index 46e9191ab81a9..c8d7f36c06f5f 100644 --- a/atom/browser/ui/file_dialog_win.cc +++ b/atom/browser/ui/file_dialog_win.cc @@ -101,13 +101,23 @@ void RunOpenDialogInNewThread(const RunState& run_state, run_state.ui_task_runner->DeleteSoon(FROM_HERE, run_state.dialog_thread); } +void OnSaveDialogDone(atom::util::Promise promise, + bool canceled, + const base::FilePath path) { + mate::Dictionary dict = mate::Dictionary::CreateEmpty(promise.isolate()); + dict.Set("canceled", canceled); + dict.Set("filePath", path); + promise.Resolve(dict.GetHandle()); +} + void RunSaveDialogInNewThread(const RunState& run_state, const DialogSettings& settings, - const SaveDialogCallback& callback) { + atom::util::Promise promise) { base::FilePath path; - bool result = ShowSaveDialog(settings, &path); - run_state.ui_task_runner->PostTask(FROM_HERE, - base::Bind(callback, result, path)); + bool result = ShowSaveDialogSync(settings, &path); + run_state.ui_task_runner->PostTask( + FROM_HERE, + base::BindOnce(&OnSaveDialogDone, std::move(promise), result, path)); run_state.ui_task_runner->DeleteSoon(FROM_HERE, run_state.dialog_thread); } @@ -275,7 +285,7 @@ void ShowOpenDialog(const DialogSettings& settings, } } -bool ShowSaveDialog(const DialogSettings& settings, base::FilePath* path) { +bool ShowSaveDialogSync(const DialogSettings& settings, base::FilePath* path) { ATL::CComPtr file_save_dialog; HRESULT hr = file_save_dialog.CoCreateInstance(CLSID_FileSaveDialog); if (FAILED(hr)) @@ -306,16 +316,18 @@ bool ShowSaveDialog(const DialogSettings& settings, base::FilePath* path) { } void ShowSaveDialog(const DialogSettings& settings, - const SaveDialogCallback& callback) { + atom::util::Promise promise) { RunState run_state; if (!CreateDialogThread(&run_state)) { - callback.Run(false, base::FilePath()); - return; + mate::Dictionary dict = mate::Dictionary::CreateEmpty(promise.isolate()); + dict.Set("canceled", false); + dict.Set("filePath", base::FilePath()); + promise.Resolve(dict.GetHandle()); + } else { + run_state.dialog_thread->task_runner()->PostTask( + FROM_HERE, base::BindOnce(&RunSaveDialogInNewThread, run_state, + settings, std::move(promise))); } - - run_state.dialog_thread->task_runner()->PostTask( - FROM_HERE, - base::Bind(&RunSaveDialogInNewThread, run_state, settings, callback)); } } // namespace file_dialog diff --git a/atom/browser/web_dialog_helper.cc b/atom/browser/web_dialog_helper.cc index b68220b5be57a..1284808c73e43 100644 --- a/atom/browser/web_dialog_helper.cc +++ b/atom/browser/web_dialog_helper.cc @@ -56,8 +56,16 @@ class FileSelectHelper : public base::RefCounted, } void ShowSaveDialog(const file_dialog::DialogSettings& settings) { - auto callback = base::Bind(&FileSelectHelper::OnSaveDialogDone, this); - file_dialog::ShowSaveDialog(settings, callback); + v8::Isolate* isolate = v8::Isolate::GetCurrent(); + v8::Local context = isolate->GetCurrentContext(); + atom::util::Promise promise(isolate); + v8::Local handle = promise.GetHandle(); + + file_dialog::ShowSaveDialog(settings, std::move(promise)); + ignore_result(handle->Then( + context, + v8::Local::Cast(mate::ConvertToV8( + isolate, base::Bind(&FileSelectHelper::OnSaveDialogDone, this))))); } private: diff --git a/docs/api/dialog.md b/docs/api/dialog.md index 5843e4e11d607..821bd55063785 100644 --- a/docs/api/dialog.md +++ b/docs/api/dialog.md @@ -155,7 +155,7 @@ dialog.showOpenDialog(mainWindow, { }) ``` -### `dialog.showSaveDialog([browserWindow, ]options[, callback])` +### `dialog.showSaveDialog([browserWindow, ]options)` * `browserWindow` [BrowserWindow](browser-window.md) (optional) * `options` Object @@ -171,22 +171,42 @@ dialog.showOpenDialog(mainWindow, { * `showsTagField` Boolean (optional) _macOS_ - Show the tags input box, defaults to `true`. * `securityScopedBookmarks` Boolean (optional) _macOS_ _mas_ - Create a [security scoped bookmark](https://developer.apple.com/library/content/documentation/Security/Conceptual/AppSandboxDesignGuide/AppSandboxInDepth/AppSandboxInDepth.html#//apple_ref/doc/uid/TP40011183-CH3-SW16) when packaged for the Mac App Store. If this option is enabled and the file doesn't already exist a blank file will be created at the chosen path. -* `callback` Function (optional) - * `filename` String (optional) If the dialog is cancelled this will be `undefined`. - * `bookmark` String (optional) _macOS_ _mas_ - Base64 encoded string which contains the security scoped bookmark data for the saved file. `securityScopedBookmarks` must be enabled for this to be present. -Returns `String | undefined`, the path of the file chosen by the user, -if a callback is provided or the dialog is cancelled it returns `undefined`. +Returns `String | undefined`, the path of the file chosen by the user; if the dialog is cancelled it returns `undefined`. The `browserWindow` argument allows the dialog to attach itself to a parent window, making it modal. The `filters` specifies an array of file types that can be displayed, see `dialog.showOpenDialog` for an example. -If a `callback` is passed, the API call will be asynchronous and the result -will be passed via `callback(filename)`. +### `dialog.showSaveDialog([browserWindow, ]options)` + +* `browserWindow` [BrowserWindow](browser-window.md) (optional) +* `options` Object + * `title` String (optional) + * `defaultPath` String (optional) - Absolute directory path, absolute file + path, or file name to use by default. + * `buttonLabel` String (optional) - Custom label for the confirmation button, when + left empty the default label will be used. + * `filters` [FileFilter[]](structures/file-filter.md) (optional) + * `message` String (optional) _macOS_ - Message to display above text fields. + * `nameFieldLabel` String (optional) _macOS_ - Custom label for the text + displayed in front of the filename text field. + * `showsTagField` Boolean (optional) _macOS_ - Show the tags input box, + defaults to `true`. + * `securityScopedBookmarks` Boolean (optional) _macOS_ _mas_ - Create a [security scoped bookmark](https://developer.apple.com/library/content/documentation/Security/Conceptual/AppSandboxDesignGuide/AppSandboxInDepth/AppSandboxInDepth.html#//apple_ref/doc/uid/TP40011183-CH3-SW16) when packaged for the Mac App Store. If this option is enabled and the file doesn't already exist a blank file will be created at the chosen path. + +Returns `Promise` - Resolve with an object containing the following: + * `canceled` Boolean - whether or not the dialog was canceled. + * `filePath` String (optional) If the dialog is canceled this will be `undefined`. + * `bookmark` String (optional) _macOS_ _mas_ - Base64 encoded string which contains the security scoped bookmark data for the saved file. `securityScopedBookmarks` must be enabled for this to be present. + +The `browserWindow` argument allows the dialog to attach itself to a parent window, making it modal. + +The `filters` specifies an array of file types that can be displayed, see +`dialog.showOpenDialog` for an example. -**Note:** On macOS, using the `callback` is recommended to avoid issues when +**Note:** On macOS, using the asynchronous version is recommended to avoid issues when expanding and collapsing the dialog. ### `dialog.showMessageBox([browserWindow, ]options[, callback])` diff --git a/docs/api/promisification.md b/docs/api/promisification.md index c0d6f30e0fb23..42af84871a408 100644 --- a/docs/api/promisification.md +++ b/docs/api/promisification.md @@ -9,7 +9,6 @@ When a majority of affected functions are migrated, this flag will be enabled by ### Candidate Functions - [app.importCertificate(options, callback)](https://github.com/electron/electron/blob/master/docs/api/app.md#importCertificate) -- [dialog.showSaveDialog([browserWindow, ]options[, callback])](https://github.com/electron/electron/blob/master/docs/api/dialog.md#showSaveDialog) - [dialog.showMessageBox([browserWindow, ]options[, callback])](https://github.com/electron/electron/blob/master/docs/api/dialog.md#showMessageBox) - [dialog.showCertificateTrustDialog([browserWindow, ]options, callback)](https://github.com/electron/electron/blob/master/docs/api/dialog.md#showCertificateTrustDialog) - [inAppPurchase.purchaseProduct(productID, quantity, callback)](https://github.com/electron/electron/blob/master/docs/api/in-app-purchase.md#purchaseProduct) @@ -44,8 +43,9 @@ When a majority of affected functions are migrated, this flag will be enabled by - [cookies.set(details, callback)](https://github.com/electron/electron/blob/master/docs/api/cookies.md#set) - [debugger.sendCommand(method[, commandParams, callback])](https://github.com/electron/electron/blob/master/docs/api/debugger.md#sendCommand) - [desktopCapturer.getSources(options, callback)](https://github.com/electron/electron/blob/master/docs/api/desktop-capturer.md#getSources) -- [netLog.stopLogging([callback])](https://github.com/electron/electron/blob/master/docs/api/net-log.md#stopLogging) - [dialog.showOpenDialog([browserWindow, ]options[, callback])](https://github.com/electron/electron/blob/master/docs/api/dialog.md#showOpenDialog) +- [dialog.showSaveDialog([browserWindow, ]options[, callback])](https://github.com/electron/electron/blob/master/docs/api/dialog.md#showSaveDialog) +- [netLog.stopLogging([callback])](https://github.com/electron/electron/blob/master/docs/api/net-log.md#stopLogging) - [protocol.isProtocolHandled(scheme, callback)](https://github.com/electron/electron/blob/master/docs/api/protocol.md#isProtocolHandled) - [shell.openExternal(url[, options, callback])](https://github.com/electron/electron/blob/master/docs/api/shell.md#openExternal) - [webviewTag.capturePage([rect, ]callback)](https://github.com/electron/electron/blob/master/docs/api/webview-tag.md#capturePage) diff --git a/lib/browser/api/dialog.js b/lib/browser/api/dialog.js index 4e7032d38525d..272735fbea031 100644 --- a/lib/browser/api/dialog.js +++ b/lib/browser/api/dialog.js @@ -70,6 +70,33 @@ const checkAppInitialized = function () { } } +const saveDialog = (sync, window, options) => { + checkAppInitialized() + + if (window.constructor !== BrowserWindow) options = window + if (options == null) options = { title: 'Save' } + + const { + buttonLabel = '', + defaultPath = '', + filters = [], + title = '', + message = '', + securityScopedBookmarks = false, + nameFieldLabel = '', + showsTagField = true + } = options + + if (typeof title !== 'string') throw new TypeError('Title must be a string') + if (typeof buttonLabel !== 'string') throw new TypeError('Button label must be a string') + if (typeof defaultPath !== 'string') throw new TypeError('Default path must be a string') + if (typeof message !== 'string') throw new TypeError('Message must be a string') + if (typeof nameFieldLabel !== 'string') throw new TypeError('Name field label must be a string') + + const settings = { buttonLabel, defaultPath, filters, title, message, securityScopedBookmarks, nameFieldLabel, showsTagField, window } + return (sync) ? binding.showSaveDialogSync(settings) : binding.showSaveDialog(settings) +} + const openDialog = (sync, window, options) => { checkAppInitialized() @@ -115,65 +142,17 @@ module.exports = { showOpenDialog: function (window, options) { return openDialog(false, window, options) }, + showOpenDialogSync: function (window, options) { return openDialog(true, window, options) }, - showSaveDialog: function (...args) { - checkAppInitialized() - - let [window, options, callback] = parseArgs(...args) - if (options == null) { - options = { - title: 'Save' - } - } - - let { buttonLabel, defaultPath, filters, title, message, securityScopedBookmarks = false, nameFieldLabel, showsTagField } = options - - if (title == null) { - title = '' - } else if (typeof title !== 'string') { - throw new TypeError('Title must be a string') - } - - if (buttonLabel == null) { - buttonLabel = '' - } else if (typeof buttonLabel !== 'string') { - throw new TypeError('Button label must be a string') - } - - if (defaultPath == null) { - defaultPath = '' - } else if (typeof defaultPath !== 'string') { - throw new TypeError('Default path must be a string') - } - - if (filters == null) { - filters = [] - } - - if (message == null) { - message = '' - } else if (typeof message !== 'string') { - throw new TypeError('Message must be a string') - } - - if (nameFieldLabel == null) { - nameFieldLabel = '' - } else if (typeof nameFieldLabel !== 'string') { - throw new TypeError('Name field label must be a string') - } - - if (showsTagField == null) { - showsTagField = true - } + showSaveDialog: function (window, options) { + return saveDialog(false, window, options) + }, - const wrappedCallback = typeof callback === 'function' ? function (success, result, bookmarkData) { - return success ? callback(result, bookmarkData) : callback() - } : null - const settings = { title, buttonLabel, defaultPath, filters, message, securityScopedBookmarks, nameFieldLabel, showsTagField, window } - return binding.showSaveDialog(settings, wrappedCallback) + showSaveDialogSync: function (window, options) { + return saveDialog(true, window, options) }, showMessageBox: function (...args) { @@ -291,6 +270,7 @@ module.exports = { } module.exports.showOpenDialog = deprecate.promisify(module.exports.showOpenDialog) +module.exports.showSaveDialog = deprecate.promisify(module.exports.showSaveDialog) // Mark standard asynchronous functions. v8Util.setHiddenValue(module.exports.showMessageBox, 'asynchronous', true) From 27336978193d4e09819811d7c334560a78afa5fc Mon Sep 17 00:00:00 2001 From: Milan Burda Date: Wed, 6 Mar 2019 22:22:45 +0100 Subject: [PATCH 09/22] refactor: make ELECTRON_INSPECTOR_SELECT_FILE handler async (#17235) --- lib/browser/chrome-devtools.js | 32 +++++++++++++------------------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/lib/browser/chrome-devtools.js b/lib/browser/chrome-devtools.js index b3aa84f0f9493..2204b3d163f6d 100644 --- a/lib/browser/chrome-devtools.js +++ b/lib/browser/chrome-devtools.js @@ -3,9 +3,12 @@ const { dialog, Menu } = require('electron') const fs = require('fs') const url = require('url') +const util = require('util') const ipcMainUtils = require('@electron/internal/browser/ipc-main-internal-utils') +const readFile = util.promisify(fs.readFile) + const convertToMenuTemplate = function (event, items) { return items.map(function (item) { const transformed = item.type === 'subMenu' ? { @@ -82,25 +85,16 @@ ipcMainUtils.handle('ELECTRON_INSPECTOR_CONTEXT_MENU', function (event, items, i menu.popup({ window }) }) -ipcMainUtils.handle('ELECTRON_INSPECTOR_SELECT_FILE', function (event) { - return new Promise((resolve, reject) => { - assertChromeDevTools(event.sender, 'window.UI.createFileSelectorElement()') - - dialog.showOpenDialog({}, function (result) { - if (!result.canceled) { - const path = result.filePaths[0] - fs.readFile(path, (error, data) => { - if (error) { - reject(error) - } else { - resolve([path, data]) - } - }) - } else { - resolve([]) - } - }) - }) +ipcMainUtils.handle('ELECTRON_INSPECTOR_SELECT_FILE', async function (event) { + assertChromeDevTools(event.sender, 'window.UI.createFileSelectorElement()') + + const result = await dialog.showOpenDialog({}) + if (result.canceled) return [] + + const path = result.filePaths[0] + const data = await readFile(path) + + return [path, data] }) ipcMainUtils.handle('ELECTRON_INSPECTOR_CONFIRM', function (event, message, title) { From 5422fd9941219156edfe14f222ede567ee275b6b Mon Sep 17 00:00:00 2001 From: Heilig Benedek Date: Thu, 7 Mar 2019 02:55:15 +0100 Subject: [PATCH 10/22] fix: backport upstream fixes for color chooser dialogs (#17227) * fix: backport upstream fixes for color chooser dialogs * chore: fix patches, Windows bad, linux good * Update color_chooser_mac.patch * Update color_chooser_win.patch --- patches/common/chromium/.patches | 3 +- patches/common/chromium/color_chooser.patch | 23 --- .../common/chromium/color_chooser_mac.patch | 186 ++++++++++++++++++ .../common/chromium/color_chooser_win.patch | 92 +++++++++ 4 files changed, 280 insertions(+), 24 deletions(-) delete mode 100644 patches/common/chromium/color_chooser.patch create mode 100644 patches/common/chromium/color_chooser_mac.patch create mode 100644 patches/common/chromium/color_chooser_win.patch diff --git a/patches/common/chromium/.patches b/patches/common/chromium/.patches index 5228b559cfd7f..f324126af55ad 100644 --- a/patches/common/chromium/.patches +++ b/patches/common/chromium/.patches @@ -60,7 +60,6 @@ content_browser_main_loop.patch dump_syms.patch command-ismediakey.patch tts.patch -color_chooser.patch printing.patch verbose_generate_breakpad_symbols.patch cross_site_document_resource_handler.patch @@ -71,3 +70,5 @@ disable_time_ticks_dcheck.patch autofill_size_calculation.patch revert_build_swiftshader_for_arm32.patch fix_backup_includes_for_ptrace_get_thread_area_on_arm_arm64.patch +color_chooser_mac.patch +color_chooser_win.patch diff --git a/patches/common/chromium/color_chooser.patch b/patches/common/chromium/color_chooser.patch deleted file mode 100644 index aff91e9fd12db..0000000000000 --- a/patches/common/chromium/color_chooser.patch +++ /dev/null @@ -1,23 +0,0 @@ -From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 -From: Heilig Benedek -Date: Thu, 18 Oct 2018 17:08:13 -0700 -Subject: color_chooser.patch - -Disables a DCHECK that crashes the ColorChooser on Windows, -that DCHECK most likely is an artifact that remained in chromium from a -long time ago (last update of that part of the code was around 2012-2013, -and this is purely UI, I don't think they have automated tests for it). - -diff --git a/chrome/browser/ui/views/color_chooser_win.cc b/chrome/browser/ui/views/color_chooser_win.cc -index 434842ba0f81206a43fb26874930bf7309782890..93b4152003eaea05f0e16cd049687fbcbc672fb0 100644 ---- a/chrome/browser/ui/views/color_chooser_win.cc -+++ b/chrome/browser/ui/views/color_chooser_win.cc -@@ -91,7 +91,7 @@ void ColorChooserWin::OnColorChooserDialogClosed() { - color_chooser_dialog_->ListenerDestroyed(); - color_chooser_dialog_ = NULL; - } -- DCHECK(current_color_chooser_ == this); -+ // DCHECK(current_color_chooser_ == this); - current_color_chooser_ = NULL; - if (web_contents_) - web_contents_->DidEndColorChooser(); diff --git a/patches/common/chromium/color_chooser_mac.patch b/patches/common/chromium/color_chooser_mac.patch new file mode 100644 index 0000000000000..9607c03010e36 --- /dev/null +++ b/patches/common/chromium/color_chooser_mac.patch @@ -0,0 +1,186 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benedek Heilig +Date: Thu, 21 Feb 2019 17:04:30 +0000 +Subject: fix a crash when using color chooser on macos + +Backports an upstream fix I made to the color chooser dialog on macos. +This can be removed once that fix lands in a chromium version we use. +Below is the original commit message: + +Fix crash when closing color chooser dialog on macos + +For some reason, closing the color chooser dialog caused a crash on +macos. By using NSNotificationCenter instead of providing a delegate +the crash goes away. I got the idea for this from the comment about the +__target method already in the code. + +Change-Id: Ide35a455ff12decc9dd83b30c80b584ab376242b + +diff --git a/AUTHORS b/AUTHORS +index 374b1a177f30d0c4b415d5c9c0fc4e4673414d39..436be8a093831020453285f0c476dcf9bd42fe8d 100644 +--- a/AUTHORS ++++ b/AUTHORS +@@ -114,6 +114,7 @@ Ben Coe + Ben Fiola + Ben Karel + Ben Noordhuis ++Benedek Heilig + Benjamin Dupont + Benjamin Jemlich + Bernard Cafarelli +diff --git a/chrome/browser/ui/cocoa/color_chooser_mac.h b/chrome/browser/ui/cocoa/color_chooser_mac.h +index 511000dc7855938ceab31694149ddf9e2125e0e4..9dbcc9fe41786555f0fb16ac47c71ee8851c8dd5 100644 +--- a/chrome/browser/ui/cocoa/color_chooser_mac.h ++++ b/chrome/browser/ui/cocoa/color_chooser_mac.h +@@ -15,7 +15,7 @@ class ColorChooserMac; + + // A Listener class to act as a event target for NSColorPanel and send + // the results to the C++ class, ColorChooserMac. +-@interface ColorPanelCocoa : NSObject { ++@interface ColorPanelCocoa : NSObject { + @protected + // We don't call DidChooseColor if the change wasn't caused by the user + // interacting with the panel. +@@ -26,6 +26,8 @@ class ColorChooserMac; + + - (id)initWithChooser:(ColorChooserMac*)chooser; + ++- (void)windowWillClose:(NSNotification*)notification; ++ + // Called from NSColorPanel. + - (void)didChooseColor:(NSColorPanel*)panel; + +@@ -45,7 +47,7 @@ class ColorChooserMac : public content::ColorChooser { + + // Called from ColorPanelCocoa. + void DidChooseColorInColorPanel(SkColor color); +- void DidCloseColorPabel(); ++ void DidCloseColorPanel(); + + // Set the color programmatically. + void SetSelectedColor(SkColor color) override; +diff --git a/chrome/browser/ui/cocoa/color_chooser_mac.mm b/chrome/browser/ui/cocoa/color_chooser_mac.mm +index bf47154f0a880f1c6143065e5c6d060f1df73765..e7cdab7a23d96345cc6e8ec578a8616d132b7d51 100644 +--- a/chrome/browser/ui/cocoa/color_chooser_mac.mm ++++ b/chrome/browser/ui/cocoa/color_chooser_mac.mm +@@ -39,16 +39,18 @@ void ColorChooserMac::DidChooseColorInColorPanel(SkColor color) { + web_contents_->DidChooseColorInColorChooser(color); + } + +-void ColorChooserMac::DidCloseColorPabel() { ++void ColorChooserMac::DidCloseColorPanel() { + End(); + } + + void ColorChooserMac::End() { +- panel_.reset(); +- DCHECK(current_color_chooser_ == this); +- current_color_chooser_ = NULL; +- if (web_contents_) ++ if (panel_) { ++ panel_.reset(); ++ DCHECK(current_color_chooser_ == this); ++ current_color_chooser_ = NULL; ++ if (web_contents_) + web_contents_->DidEndColorChooser(); ++ } + } + + void ColorChooserMac::SetSelectedColor(SkColor color) { +@@ -67,9 +69,14 @@ void ColorChooserMac::SetSelectedColor(SkColor color) { + chooser_ = chooser; + NSColorPanel* panel = [NSColorPanel sharedColorPanel]; + [panel setShowsAlpha:NO]; +- [panel setDelegate:self]; + [panel setTarget:self]; + [panel setAction:@selector(didChooseColor:)]; ++ ++ [[NSNotificationCenter defaultCenter] ++ addObserver:self ++ selector:@selector(windowWillClose:) ++ name:NSWindowWillCloseNotification ++ object:panel]; + } + return self; + } +@@ -82,19 +89,21 @@ void ColorChooserMac::SetSelectedColor(SkColor color) { + // the ColorPanelCocoa is still the target. + BOOL respondsToPrivateTargetMethod = + [panel respondsToSelector:@selector(__target)]; +- +- if ([panel delegate] == self || +- (respondsToPrivateTargetMethod && [panel __target] == self)) { +- [panel setDelegate:nil]; ++ if (respondsToPrivateTargetMethod && [panel __target] == self) { + [panel setTarget:nil]; + [panel setAction:nullptr]; + } + ++ [[NSNotificationCenter defaultCenter] ++ removeObserver:self ++ name:NSWindowWillCloseNotification ++ object:panel]; ++ + [super dealloc]; + } + + - (void)windowWillClose:(NSNotification*)notification { +- chooser_->DidCloseColorPabel(); ++ chooser_->DidCloseColorPanel(); + nonUserChange_ = NO; + } + +diff --git a/chrome/browser/ui/cocoa/color_panel_cocoa_unittest.mm b/chrome/browser/ui/cocoa/color_panel_cocoa_unittest.mm +index 83185ceb9bbd29bf38fce7f72c23f3a5b15ba6c8..823365fdfc0bceceeed6f5edc00b3462715a4751 100644 +--- a/chrome/browser/ui/cocoa/color_panel_cocoa_unittest.mm ++++ b/chrome/browser/ui/cocoa/color_panel_cocoa_unittest.mm +@@ -28,28 +28,6 @@ class ColorPanelCocoaTest : public CocoaTest { + } + }; + +-TEST_F(ColorPanelCocoaTest, ClearTargetAndDelegateOnEnd) { +- NSColorPanel* nscolor_panel = [NSColorPanel sharedColorPanel]; +- @autoreleasepool { +- EXPECT_TRUE([nscolor_panel respondsToSelector:@selector(__target)]); +- +- // Create a ColorPanelCocoa. +- ColorChooserMac* color_chooser_mac = +- ColorChooserMac::Open(nullptr, SK_ColorBLACK); +- +- // Confirm the NSColorPanel's configuration by the ColorChooserMac's +- // ColorPanelCocoa. +- EXPECT_TRUE([nscolor_panel delegate]); +- EXPECT_TRUE([nscolor_panel __target]); +- +- // Release the ColorPanelCocoa and confirm it's no longer the NSColorPanel's +- // target or delegate. +- color_chooser_mac->End(); +- } +- EXPECT_EQ([nscolor_panel delegate], nil); +- EXPECT_EQ([nscolor_panel __target], nil); +-} +- + TEST_F(ColorPanelCocoaTest, ClearTargetOnEnd) { + NSColorPanel* nscolor_panel = [NSColorPanel sharedColorPanel]; + @autoreleasepool { +@@ -61,19 +39,12 @@ TEST_F(ColorPanelCocoaTest, ClearTargetOnEnd) { + + // Confirm the NSColorPanel's configuration by the ColorChooserMac's + // ColorPanelCocoa. +- EXPECT_TRUE([nscolor_panel delegate]); + EXPECT_TRUE([nscolor_panel __target]); + +- // Clear the delegate and release the ColorPanelCocoa. +- [nscolor_panel setDelegate:nil]; +- + // Release the ColorPanelCocoa. + color_chooser_mac->End(); + } +- // Confirm the ColorPanelCocoa is no longer the NSColorPanel's target or +- // delegate. Previously the ColorPanelCocoa would not clear the target if +- // the delegate had already been cleared. +- EXPECT_EQ([nscolor_panel delegate], nil); ++ // Confirm the ColorPanelCocoa is no longer the NSColorPanel's target + EXPECT_EQ([nscolor_panel __target], nil); + } + diff --git a/patches/common/chromium/color_chooser_win.patch b/patches/common/chromium/color_chooser_win.patch new file mode 100644 index 0000000000000..20fe0df1b9737 --- /dev/null +++ b/patches/common/chromium/color_chooser_win.patch @@ -0,0 +1,92 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benedek Heilig +Date: Thu, 21 Feb 2019 17:16:33 +0000 +Subject: fix some DCHECKs on Windows when using color chooser dialog + +Backports an upstream fix I made to the color chooser dialog on +Windows. This can be removed once that fix lands in a chromium version we +use. Below is the original commit message: + +Fix DCHECKs being triggered while using color chooser dialog on Windows + +This fixes two DHCECKs being triggered on Windows when using the +dialog in a debug build. The main source of these triggered DCHECKs was +that when the user closes the dialog, OnColorChooserDialog is called, +which calls WebContentsImpl::DidEndColorChooser, which calls End on the +dialog, which calls OnColorChooserDialog again. This also happened on +macos. The other DCHECK was the receiver->HasAtLeastOneRef() one, +because ColorChooserDialog had a PostTask in it's constructor. + +Change-Id: I45ec32083a5850e006034073bc29d7ec4bb63189 + +diff --git a/chrome/browser/ui/views/color_chooser_dialog.cc b/chrome/browser/ui/views/color_chooser_dialog.cc +index c26be855120fcef78ce995d09fb3965f796746a9..1f568a2657250c5f21ab01da88950eedc6d4e177 100644 +--- a/chrome/browser/ui/views/color_chooser_dialog.cc ++++ b/chrome/browser/ui/views/color_chooser_dialog.cc +@@ -24,12 +24,14 @@ using content::BrowserThread; + // static + COLORREF ColorChooserDialog::g_custom_colors[16]; + +-ColorChooserDialog::ColorChooserDialog(views::ColorChooserListener* listener, +- SkColor initial_color, +- gfx::NativeWindow owning_window) ++ColorChooserDialog::ColorChooserDialog(views::ColorChooserListener* listener) + : listener_(listener) { + DCHECK(listener_); + CopyCustomColors(g_custom_colors, custom_colors_); ++} ++ ++void ColorChooserDialog::Open(SkColor initial_color, ++ gfx::NativeWindow owning_window) { + HWND owning_hwnd = views::HWNDForNativeWindow(owning_window); + + std::unique_ptr run_state = BeginRun(owning_hwnd); +diff --git a/chrome/browser/ui/views/color_chooser_dialog.h b/chrome/browser/ui/views/color_chooser_dialog.h +index 838e22dcd1a707714a098320d5bf8174fa60a2ea..b2558ce2a226ccde0f20de9712bf33051b21799c 100644 +--- a/chrome/browser/ui/views/color_chooser_dialog.h ++++ b/chrome/browser/ui/views/color_chooser_dialog.h +@@ -23,9 +23,9 @@ class ColorChooserDialog + public ui::BaseShellDialog, + public ui::BaseShellDialogImpl { + public: +- ColorChooserDialog(views::ColorChooserListener* listener, +- SkColor initial_color, +- gfx::NativeWindow owning_window); ++ explicit ColorChooserDialog(views::ColorChooserListener* listener); ++ ++ void Open(SkColor initial_color, gfx::NativeWindow owning_window); + + // BaseShellDialog: + bool IsRunning(gfx::NativeWindow owning_window) const override; +diff --git a/chrome/browser/ui/views/color_chooser_win.cc b/chrome/browser/ui/views/color_chooser_win.cc +index 434842ba0f81206a43fb26874930bf7309782890..ffc58eec78dc36e21c2c04aa0b0cd9097541d2f0 100644 +--- a/chrome/browser/ui/views/color_chooser_win.cc ++++ b/chrome/browser/ui/views/color_chooser_win.cc +@@ -61,9 +61,8 @@ ColorChooserWin::ColorChooserWin(content::WebContents* web_contents, + ->GetWidget() + ->GetView() + ->GetNativeView()); +- color_chooser_dialog_ = new ColorChooserDialog(this, +- initial_color, +- owning_window); ++ color_chooser_dialog_ = new ColorChooserDialog(this); ++ color_chooser_dialog_->Open(initial_color, owning_window); + } + + ColorChooserWin::~ColorChooserWin() { +@@ -90,11 +89,11 @@ void ColorChooserWin::OnColorChooserDialogClosed() { + if (color_chooser_dialog_.get()) { + color_chooser_dialog_->ListenerDestroyed(); + color_chooser_dialog_ = NULL; ++ DCHECK(current_color_chooser_ == this); ++ current_color_chooser_ = NULL; ++ if (web_contents_) ++ web_contents_->DidEndColorChooser(); + } +- DCHECK(current_color_chooser_ == this); +- current_color_chooser_ = NULL; +- if (web_contents_) +- web_contents_->DidEndColorChooser(); + } + + namespace chrome { From b575631bb03b455c9addbdb2cac5d392075d441e Mon Sep 17 00:00:00 2001 From: Jeremy Apthorp Date: Wed, 6 Mar 2019 17:59:53 -0800 Subject: [PATCH 11/22] fix: bad assertion in crypto functions that use BN_bn2bin_padded (#17220) * chore: roll node in particular, this picks up electron/node#70a78f07b, which fixes an issue with incorrect usage of the BN_bn2bin_padded API in boringssl * fix tests --- DEPS | 2 +- spec/node-spec.js | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/DEPS b/DEPS index 35cc92e111e34..dbb0de8ce7637 100644 --- a/DEPS +++ b/DEPS @@ -12,7 +12,7 @@ vars = { 'chromium_version': '73.0.3683.65', 'node_version': - 'fac6d766c143db8db05bb3b0c0871df8f032363c', + '70a78f07b1c4d53f3da462b08cef42a4ff8f949f', 'boto_version': 'f7574aa6cc2c819430c1f05e9a1a1a666ef8169b', 'pyyaml_version': '3.12', diff --git a/spec/node-spec.js b/spec/node-spec.js index fea62025c200e..566e906a301db 100644 --- a/spec/node-spec.js +++ b/spec/node-spec.js @@ -472,6 +472,21 @@ describe('node feature', () => { const iv = Buffer.from('fedcba9876543210', 'hex') require('crypto').createCipheriv('des-ede-cbc', key, iv) }) + + it('should not crash when getting an ECDH key', () => { + const ecdh = require('crypto').createECDH('prime256v1') + expect(ecdh.generateKeys()).to.be.an.instanceof(Buffer) + expect(ecdh.getPrivateKey()).to.be.an.instanceof(Buffer) + }) + + it('should not crash when generating DH keys or fetching DH fields', () => { + const dh = require('crypto').createDiffieHellman('modp15') + expect(dh.generateKeys()).to.be.an.instanceof(Buffer) + expect(dh.getPublicKey()).to.be.an.instanceof(Buffer) + expect(dh.getPrivateKey()).to.be.an.instanceof(Buffer) + expect(dh.getPrime()).to.be.an.instanceof(Buffer) + expect(dh.getGenerator()).to.be.an.instanceof(Buffer) + }) }) it('includes the electron version in process.versions', () => { From d4d6b9862f1a1cbc4199178cf80132956071fb8e Mon Sep 17 00:00:00 2001 From: Robo Date: Thu, 7 Mar 2019 20:50:03 +0530 Subject: [PATCH 12/22] fix: make StreamSubscriber ref counted (#17221) It is owned by URLRequestStreamJob on the IO thread once request starts, but if the ownership was abondoned while transfering it to IO thread which is possible when a request is aborted, then we need to make sure its destroyed on the right thread to avoid lock in v8. --- atom/browser/api/stream_subscriber.cc | 15 ++++++++----- atom/browser/api/stream_subscriber.h | 16 ++++++++++--- atom/browser/net/url_request_stream_job.cc | 26 +++++++++++----------- atom/browser/net/url_request_stream_job.h | 4 ++-- 4 files changed, 37 insertions(+), 24 deletions(-) diff --git a/atom/browser/api/stream_subscriber.cc b/atom/browser/api/stream_subscriber.cc index 7bdeb20ded354..9ac07cf056c33 100644 --- a/atom/browser/api/stream_subscriber.cc +++ b/atom/browser/api/stream_subscriber.cc @@ -19,12 +19,15 @@ namespace mate { StreamSubscriber::StreamSubscriber( v8::Isolate* isolate, v8::Local emitter, - base::WeakPtr url_job) - : isolate_(isolate), + base::WeakPtr url_job, + scoped_refptr ui_task_runner) + : base::RefCountedDeleteOnSequence(ui_task_runner), + isolate_(isolate), emitter_(isolate, emitter), url_job_(url_job), weak_factory_(this) { - DCHECK_CURRENTLY_ON(content::BrowserThread::UI); + DCHECK(ui_task_runner->RunsTasksInCurrentSequence()); + auto weak_self = weak_factory_.GetWeakPtr(); On("data", base::Bind(&StreamSubscriber::OnData, weak_self)); On("end", base::Bind(&StreamSubscriber::OnEnd, weak_self)); @@ -32,13 +35,12 @@ StreamSubscriber::StreamSubscriber( } StreamSubscriber::~StreamSubscriber() { - DCHECK_CURRENTLY_ON(content::BrowserThread::UI); RemoveAllListeners(); } void StreamSubscriber::On(const std::string& event, EventCallback&& callback) { // NOLINT - DCHECK_CURRENTLY_ON(content::BrowserThread::UI); + DCHECK(owning_task_runner()->RunsTasksInCurrentSequence()); DCHECK(js_handlers_.find(event) == js_handlers_.end()); v8::Locker locker(isolate_); @@ -52,7 +54,7 @@ void StreamSubscriber::On(const std::string& event, } void StreamSubscriber::Off(const std::string& event) { - DCHECK_CURRENTLY_ON(content::BrowserThread::UI); + DCHECK(owning_task_runner()->RunsTasksInCurrentSequence()); DCHECK(js_handlers_.find(event) != js_handlers_.end()); v8::Locker locker(isolate_); @@ -96,6 +98,7 @@ void StreamSubscriber::OnError(mate::Arguments* args) { } void StreamSubscriber::RemoveAllListeners() { + DCHECK(owning_task_runner()->RunsTasksInCurrentSequence()); v8::Locker locker(isolate_); v8::Isolate::Scope isolate_scope(isolate_); v8::HandleScope handle_scope(isolate_); diff --git a/atom/browser/api/stream_subscriber.h b/atom/browser/api/stream_subscriber.h index 3a766b92b52df..6f241075aef47 100644 --- a/atom/browser/api/stream_subscriber.h +++ b/atom/browser/api/stream_subscriber.h @@ -11,6 +11,8 @@ #include #include "base/callback.h" +#include "base/memory/ref_counted.h" +#include "base/memory/ref_counted_delete_on_sequence.h" #include "base/memory/weak_ptr.h" #include "content/public/browser/browser_thread.h" #include "v8/include/v8.h" @@ -23,17 +25,25 @@ namespace mate { class Arguments; -class StreamSubscriber { +class StreamSubscriber + : public base::RefCountedDeleteOnSequence { public: + REQUIRE_ADOPTION_FOR_REFCOUNTED_TYPE(); + StreamSubscriber(v8::Isolate* isolate, v8::Local emitter, - base::WeakPtr url_job); - ~StreamSubscriber(); + base::WeakPtr url_job, + scoped_refptr ui_task_runner); private: + friend class base::DeleteHelper; + friend class base::RefCountedDeleteOnSequence; + using JSHandlersMap = std::map>; using EventCallback = base::Callback; + ~StreamSubscriber(); + void On(const std::string& event, EventCallback&& callback); // NOLINT void Off(const std::string& event); diff --git a/atom/browser/net/url_request_stream_job.cc b/atom/browser/net/url_request_stream_job.cc index 5fc4114d0029a..5015cc98e1fa4 100644 --- a/atom/browser/net/url_request_stream_job.cc +++ b/atom/browser/net/url_request_stream_job.cc @@ -17,6 +17,7 @@ #include "base/strings/string_number_conversions.h" #include "base/strings/string_util.h" #include "base/task/post_task.h" +#include "base/threading/thread_task_runner_handle.h" #include "base/time/time.h" #include "content/public/browser/browser_task_traits.h" #include "native_mate/dictionary.h" @@ -84,14 +85,14 @@ void BeforeStartInUI(base::WeakPtr job, return; } - auto subscriber = std::make_unique( - args->isolate(), data.GetHandle(), job); + auto subscriber = base::MakeRefCounted( + args->isolate(), data.GetHandle(), job, + base::ThreadTaskRunnerHandle::Get()); base::PostTaskWithTraits( FROM_HERE, {content::BrowserThread::IO}, - base::BindOnce(&URLRequestStreamJob::StartAsync, job, - std::move(subscriber), base::RetainedRef(response_headers), - ended, error)); + base::BindOnce(&URLRequestStreamJob::StartAsync, job, subscriber, + base::RetainedRef(response_headers), ended, error)); } } // namespace @@ -106,10 +107,7 @@ URLRequestStreamJob::URLRequestStreamJob(net::URLRequest* request, weak_factory_(this) {} URLRequestStreamJob::~URLRequestStreamJob() { - if (subscriber_) { - content::BrowserThread::DeleteSoon(content::BrowserThread::UI, FROM_HERE, - std::move(subscriber_)); - } + DCHECK(!subscriber_ || subscriber_->HasOneRef()); } void URLRequestStreamJob::Start() { @@ -123,7 +121,7 @@ void URLRequestStreamJob::Start() { } void URLRequestStreamJob::StartAsync( - std::unique_ptr subscriber, + scoped_refptr subscriber, scoped_refptr response_headers, bool ended, int error) { @@ -135,7 +133,7 @@ void URLRequestStreamJob::StartAsync( ended_ = ended; response_headers_ = response_headers; - subscriber_ = std::move(subscriber); + subscriber_ = subscriber; request_start_time_ = base::TimeTicks::Now(); NotifyHeadersComplete(); } @@ -194,12 +192,14 @@ int URLRequestStreamJob::ReadRawData(net::IOBuffer* dest, int dest_size) { } void URLRequestStreamJob::DoneReading() { - content::BrowserThread::DeleteSoon(content::BrowserThread::UI, FROM_HERE, - std::move(subscriber_)); write_buffer_.clear(); } void URLRequestStreamJob::DoneReadingRedirectResponse() { + if (subscriber_) { + DCHECK(subscriber_->HasAtLeastOneRef()); + subscriber_ = nullptr; + } DoneReading(); } diff --git a/atom/browser/net/url_request_stream_job.h b/atom/browser/net/url_request_stream_job.h index e56e9f1699299..950fd9ce224b8 100644 --- a/atom/browser/net/url_request_stream_job.h +++ b/atom/browser/net/url_request_stream_job.h @@ -24,7 +24,7 @@ class URLRequestStreamJob : public JsAsker, public net::URLRequestJob { net::NetworkDelegate* network_delegate); ~URLRequestStreamJob() override; - void StartAsync(std::unique_ptr subscriber, + void StartAsync(scoped_refptr subscriber, scoped_refptr response_headers, bool ended, int error); @@ -62,7 +62,7 @@ class URLRequestStreamJob : public JsAsker, public net::URLRequestJob { base::TimeTicks request_start_time_; base::TimeTicks response_start_time_; scoped_refptr response_headers_; - std::unique_ptr subscriber_; + scoped_refptr subscriber_; base::WeakPtrFactory weak_factory_; From ab785e73aceb62183d42c8e4bd2e550eaf80e2a0 Mon Sep 17 00:00:00 2001 From: Electron Bot Date: Thu, 7 Mar 2019 12:34:10 -0800 Subject: [PATCH 13/22] Bump v6.0.0-nightly.20190307 --- ELECTRON_VERSION | 2 +- atom/browser/resources/mac/Info.plist | 4 ++-- atom/browser/resources/win/atom.rc | 4 ++-- atom/common/atom_version.h | 2 +- package-lock.json | 2 +- package.json | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/ELECTRON_VERSION b/ELECTRON_VERSION index 51b1d4e500c3d..08bfd3bb80ff7 100644 --- a/ELECTRON_VERSION +++ b/ELECTRON_VERSION @@ -1 +1 @@ -6.0.0-nightly.20190227 \ No newline at end of file +6.0.0-nightly.20190307 \ No newline at end of file diff --git a/atom/browser/resources/mac/Info.plist b/atom/browser/resources/mac/Info.plist index 721b7b6311d65..fdc6be879d986 100644 --- a/atom/browser/resources/mac/Info.plist +++ b/atom/browser/resources/mac/Info.plist @@ -17,9 +17,9 @@ CFBundleIconFile electron.icns CFBundleVersion - 6.0.0-nightly.20190227 + 6.0.0-nightly.20190307 CFBundleShortVersionString - 6.0.0-nightly.20190227 + 6.0.0-nightly.20190307 LSApplicationCategoryType public.app-category.developer-tools LSMinimumSystemVersion diff --git a/atom/browser/resources/win/atom.rc b/atom/browser/resources/win/atom.rc index 621f141c1f496..53d2834a9f912 100644 --- a/atom/browser/resources/win/atom.rc +++ b/atom/browser/resources/win/atom.rc @@ -50,8 +50,8 @@ END // VS_VERSION_INFO VERSIONINFO - FILEVERSION 6,0,0,20190227 - PRODUCTVERSION 6,0,0,20190227 + FILEVERSION 6,0,0,20190307 + PRODUCTVERSION 6,0,0,20190307 FILEFLAGSMASK 0x3fL #ifdef _DEBUG FILEFLAGS 0x1L diff --git a/atom/common/atom_version.h b/atom/common/atom_version.h index 7b0f24e03642e..35a50832a2e69 100644 --- a/atom/common/atom_version.h +++ b/atom/common/atom_version.h @@ -9,7 +9,7 @@ #define ATOM_MINOR_VERSION 0 #define ATOM_PATCH_VERSION 0 // clang-format off -#define ATOM_PRE_RELEASE_VERSION -nightly.20190227 +#define ATOM_PRE_RELEASE_VERSION -nightly.20190307 // clang-format on #ifndef ATOM_STRINGIFY diff --git a/package-lock.json b/package-lock.json index 0b65f3ce03e95..1f0eda0705ebb 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "electron", - "version": "6.0.0-nightly.20190227", + "version": "6.0.0-nightly.20190307", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index dc8c684ec1745..c0379db775cc9 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "electron", - "version": "6.0.0-nightly.20190227", + "version": "6.0.0-nightly.20190307", "repository": "https://github.com/electron/electron", "description": "Build cross platform desktop apps with JavaScript, HTML, and CSS", "devDependencies": { From 229934ec669436632ac1455fd921bd43ea0e4b2f Mon Sep 17 00:00:00 2001 From: Electron Bot Date: Thu, 7 Mar 2019 12:47:33 -0800 Subject: [PATCH 14/22] Revert "Bump v6.0.0-nightly.20190307" This reverts commit ab785e73aceb62183d42c8e4bd2e550eaf80e2a0. --- ELECTRON_VERSION | 2 +- atom/browser/resources/mac/Info.plist | 4 ++-- atom/browser/resources/win/atom.rc | 4 ++-- atom/common/atom_version.h | 2 +- package-lock.json | 2 +- package.json | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/ELECTRON_VERSION b/ELECTRON_VERSION index 08bfd3bb80ff7..51b1d4e500c3d 100644 --- a/ELECTRON_VERSION +++ b/ELECTRON_VERSION @@ -1 +1 @@ -6.0.0-nightly.20190307 \ No newline at end of file +6.0.0-nightly.20190227 \ No newline at end of file diff --git a/atom/browser/resources/mac/Info.plist b/atom/browser/resources/mac/Info.plist index fdc6be879d986..721b7b6311d65 100644 --- a/atom/browser/resources/mac/Info.plist +++ b/atom/browser/resources/mac/Info.plist @@ -17,9 +17,9 @@ CFBundleIconFile electron.icns CFBundleVersion - 6.0.0-nightly.20190307 + 6.0.0-nightly.20190227 CFBundleShortVersionString - 6.0.0-nightly.20190307 + 6.0.0-nightly.20190227 LSApplicationCategoryType public.app-category.developer-tools LSMinimumSystemVersion diff --git a/atom/browser/resources/win/atom.rc b/atom/browser/resources/win/atom.rc index 53d2834a9f912..621f141c1f496 100644 --- a/atom/browser/resources/win/atom.rc +++ b/atom/browser/resources/win/atom.rc @@ -50,8 +50,8 @@ END // VS_VERSION_INFO VERSIONINFO - FILEVERSION 6,0,0,20190307 - PRODUCTVERSION 6,0,0,20190307 + FILEVERSION 6,0,0,20190227 + PRODUCTVERSION 6,0,0,20190227 FILEFLAGSMASK 0x3fL #ifdef _DEBUG FILEFLAGS 0x1L diff --git a/atom/common/atom_version.h b/atom/common/atom_version.h index 35a50832a2e69..7b0f24e03642e 100644 --- a/atom/common/atom_version.h +++ b/atom/common/atom_version.h @@ -9,7 +9,7 @@ #define ATOM_MINOR_VERSION 0 #define ATOM_PATCH_VERSION 0 // clang-format off -#define ATOM_PRE_RELEASE_VERSION -nightly.20190307 +#define ATOM_PRE_RELEASE_VERSION -nightly.20190227 // clang-format on #ifndef ATOM_STRINGIFY diff --git a/package-lock.json b/package-lock.json index 1f0eda0705ebb..0b65f3ce03e95 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "electron", - "version": "6.0.0-nightly.20190307", + "version": "6.0.0-nightly.20190227", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index c0379db775cc9..dc8c684ec1745 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "electron", - "version": "6.0.0-nightly.20190307", + "version": "6.0.0-nightly.20190227", "repository": "https://github.com/electron/electron", "description": "Build cross platform desktop apps with JavaScript, HTML, and CSS", "devDependencies": { From 8c4d6438de5f374e39afe144773be4d05675d260 Mon Sep 17 00:00:00 2001 From: Electron Bot Date: Thu, 7 Mar 2019 12:51:33 -0800 Subject: [PATCH 15/22] Bump v6.0.0-nightly.20190307 --- ELECTRON_VERSION | 2 +- atom/browser/resources/mac/Info.plist | 4 ++-- atom/browser/resources/win/atom.rc | 4 ++-- atom/common/atom_version.h | 2 +- package-lock.json | 2 +- package.json | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/ELECTRON_VERSION b/ELECTRON_VERSION index 51b1d4e500c3d..08bfd3bb80ff7 100644 --- a/ELECTRON_VERSION +++ b/ELECTRON_VERSION @@ -1 +1 @@ -6.0.0-nightly.20190227 \ No newline at end of file +6.0.0-nightly.20190307 \ No newline at end of file diff --git a/atom/browser/resources/mac/Info.plist b/atom/browser/resources/mac/Info.plist index 721b7b6311d65..fdc6be879d986 100644 --- a/atom/browser/resources/mac/Info.plist +++ b/atom/browser/resources/mac/Info.plist @@ -17,9 +17,9 @@ CFBundleIconFile electron.icns CFBundleVersion - 6.0.0-nightly.20190227 + 6.0.0-nightly.20190307 CFBundleShortVersionString - 6.0.0-nightly.20190227 + 6.0.0-nightly.20190307 LSApplicationCategoryType public.app-category.developer-tools LSMinimumSystemVersion diff --git a/atom/browser/resources/win/atom.rc b/atom/browser/resources/win/atom.rc index 621f141c1f496..53d2834a9f912 100644 --- a/atom/browser/resources/win/atom.rc +++ b/atom/browser/resources/win/atom.rc @@ -50,8 +50,8 @@ END // VS_VERSION_INFO VERSIONINFO - FILEVERSION 6,0,0,20190227 - PRODUCTVERSION 6,0,0,20190227 + FILEVERSION 6,0,0,20190307 + PRODUCTVERSION 6,0,0,20190307 FILEFLAGSMASK 0x3fL #ifdef _DEBUG FILEFLAGS 0x1L diff --git a/atom/common/atom_version.h b/atom/common/atom_version.h index 7b0f24e03642e..35a50832a2e69 100644 --- a/atom/common/atom_version.h +++ b/atom/common/atom_version.h @@ -9,7 +9,7 @@ #define ATOM_MINOR_VERSION 0 #define ATOM_PATCH_VERSION 0 // clang-format off -#define ATOM_PRE_RELEASE_VERSION -nightly.20190227 +#define ATOM_PRE_RELEASE_VERSION -nightly.20190307 // clang-format on #ifndef ATOM_STRINGIFY diff --git a/package-lock.json b/package-lock.json index 0b65f3ce03e95..1f0eda0705ebb 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "electron", - "version": "6.0.0-nightly.20190227", + "version": "6.0.0-nightly.20190307", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index dc8c684ec1745..c0379db775cc9 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "electron", - "version": "6.0.0-nightly.20190227", + "version": "6.0.0-nightly.20190307", "repository": "https://github.com/electron/electron", "description": "Build cross platform desktop apps with JavaScript, HTML, and CSS", "devDependencies": { From 5581990d78e3601c00775dcbdcc48ca94d898015 Mon Sep 17 00:00:00 2001 From: Felix Rieseberg Date: Thu, 7 Mar 2019 12:56:02 -0800 Subject: [PATCH 16/22] build: Update TypeScript, use @typescript-eslint (#17251) * build: Update TypeScript to v3.3 * build: Update TypeScript, use @typescript-eslint --- .eslintrc.json | 20 ++++- default_app/.eslintrc | 8 -- lib/.eslintrc | 8 -- lib/browser/api/content-tracing.js | 5 +- lib/browser/api/protocol.ts | 4 +- lib/common/api/deprecate.ts | 3 +- package-lock.json | 136 ++++++++++++++++------------- package.json | 7 +- tsconfig.json | 1 + typings/internal-electron.d.ts | 4 +- 10 files changed, 104 insertions(+), 92 deletions(-) delete mode 100644 default_app/.eslintrc delete mode 100644 lib/.eslintrc diff --git a/.eslintrc.json b/.eslintrc.json index 4c34539e0edc0..93f6d6a96bd5d 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -1,7 +1,7 @@ { "extends": "standard", - "parser": "typescript-eslint-parser", - "plugins": ["typescript"], + "parser": "@typescript-eslint/parser", + "plugins": ["@typescript-eslint"], "env": { "browser": true }, @@ -9,7 +9,11 @@ "no-var": "error", "no-unused-vars": 0, "no-global-assign": 0, - "typescript/no-unused-vars": "error", + "@typescript-eslint/no-unused-vars": ["error", { + "vars": "all", + "args": "after-used", + "ignoreRestSiblings": false + }], "prefer-const": ["error", { "destructuring": "all" }] @@ -17,5 +21,13 @@ "parserOptions": { "ecmaVersion": 6, "sourceType": "module" - } + }, + "overrides": [ + { + "files": "*.js", + "rules": { + "@typescript-eslint/no-unused-vars": "off" + } + } + ] } diff --git a/default_app/.eslintrc b/default_app/.eslintrc deleted file mode 100644 index 7f629e4735d80..0000000000000 --- a/default_app/.eslintrc +++ /dev/null @@ -1,8 +0,0 @@ -{ - "parserOptions": { - "sourceType": "script" - }, - "rules": { - "strict": ["error", "global"] - } -} diff --git a/lib/.eslintrc b/lib/.eslintrc deleted file mode 100644 index 7f629e4735d80..0000000000000 --- a/lib/.eslintrc +++ /dev/null @@ -1,8 +0,0 @@ -{ - "parserOptions": { - "sourceType": "script" - }, - "rules": { - "strict": ["error", "global"] - } -} diff --git a/lib/browser/api/content-tracing.js b/lib/browser/api/content-tracing.js index af92f4621539e..c915ec3463362 100644 --- a/lib/browser/api/content-tracing.js +++ b/lib/browser/api/content-tracing.js @@ -6,8 +6,9 @@ contentTracing.getCategories = deprecate.promisify(contentTracing.getCategories) contentTracing.startRecording = deprecate.promisify(contentTracing.startRecording) contentTracing.stopRecording = deprecate.promisify(contentTracing.stopRecording) contentTracing.getTraceBufferUsage = deprecate.promisifyMultiArg( - contentTracing.getTraceBufferUsage, - (value) => [value.paths, value.bookmarks] + contentTracing.getTraceBufferUsage + // convertPromiseValue: Temporarily disabled until it's used + /* (value) => [value.paths, value.bookmarks] */ ) module.exports = contentTracing diff --git a/lib/browser/api/protocol.ts b/lib/browser/api/protocol.ts index e7940e3437264..8045cd3f73fbf 100644 --- a/lib/browser/api/protocol.ts +++ b/lib/browser/api/protocol.ts @@ -5,7 +5,7 @@ const protocol = process.atomBinding('protocol') // Fallback protocol APIs of default session. Object.setPrototypeOf(protocol, new Proxy({}, { - get (target, property) { + get (_target, property) { if (!app.isReady()) return const protocol = session.defaultSession!.protocol @@ -21,7 +21,7 @@ Object.setPrototypeOf(protocol, new Proxy({}, { return Object.getOwnPropertyNames(Object.getPrototypeOf(session.defaultSession!.protocol)) }, - getOwnPropertyDescriptor (target) { + getOwnPropertyDescriptor () { return { configurable: true, enumerable: true } } })) diff --git a/lib/common/api/deprecate.ts b/lib/common/api/deprecate.ts index 1238c99343976..896f04a118fe3 100644 --- a/lib/common/api/deprecate.ts +++ b/lib/common/api/deprecate.ts @@ -102,7 +102,8 @@ const deprecate: ElectronInternal.DeprecationUtil = { } as T }, - promisifyMultiArg: any>(fn: T, convertPromiseValue: (v: any) => any): T => { + // convertPromiseValue: Temporarily disabled until it's used + promisifyMultiArg: any>(fn: T /* convertPromiseValue: (v: any) => any */): T => { const fnName = fn.name || 'function' const oldName = `${fnName} with callbacks` const newName = `${fnName} with Promises` diff --git a/package-lock.json b/package-lock.json index 1f0eda0705ebb..bc3550d69dd03 100644 --- a/package-lock.json +++ b/package-lock.json @@ -161,6 +161,64 @@ "integrity": "sha512-CBgLNk4o3XMnqMc0rhb6lc77IwShMEglz05deDcn2lQxyXEZivfwgYJu7SMha9V5XcrP6qZuevTHV/QrN2vjKQ==", "dev": true }, + "@typescript-eslint/eslint-plugin": { + "version": "1.4.2", + "resolved": "https://registry.npmjs.org/@typescript-eslint/eslint-plugin/-/eslint-plugin-1.4.2.tgz", + "integrity": "sha512-6WInypy/cK4rM1dirKbD5p7iFW28DbSRKT/+PGn+DYzBWEvHq5KnZAqQ5cX25JBc0qMkFxJNxNfBbFXJyyzVcw==", + "dev": true, + "requires": { + "@typescript-eslint/parser": "1.4.2", + "@typescript-eslint/typescript-estree": "1.4.2", + "requireindex": "^1.2.0", + "tsutils": "^3.7.0" + }, + "dependencies": { + "requireindex": { + "version": "1.2.0", + "resolved": "https://registry.npmjs.org/requireindex/-/requireindex-1.2.0.tgz", + "integrity": "sha512-L9jEkOi3ASd9PYit2cwRfyppc9NoABujTP8/5gFcbERmo5jUoAKovIC3fsF17pkTnGsrByysqX+Kxd2OTNI1ww==", + "dev": true + }, + "tsutils": { + "version": "3.8.0", + "resolved": "https://registry.npmjs.org/tsutils/-/tsutils-3.8.0.tgz", + "integrity": "sha512-XQdPhgcoTbCD8baXC38PQ0vpTZ8T3YrE+vR66YIj/xvDt1//8iAhafpIT/4DmvzzC1QFapEImERu48Pa01dIUA==", + "dev": true, + "requires": { + "tslib": "^1.8.1" + } + } + } + }, + "@typescript-eslint/parser": { + "version": "1.4.2", + "resolved": "https://registry.npmjs.org/@typescript-eslint/parser/-/parser-1.4.2.tgz", + "integrity": "sha512-OqLkY9295DXXaWToItUv3olO2//rmzh6Th6Sc7YjFFEpEuennsm5zhygLLvHZjPxPlzrQgE8UDaOPurDylaUuw==", + "dev": true, + "requires": { + "@typescript-eslint/typescript-estree": "1.4.2", + "eslint-scope": "^4.0.0", + "eslint-visitor-keys": "^1.0.0" + } + }, + "@typescript-eslint/typescript-estree": { + "version": "1.4.2", + "resolved": "https://registry.npmjs.org/@typescript-eslint/typescript-estree/-/typescript-estree-1.4.2.tgz", + "integrity": "sha512-wKgi/w6k1v3R4b6oDc20cRWro2gBzp0wn6CAeYC8ExJMfvXMfiaXzw2tT9ilxdONaVWMCk7B9fMdjos7bF/CWw==", + "dev": true, + "requires": { + "lodash.unescape": "4.0.1", + "semver": "5.5.0" + }, + "dependencies": { + "semver": { + "version": "5.5.0", + "resolved": "https://registry.npmjs.org/semver/-/semver-5.5.0.tgz", + "integrity": "sha512-4SJ3dm0WAwWy/NVeioZh5AntkdJoWKxHxcmyP622fOkgHa4z3R0TdBJICINyaSDE6uNwVc8gZr+ZinwZAH4xIA==", + "dev": true + } + } + }, "JSONStream": { "version": "1.3.2", "resolved": "https://registry.npmjs.org/JSONStream/-/JSONStream-1.3.2.tgz", @@ -4622,8 +4680,7 @@ "version": "2.1.1", "resolved": false, "integrity": "sha1-w7M6te42DYbg5ijwRorn7yfWVN8=", - "dev": true, - "optional": true + "dev": true }, "aproba": { "version": "1.2.0", @@ -4647,15 +4704,13 @@ "version": "1.0.0", "resolved": false, "integrity": "sha1-ibTRmasr7kneFk6gK4nORi1xt2c=", - "dev": true, - "optional": true + "dev": true }, "brace-expansion": { "version": "1.1.11", "resolved": false, "integrity": "sha512-iCuPHDFgrHX7H2vEI/5xpz07zSHB00TpugqhmYtVmMO6518mCuRMoOYFldEBl0g187ufozdaHgWKcYFb61qGiA==", "dev": true, - "optional": true, "requires": { "balanced-match": "^1.0.0", "concat-map": "0.0.1" @@ -4672,22 +4727,19 @@ "version": "1.1.0", "resolved": false, "integrity": "sha1-DQcLTQQ6W+ozovGkDi7bPZpMz3c=", - "dev": true, - "optional": true + "dev": true }, "concat-map": { "version": "0.0.1", "resolved": false, "integrity": "sha1-2Klr13/Wjfd5OnMDajug1UBdR3s=", - "dev": true, - "optional": true + "dev": true }, "console-control-strings": { "version": "1.1.0", "resolved": false, "integrity": "sha1-PXz0Rk22RG6mRL9LOVB/mFEAjo4=", - "dev": true, - "optional": true + "dev": true }, "core-util-is": { "version": "1.0.2", @@ -4818,8 +4870,7 @@ "version": "2.0.3", "resolved": false, "integrity": "sha1-Yzwsg+PaQqUC9SRmAiSA9CCCYd4=", - "dev": true, - "optional": true + "dev": true }, "ini": { "version": "1.3.5", @@ -4833,7 +4884,6 @@ "resolved": false, "integrity": "sha1-754xOG8DGn8NZDr4L95QxFfvAMs=", "dev": true, - "optional": true, "requires": { "number-is-nan": "^1.0.0" } @@ -4850,7 +4900,6 @@ "resolved": false, "integrity": "sha512-yJHVQEhyqPLUTgt9B83PXu6W3rx4MvvHvSUvToogpwoGDOUQ+yDrR0HRot+yOCdCO7u4hX3pWft6kWBBcqh0UA==", "dev": true, - "optional": true, "requires": { "brace-expansion": "^1.1.7" } @@ -4859,15 +4908,13 @@ "version": "0.0.8", "resolved": false, "integrity": "sha1-hX/Kv8M5fSYluCKCYuhqp6ARsF0=", - "dev": true, - "optional": true + "dev": true }, "minipass": { "version": "2.2.4", "resolved": false, "integrity": "sha512-hzXIWWet/BzWhYs2b+u7dRHlruXhwdgvlTMDKC6Cb1U7ps6Ac6yQlR39xsbjWJE377YTCtKwIXIpJ5oP+j5y8g==", "dev": true, - "optional": true, "requires": { "safe-buffer": "^5.1.1", "yallist": "^3.0.0" @@ -4888,7 +4935,6 @@ "resolved": false, "integrity": "sha1-MAV0OOrGz3+MR2fzhkjWaX11yQM=", "dev": true, - "optional": true, "requires": { "minimist": "0.0.8" } @@ -4977,8 +5023,7 @@ "version": "1.0.1", "resolved": false, "integrity": "sha1-CXtgK1NCKlIsGvuHkDGDNpQaAR0=", - "dev": true, - "optional": true + "dev": true }, "object-assign": { "version": "4.1.1", @@ -4992,7 +5037,6 @@ "resolved": false, "integrity": "sha1-WDsap3WWHUsROsF9nFC6753Xa9E=", "dev": true, - "optional": true, "requires": { "wrappy": "1" } @@ -5088,8 +5132,7 @@ "version": "5.1.1", "resolved": false, "integrity": "sha512-kKvNJn6Mm93gAczWVJg7wH+wGYWNrDHdWvpUmHyEsgCtIwwo3bqPtV4tR5tuPaUhTOo/kvhVwd8XwwOllGYkbg==", - "dev": true, - "optional": true + "dev": true }, "safer-buffer": { "version": "2.1.2", @@ -5131,7 +5174,6 @@ "resolved": false, "integrity": "sha1-EYvfW4zcUaKn5w0hHgfisLmxB9M=", "dev": true, - "optional": true, "requires": { "code-point-at": "^1.0.0", "is-fullwidth-code-point": "^1.0.0", @@ -5153,7 +5195,6 @@ "resolved": false, "integrity": "sha1-ajhfuIU9lS1f8F0Oiq+UJ43GPc8=", "dev": true, - "optional": true, "requires": { "ansi-regex": "^2.0.0" } @@ -5202,15 +5243,13 @@ "version": "1.0.2", "resolved": false, "integrity": "sha1-tSQ9jz7BqjXxNkYFvA0QNuMKtp8=", - "dev": true, - "optional": true + "dev": true }, "yallist": { "version": "3.0.2", "resolved": false, "integrity": "sha1-hFK0u36Dx8GI2AQcGoN8dz1ti7k=", - "dev": true, - "optional": true + "dev": true } } }, @@ -12608,40 +12647,11 @@ "dev": true }, "typescript": { - "version": "3.1.6", - "resolved": "https://registry.npmjs.org/typescript/-/typescript-3.1.6.tgz", - "integrity": "sha512-tDMYfVtvpb96msS1lDX9MEdHrW4yOuZ4Kdc4Him9oU796XldPYF/t2+uKoX0BBa0hXXwDlqYQbXY5Rzjzc5hBA==", + "version": "3.3.3333", + "resolved": "https://registry.npmjs.org/typescript/-/typescript-3.3.3333.tgz", + "integrity": "sha512-JjSKsAfuHBE/fB2oZ8NxtRTk5iGcg6hkYXMnZ3Wc+b2RSqejEqTaem11mHASMnFilHrax3sLK0GDzcJrekZYLw==", "dev": true }, - "typescript-eslint-parser": { - "version": "21.0.2", - "resolved": "https://registry.npmjs.org/typescript-eslint-parser/-/typescript-eslint-parser-21.0.2.tgz", - "integrity": "sha512-u+pj4RVJBr4eTzj0n5npoXD/oRthvfUCjSKndhNI714MG0mQq2DJw5WP7qmonRNIFgmZuvdDOH3BHm9iOjIAfg==", - "dev": true, - "requires": { - "eslint-scope": "^4.0.0", - "eslint-visitor-keys": "^1.0.0", - "typescript-estree": "5.3.0" - } - }, - "typescript-estree": { - "version": "5.3.0", - "resolved": "https://registry.npmjs.org/typescript-estree/-/typescript-estree-5.3.0.tgz", - "integrity": "sha512-Vu0KmYdSCkpae+J48wsFC1ti19Hq3Wi/lODUaE+uesc3gzqhWbZ5itWbsjylLVbjNW4K41RqDzSfnaYNbmEiMQ==", - "dev": true, - "requires": { - "lodash.unescape": "4.0.1", - "semver": "5.5.0" - }, - "dependencies": { - "semver": { - "version": "5.5.0", - "resolved": "https://registry.npmjs.org/semver/-/semver-5.5.0.tgz", - "integrity": "sha512-4SJ3dm0WAwWy/NVeioZh5AntkdJoWKxHxcmyP622fOkgHa4z3R0TdBJICINyaSDE6uNwVc8gZr+ZinwZAH4xIA==", - "dev": true - } - } - }, "uc.micro": { "version": "1.0.6", "resolved": "https://registry.npmjs.org/uc.micro/-/uc.micro-1.0.6.tgz", @@ -13544,4 +13554,4 @@ "dev": true } } -} \ No newline at end of file +} diff --git a/package.json b/package.json index c0379db775cc9..8518f428dfd0b 100644 --- a/package.json +++ b/package.json @@ -6,6 +6,8 @@ "devDependencies": { "@octokit/rest": "^16.3.2", "@types/node": "^10.12.21", + "@typescript-eslint/eslint-plugin": "^1.4.2", + "@typescript-eslint/parser": "^1.4.2", "aliasify": "^2.1.0", "asar": "^1.0.0", "browserify": "^16.2.3", @@ -40,8 +42,7 @@ "sumchecker": "^2.0.2", "temp": "^0.8.3", "tsify": "^4.0.1", - "typescript": "~3.1.1", - "typescript-eslint-parser": "^21.0.0" + "typescript": "~3.3.3333" }, "private": true, "scripts": { @@ -113,4 +114,4 @@ "git add filenames.auto.gni" ] } -} \ No newline at end of file +} diff --git a/tsconfig.json b/tsconfig.json index e2fbd028f2d59..562d65f3f90a9 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -12,6 +12,7 @@ "strict": true, "baseUrl": ".", "allowJs": true, + "noUnusedLocals": true, "outDir": "ts-gen", "paths": { "@electron/internal/*": ["lib/*"] diff --git a/typings/internal-electron.d.ts b/typings/internal-electron.d.ts index e5a09cc922df9..cc2566b936923 100644 --- a/typings/internal-electron.d.ts +++ b/typings/internal-electron.d.ts @@ -66,7 +66,9 @@ declare namespace ElectronInternal { renameProperty(object: T, oldName: string, newName: K): T; promisify any>(fn: T): T; - promisifyMultiArg any>(fn: T, convertPromiseValue: (v: any) => any): T; + + // convertPromiseValue: Temporarily disabled until it's used + promisifyMultiArg any>(fn: T, /*convertPromiseValue: (v: any) => any*/): T; } // Internal IPC has _replyInternal and NO reply method From 8ec304f32f9d22997074cb7c8240bfa7d66e6a88 Mon Sep 17 00:00:00 2001 From: Samuel Maddock Date: Thu, 7 Mar 2019 17:46:57 -0500 Subject: [PATCH 17/22] fix: run subframe preload bundles in isolated context (#17165) * fix: run subframe preload bundles in isolated context * test subframe contextIsolation when disabled --- atom/renderer/atom_render_frame_observer.cc | 23 +++++++++-- spec/api-subframe-spec.js | 43 +++++++++++++++++---- spec/fixtures/sub-frames/preload.js | 2 + 3 files changed, 56 insertions(+), 12 deletions(-) diff --git a/atom/renderer/atom_render_frame_observer.cc b/atom/renderer/atom_render_frame_observer.cc index 4be1fb85368bc..6bddad4d085db 100644 --- a/atom/renderer/atom_render_frame_observer.cc +++ b/atom/renderer/atom_render_frame_observer.cc @@ -12,6 +12,8 @@ #include "atom/common/heap_snapshot.h" #include "atom/common/native_mate_converters/value_converter.h" #include "atom/common/node_includes.h" +#include "atom/common/options_switches.h" +#include "base/command_line.h" #include "base/strings/string_number_conversions.h" #include "base/threading/thread_restrictions.h" #include "base/trace_event/trace_event.h" @@ -96,9 +98,18 @@ void AtomRenderFrameObserver::DidCreateScriptContext( if (ShouldNotifyClient(world_id)) renderer_client_->DidCreateScriptContext(context, render_frame_); - if (renderer_client_->isolated_world() && IsMainWorld(world_id) && - // Only the top window's main frame has isolated world. - render_frame_->IsMainFrame() && !render_frame_->GetWebFrame()->Opener()) { + bool use_context_isolation = renderer_client_->isolated_world(); + bool is_main_world = IsMainWorld(world_id); + bool is_main_frame = render_frame_->IsMainFrame(); + bool is_not_opened = !render_frame_->GetWebFrame()->Opener(); + bool allow_node_in_sub_frames = + base::CommandLine::ForCurrentProcess()->HasSwitch( + switches::kNodeIntegrationInSubFrames); + bool should_create_isolated_context = + use_context_isolation && is_main_world && + (is_main_frame || allow_node_in_sub_frames) && is_not_opened; + + if (should_create_isolated_context) { CreateIsolatedWorldContext(); renderer_client_->SetupMainWorldOverrides(context, render_frame_); } @@ -155,7 +166,11 @@ bool AtomRenderFrameObserver::IsIsolatedWorld(int world_id) { } bool AtomRenderFrameObserver::ShouldNotifyClient(int world_id) { - if (renderer_client_->isolated_world() && render_frame_->IsMainFrame()) + bool allow_node_in_sub_frames = + base::CommandLine::ForCurrentProcess()->HasSwitch( + switches::kNodeIntegrationInSubFrames); + if (renderer_client_->isolated_world() && + (render_frame_->IsMainFrame() || allow_node_in_sub_frames)) return IsIsolatedWorld(world_id); else return IsMainWorld(world_id); diff --git a/spec/api-subframe-spec.js b/spec/api-subframe-spec.js index dc31a370b3da5..df56873490bde 100644 --- a/spec/api-subframe-spec.js +++ b/spec/api-subframe-spec.js @@ -8,8 +8,8 @@ const { closeWindow } = require('./window-helpers') const { BrowserWindow } = remote describe('renderer nodeIntegrationInSubFrames', () => { - const generateTests = (sandboxEnabled) => { - describe(`with sandbox ${sandboxEnabled ? 'enabled' : 'disabled'}`, () => { + const generateTests = (description, webPreferences) => { + describe(description, () => { let w beforeEach(async () => { @@ -19,15 +19,17 @@ describe('renderer nodeIntegrationInSubFrames', () => { width: 400, height: 400, webPreferences: { - sandbox: sandboxEnabled, preload: path.resolve(__dirname, 'fixtures/sub-frames/preload.js'), - nodeIntegrationInSubFrames: true + nodeIntegrationInSubFrames: true, + ...webPreferences } }) }) afterEach(() => { - return closeWindow(w).then(() => { w = null }) + return closeWindow(w).then(() => { + w = null + }) }) it('should load preload scripts in top level iframes', async () => { @@ -74,15 +76,40 @@ describe('renderer nodeIntegrationInSubFrames', () => { it('should correctly reply to the nested sub-frames with using event.reply', async () => { const detailsPromise = emittedNTimes(remote.ipcMain, 'preload-ran', 3) w.loadFile(path.resolve(__dirname, 'fixtures/sub-frames/frame-with-frame-container.html')) - const [,, event3] = await detailsPromise + const [, , event3] = await detailsPromise const pongPromise = emittedOnce(remote.ipcMain, 'preload-pong') event3[0].reply('preload-ping') const details = await pongPromise expect(details[1]).to.equal(event3[0].frameId) }) + + it('should not expose globals in main world', async () => { + const detailsPromise = emittedNTimes(remote.ipcMain, 'preload-ran', 2) + w.loadFile(path.resolve(__dirname, 'fixtures/sub-frames/frame-container.html')) + const details = await detailsPromise + const senders = details.map(event => event[0].sender) + await new Promise((resolve) => { + let resultCount = 0 + senders.forEach(sender => { + sender.webContents.executeJavaScript('window.isolatedGlobal', result => { + if (webPreferences.contextIsolation) { + expect(result).to.be.null() + } else { + expect(result).to.equal(true) + } + resultCount++ + if (resultCount === senders.length) { + resolve() + } + }) + }) + }) + }) }) } - generateTests(false) - generateTests(true) + generateTests('without sandbox', {}) + generateTests('with sandbox', { sandbox: true }) + generateTests('with contextIsolation', { contextIsolation: true }) + generateTests('with contextIsolation + sandbox', { contextIsolation: true, sandbox: true }) }) diff --git a/spec/fixtures/sub-frames/preload.js b/spec/fixtures/sub-frames/preload.js index 3b6c461759b25..11c77a5281398 100644 --- a/spec/fixtures/sub-frames/preload.js +++ b/spec/fixtures/sub-frames/preload.js @@ -1,5 +1,7 @@ const { ipcRenderer, webFrame } = require('electron') +window.isolatedGlobal = true + ipcRenderer.send('preload-ran', window.location.href, webFrame.routingId) ipcRenderer.on('preload-ping', () => { From bb88a07a9414499952481b4b17e54eaa9f52922a Mon Sep 17 00:00:00 2001 From: Robo Date: Fri, 8 Mar 2019 04:18:22 +0530 Subject: [PATCH 18/22] fix: gn check errors in release build (#17274) --- BUILD.gn | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/BUILD.gn b/BUILD.gn index c4f0c3f62b9d3..3500af17f44db 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -358,6 +358,7 @@ static_library("electron_lib") { "//ppapi/shared_impl", "//printing/buildflags", "//services/audio/public/mojom:constants", + "//services/device/public/cpp/geolocation", "//services/device/public/mojom", "//services/proxy_resolver:lib", "//services/video_capture/public/mojom:constants", @@ -447,7 +448,6 @@ static_library("electron_lib") { "atom/browser/fake_location_provider.cc", "atom/browser/fake_location_provider.h", ] - deps += [ "//services/device/public/cpp/geolocation" ] } if (is_mac) { From 8c6bf9c8480ccfb60ce0b728c6e19a501a67e600 Mon Sep 17 00:00:00 2001 From: Roller Bot <42361097+roller-bot@users.noreply.github.com> Date: Thu, 7 Mar 2019 14:52:02 -0800 Subject: [PATCH 19/22] chore: bump chromium in DEPS to 73.0.3683.68 (#17262) --- DEPS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DEPS b/DEPS index dbb0de8ce7637..4c7cd0a23aeb6 100644 --- a/DEPS +++ b/DEPS @@ -10,7 +10,7 @@ gclient_gn_args = [ vars = { 'chromium_version': - '73.0.3683.65', + '73.0.3683.68', 'node_version': '70a78f07b1c4d53f3da462b08cef42a4ff8f949f', From f3fc4023cfc749807df8b6c34c61ba09e2d4b05d Mon Sep 17 00:00:00 2001 From: Felix Rieseberg Date: Thu, 7 Mar 2019 15:26:23 -0800 Subject: [PATCH 20/22] refactor: Port renderer/web-view to TypeScript (#17250) --- filenames.gni | 12 +- lib/renderer/init.ts | 3 +- lib/renderer/web-view/guest-view-internal.js | 105 ------------- lib/renderer/web-view/guest-view-internal.ts | 124 ++++++++++++++++ ...w-attributes.js => web-view-attributes.ts} | 138 ++++++++++-------- lib/renderer/web-view/web-view-constants.js | 28 ---- lib/renderer/web-view/web-view-constants.ts | 26 ++++ ...eb-view-element.js => web-view-element.ts} | 73 ++++----- .../{web-view-impl.js => web-view-impl.ts} | 136 ++++++++++------- .../{web-view-init.js => web-view-init.ts} | 15 +- lib/sandboxed_renderer/init.js | 3 +- typings/internal-ambient.d.ts | 53 ++++++- typings/internal-electron.d.ts | 28 ++++ 13 files changed, 442 insertions(+), 302 deletions(-) delete mode 100644 lib/renderer/web-view/guest-view-internal.js create mode 100644 lib/renderer/web-view/guest-view-internal.ts rename lib/renderer/web-view/{web-view-attributes.js => web-view-attributes.ts} (58%) delete mode 100644 lib/renderer/web-view/web-view-constants.js create mode 100644 lib/renderer/web-view/web-view-constants.ts rename lib/renderer/web-view/{web-view-element.js => web-view-element.ts} (52%) rename lib/renderer/web-view/{web-view-impl.js => web-view-impl.ts} (66%) rename lib/renderer/web-view/{web-view-init.js => web-view-init.ts} (67%) diff --git a/filenames.gni b/filenames.gni index cbdca3c822303..965b36ea77b23 100644 --- a/filenames.gni +++ b/filenames.gni @@ -73,12 +73,12 @@ filenames = { "lib/renderer/security-warnings.ts", "lib/renderer/window-setup.ts", "lib/renderer/web-frame-init.ts", - "lib/renderer/web-view/guest-view-internal.js", - "lib/renderer/web-view/web-view-attributes.js", - "lib/renderer/web-view/web-view-constants.js", - "lib/renderer/web-view/web-view-element.js", - "lib/renderer/web-view/web-view-impl.js", - "lib/renderer/web-view/web-view-init.js", + "lib/renderer/web-view/guest-view-internal.ts", + "lib/renderer/web-view/web-view-attributes.ts", + "lib/renderer/web-view/web-view-constants.ts", + "lib/renderer/web-view/web-view-element.ts", + "lib/renderer/web-view/web-view-impl.ts", + "lib/renderer/web-view/web-view-init.ts", "lib/renderer/api/exports/electron.js", "lib/renderer/api/crash-reporter.js", "lib/renderer/api/ipc-renderer.js", diff --git a/lib/renderer/init.ts b/lib/renderer/init.ts index e00756259bb92..dc8d2f35a52f6 100644 --- a/lib/renderer/init.ts +++ b/lib/renderer/init.ts @@ -93,7 +93,8 @@ switch (window.location.protocol) { // Load webview tag implementation. if (process.isMainFrame) { - require('@electron/internal/renderer/web-view/web-view-init')(contextIsolation, webviewTag, guestInstanceId) + const { webViewInit } = require('@electron/internal/renderer/web-view/web-view-init') + webViewInit(contextIsolation, webviewTag, guestInstanceId) } // Pass the arguments to isolatedWorld. diff --git a/lib/renderer/web-view/guest-view-internal.js b/lib/renderer/web-view/guest-view-internal.js deleted file mode 100644 index c66948dc04f99..0000000000000 --- a/lib/renderer/web-view/guest-view-internal.js +++ /dev/null @@ -1,105 +0,0 @@ -'use strict' - -const { webFrame } = require('electron') -const { ipcRendererInternal } = require('@electron/internal/renderer/ipc-renderer-internal') -const ipcRendererUtils = require('@electron/internal/renderer/ipc-renderer-internal-utils') - -const WEB_VIEW_EVENTS = { - 'load-commit': ['url', 'isMainFrame'], - 'did-attach': [], - 'did-finish-load': [], - 'did-fail-load': ['errorCode', 'errorDescription', 'validatedURL', 'isMainFrame', 'frameProcessId', 'frameRoutingId'], - 'did-frame-finish-load': ['isMainFrame', 'frameProcessId', 'frameRoutingId'], - 'did-start-loading': [], - 'did-stop-loading': [], - 'dom-ready': [], - 'console-message': ['level', 'message', 'line', 'sourceId'], - 'context-menu': ['params'], - 'devtools-opened': [], - 'devtools-closed': [], - 'devtools-focused': [], - 'new-window': ['url', 'frameName', 'disposition', 'options'], - 'will-navigate': ['url'], - 'did-start-navigation': ['url', 'isInPlace', 'isMainFrame', 'frameProcessId', 'frameRoutingId'], - 'did-navigate': ['url', 'httpResponseCode', 'httpStatusText'], - 'did-frame-navigate': ['url', 'httpResponseCode', 'httpStatusText', 'isMainFrame', 'frameProcessId', 'frameRoutingId'], - 'did-navigate-in-page': ['url', 'isMainFrame', 'frameProcessId', 'frameRoutingId'], - 'focus-change': ['focus', 'guestInstanceId'], - 'close': [], - 'crashed': [], - 'gpu-crashed': [], - 'plugin-crashed': ['name', 'version'], - 'destroyed': [], - 'page-title-updated': ['title', 'explicitSet'], - 'page-favicon-updated': ['favicons'], - 'enter-html-full-screen': [], - 'leave-html-full-screen': [], - 'media-started-playing': [], - 'media-paused': [], - 'found-in-page': ['result'], - 'did-change-theme-color': ['themeColor'], - 'update-target-url': ['url'] -} - -const DEPRECATED_EVENTS = { - 'page-title-updated': 'page-title-set' -} - -const dispatchEvent = function (webView, eventName, eventKey, ...args) { - if (DEPRECATED_EVENTS[eventName] != null) { - dispatchEvent(webView, DEPRECATED_EVENTS[eventName], eventKey, ...args) - } - const domEvent = new Event(eventName) - WEB_VIEW_EVENTS[eventKey].forEach((prop, index) => { - domEvent[prop] = args[index] - }) - webView.dispatchEvent(domEvent) - if (eventName === 'load-commit') { - webView.onLoadCommit(domEvent) - } else if (eventName === 'focus-change') { - webView.onFocusChange(domEvent) - } -} - -module.exports = { - registerEvents: function (webView, viewInstanceId) { - ipcRendererInternal.on(`ELECTRON_GUEST_VIEW_INTERNAL_DESTROY_GUEST-${viewInstanceId}`, function () { - webView.guestInstanceId = undefined - webView.reset() - const domEvent = new Event('destroyed') - webView.dispatchEvent(domEvent) - }) - - ipcRendererInternal.on(`ELECTRON_GUEST_VIEW_INTERNAL_DISPATCH_EVENT-${viewInstanceId}`, function (event, eventName, ...args) { - dispatchEvent(webView, eventName, eventName, ...args) - }) - - ipcRendererInternal.on(`ELECTRON_GUEST_VIEW_INTERNAL_IPC_MESSAGE-${viewInstanceId}`, function (event, channel, ...args) { - const domEvent = new Event('ipc-message') - domEvent.channel = channel - domEvent.args = args - webView.dispatchEvent(domEvent) - }) - }, - deregisterEvents: function (viewInstanceId) { - ipcRendererInternal.removeAllListeners(`ELECTRON_GUEST_VIEW_INTERNAL_DESTROY_GUEST-${viewInstanceId}`) - ipcRendererInternal.removeAllListeners(`ELECTRON_GUEST_VIEW_INTERNAL_DISPATCH_EVENT-${viewInstanceId}`) - ipcRendererInternal.removeAllListeners(`ELECTRON_GUEST_VIEW_INTERNAL_IPC_MESSAGE-${viewInstanceId}`) - }, - createGuest: function (params) { - return ipcRendererUtils.invoke('ELECTRON_GUEST_VIEW_MANAGER_CREATE_GUEST', params) - }, - createGuestSync: function (params) { - return ipcRendererUtils.invokeSync('ELECTRON_GUEST_VIEW_MANAGER_CREATE_GUEST', params) - }, - destroyGuest: function (guestInstanceId) { - ipcRendererUtils.invoke('ELECTRON_GUEST_VIEW_MANAGER_DESTROY_GUEST', guestInstanceId) - }, - attachGuest: function (elementInstanceId, guestInstanceId, params, contentWindow) { - const embedderFrameId = webFrame.getWebFrameId(contentWindow) - if (embedderFrameId < 0) { // this error should not happen. - throw new Error('Invalid embedder frame') - } - ipcRendererUtils.invoke('ELECTRON_GUEST_VIEW_MANAGER_ATTACH_GUEST', embedderFrameId, elementInstanceId, guestInstanceId, params) - } -} diff --git a/lib/renderer/web-view/guest-view-internal.ts b/lib/renderer/web-view/guest-view-internal.ts new file mode 100644 index 0000000000000..789e7fdb6f038 --- /dev/null +++ b/lib/renderer/web-view/guest-view-internal.ts @@ -0,0 +1,124 @@ +import { webFrame, IpcMessageEvent } from 'electron' +import { ipcRendererInternal } from '@electron/internal/renderer/ipc-renderer-internal' +import { invoke, invokeSync } from '@electron/internal/renderer/ipc-renderer-internal-utils' + +import { WebViewImpl } from '@electron/internal/renderer/web-view/web-view-impl' + +const WEB_VIEW_EVENTS: Record> = { + 'load-commit': ['url', 'isMainFrame'], + 'did-attach': [], + 'did-finish-load': [], + 'did-fail-load': ['errorCode', 'errorDescription', 'validatedURL', 'isMainFrame', 'frameProcessId', 'frameRoutingId'], + 'did-frame-finish-load': ['isMainFrame', 'frameProcessId', 'frameRoutingId'], + 'did-start-loading': [], + 'did-stop-loading': [], + 'dom-ready': [], + 'console-message': ['level', 'message', 'line', 'sourceId'], + 'context-menu': ['params'], + 'devtools-opened': [], + 'devtools-closed': [], + 'devtools-focused': [], + 'new-window': ['url', 'frameName', 'disposition', 'options'], + 'will-navigate': ['url'], + 'did-start-navigation': ['url', 'isInPlace', 'isMainFrame', 'frameProcessId', 'frameRoutingId'], + 'did-navigate': ['url', 'httpResponseCode', 'httpStatusText'], + 'did-frame-navigate': ['url', 'httpResponseCode', 'httpStatusText', 'isMainFrame', 'frameProcessId', 'frameRoutingId'], + 'did-navigate-in-page': ['url', 'isMainFrame', 'frameProcessId', 'frameRoutingId'], + 'focus-change': ['focus', 'guestInstanceId'], + 'close': [], + 'crashed': [], + 'gpu-crashed': [], + 'plugin-crashed': ['name', 'version'], + 'destroyed': [], + 'page-title-updated': ['title', 'explicitSet'], + 'page-favicon-updated': ['favicons'], + 'enter-html-full-screen': [], + 'leave-html-full-screen': [], + 'media-started-playing': [], + 'media-paused': [], + 'found-in-page': ['result'], + 'did-change-theme-color': ['themeColor'], + 'update-target-url': ['url'] +} + +const DEPRECATED_EVENTS: Record = { + 'page-title-updated': 'page-title-set' +} + +const dispatchEvent = function ( + webView: WebViewImpl, eventName: string, eventKey: string, ...args: Array +) { + if (DEPRECATED_EVENTS[eventName] != null) { + dispatchEvent(webView, DEPRECATED_EVENTS[eventName], eventKey, ...args) + } + + const domEvent = new Event(eventName) as ElectronInternal.WebViewEvent + WEB_VIEW_EVENTS[eventKey].forEach((prop, index) => { + (domEvent as any)[prop] = args[index] + }) + + webView.dispatchEvent(domEvent) + + if (eventName === 'load-commit') { + webView.onLoadCommit(domEvent) + } else if (eventName === 'focus-change') { + webView.onFocusChange() + } +} + +export function registerEvents (webView: WebViewImpl, viewInstanceId: number) { + ipcRendererInternal.on(`ELECTRON_GUEST_VIEW_INTERNAL_DESTROY_GUEST-${viewInstanceId}`, function () { + webView.guestInstanceId = undefined + webView.reset() + const domEvent = new Event('destroyed') + webView.dispatchEvent(domEvent) + }) + + ipcRendererInternal.on(`ELECTRON_GUEST_VIEW_INTERNAL_DISPATCH_EVENT-${viewInstanceId}`, function (event, eventName, ...args) { + dispatchEvent(webView, eventName, eventName, ...args) + }) + + ipcRendererInternal.on(`ELECTRON_GUEST_VIEW_INTERNAL_IPC_MESSAGE-${viewInstanceId}`, function (event, channel, ...args) { + const domEvent = new Event('ipc-message') as IpcMessageEvent + domEvent.channel = channel + domEvent.args = args + + webView.dispatchEvent(domEvent) + }) +} + +export function deregisterEvents (viewInstanceId: number) { + ipcRendererInternal.removeAllListeners(`ELECTRON_GUEST_VIEW_INTERNAL_DESTROY_GUEST-${viewInstanceId}`) + ipcRendererInternal.removeAllListeners(`ELECTRON_GUEST_VIEW_INTERNAL_DISPATCH_EVENT-${viewInstanceId}`) + ipcRendererInternal.removeAllListeners(`ELECTRON_GUEST_VIEW_INTERNAL_IPC_MESSAGE-${viewInstanceId}`) +} + +export function createGuest (params: Record): Promise { + return invoke('ELECTRON_GUEST_VIEW_MANAGER_CREATE_GUEST', params) +} + +export function createGuestSync (params: Record): number { + return invokeSync('ELECTRON_GUEST_VIEW_MANAGER_CREATE_GUEST', params) +} + +export function destroyGuest (guestInstanceId: number): void { + invoke('ELECTRON_GUEST_VIEW_MANAGER_DESTROY_GUEST', guestInstanceId) +} + +export function attachGuest ( + elementInstanceId: number, guestInstanceId: number, params: Record, contentWindow: Window +) { + const embedderFrameId = (webFrame as ElectronInternal.WebFrameInternal).getWebFrameId(contentWindow) + if (embedderFrameId < 0) { // this error should not happen. + throw new Error('Invalid embedder frame') + } + invoke('ELECTRON_GUEST_VIEW_MANAGER_ATTACH_GUEST', embedderFrameId, elementInstanceId, guestInstanceId, params) +} + +export const guestViewInternalModule = { + deregisterEvents, + createGuest, + createGuestSync, + destroyGuest, + attachGuest +} diff --git a/lib/renderer/web-view/web-view-attributes.js b/lib/renderer/web-view/web-view-attributes.ts similarity index 58% rename from lib/renderer/web-view/web-view-attributes.js rename to lib/renderer/web-view/web-view-attributes.ts index 818813bd8abc1..07030b7015f72 100644 --- a/lib/renderer/web-view/web-view-attributes.js +++ b/lib/renderer/web-view/web-view-attributes.ts @@ -1,14 +1,12 @@ -'use strict' - -const ipcRendererUtils = require('@electron/internal/renderer/ipc-renderer-internal-utils') -const { WebViewImpl } = require('@electron/internal/renderer/web-view/web-view-impl') -const webViewConstants = require('@electron/internal/renderer/web-view/web-view-constants') +import * as ipcRendererUtils from '@electron/internal/renderer/ipc-renderer-internal-utils' +import { WebViewImpl } from '@electron/internal/renderer/web-view/web-view-impl' +import { WEB_VIEW_CONSTANTS } from '@electron/internal/renderer/web-view/web-view-constants' // Helper function to resolve url set in attribute. const a = document.createElement('a') -const resolveURL = function (url) { - if (url === '') return '' +const resolveURL = function (url?: string | null) { + if (!url) return '' a.href = url return a.href } @@ -16,33 +14,35 @@ const resolveURL = function (url) { // Attribute objects. // Default implementation of a WebView attribute. class WebViewAttribute { - constructor (name, webViewImpl) { + public value: any; + public ignoreMutation = false; + + constructor (public name: string, public webViewImpl: WebViewImpl) { this.name = name - this.value = webViewImpl.webviewNode[name] || '' + this.value = (webViewImpl.webviewNode as Record)[name] || '' this.webViewImpl = webViewImpl - this.ignoreMutation = false this.defineProperty() } // Retrieves and returns the attribute's value. - getValue () { + public getValue () { return this.webViewImpl.webviewNode.getAttribute(this.name) || this.value } // Sets the attribute's value. - setValue (value) { + public setValue (value: any) { this.webViewImpl.webviewNode.setAttribute(this.name, value || '') } // Changes the attribute's value without triggering its mutation handler. - setValueIgnoreMutation (value) { + public setValueIgnoreMutation (value: any) { this.ignoreMutation = true this.setValue(value) this.ignoreMutation = false } // Defines this attribute as a property on the webview node. - defineProperty () { + public defineProperty () { return Object.defineProperty(this.webViewImpl.webviewNode, this.name, { get: () => { return this.getValue() @@ -55,7 +55,7 @@ class WebViewAttribute { } // Called when the attribute's value changes. - handleMutation () {} + public handleMutation (..._args: Array): any {} } // An attribute that is treated as a Boolean. @@ -64,7 +64,7 @@ class BooleanAttribute extends WebViewAttribute { return this.webViewImpl.webviewNode.hasAttribute(this.name) } - setValue (value) { + setValue (value: boolean) { if (value) { this.webViewImpl.webviewNode.setAttribute(this.name, '') } else { @@ -75,35 +75,38 @@ class BooleanAttribute extends WebViewAttribute { // Attribute representing the state of the storage partition. class PartitionAttribute extends WebViewAttribute { - constructor (webViewImpl) { - super(webViewConstants.ATTRIBUTE_PARTITION, webViewImpl) - this.validPartitionId = true + public validPartitionId = true + + constructor (public webViewImpl: WebViewImpl) { + super(WEB_VIEW_CONSTANTS.ATTRIBUTE_PARTITION, webViewImpl) } - handleMutation (oldValue, newValue) { + public handleMutation (oldValue: any, newValue: any) { newValue = newValue || '' // The partition cannot change if the webview has already navigated. if (!this.webViewImpl.beforeFirstNavigation) { - console.error(webViewConstants.ERROR_MSG_ALREADY_NAVIGATED) + console.error(WEB_VIEW_CONSTANTS.ERROR_MSG_ALREADY_NAVIGATED) this.setValueIgnoreMutation(oldValue) return } if (newValue === 'persist:') { this.validPartitionId = false - console.error(webViewConstants.ERROR_MSG_INVALID_PARTITION_ATTRIBUTE) + console.error(WEB_VIEW_CONSTANTS.ERROR_MSG_INVALID_PARTITION_ATTRIBUTE) } } } // Attribute that handles the location and navigation of the webview. class SrcAttribute extends WebViewAttribute { - constructor (webViewImpl) { - super(webViewConstants.ATTRIBUTE_SRC, webViewImpl) + public observer!: MutationObserver; + + constructor (public webViewImpl: WebViewImpl) { + super(WEB_VIEW_CONSTANTS.ATTRIBUTE_SRC, webViewImpl) this.setupMutationObserver() } - getValue () { + public getValue () { if (this.webViewImpl.webviewNode.hasAttribute(this.name)) { return resolveURL(this.webViewImpl.webviewNode.getAttribute(this.name)) } else { @@ -111,7 +114,7 @@ class SrcAttribute extends WebViewAttribute { } } - setValueIgnoreMutation (value) { + public setValueIgnoreMutation (value: any) { super.setValueIgnoreMutation(value) // takeRecords() is needed to clear queued up src mutations. Without it, it @@ -121,7 +124,7 @@ class SrcAttribute extends WebViewAttribute { this.observer.takeRecords() } - handleMutation (oldValue, newValue) { + public handleMutation (oldValue: any, newValue: any) { // Once we have navigated, we don't allow clearing the src attribute. // Once enters a navigated state, it cannot return to a // placeholder state. @@ -139,7 +142,7 @@ class SrcAttribute extends WebViewAttribute { // attribute without any changes to its value. This is useful in the case // where the webview guest has crashed and navigating to the same address // spawns off a new process. - setupMutationObserver () { + public setupMutationObserver () { this.observer = new MutationObserver((mutations) => { for (const mutation of mutations) { const { oldValue } = mutation @@ -150,16 +153,18 @@ class SrcAttribute extends WebViewAttribute { this.handleMutation(oldValue, newValue) } }) + const params = { attributes: true, attributeOldValue: true, attributeFilter: [this.name] } + this.observer.observe(this.webViewImpl.webviewNode, params) } - parse () { - if (!this.webViewImpl.elementAttached || !this.webViewImpl.attributes[webViewConstants.ATTRIBUTE_PARTITION].validPartitionId || !this.getValue()) { + public parse () { + if (!this.webViewImpl.elementAttached || !this.webViewImpl.attributes[WEB_VIEW_CONSTANTS.ATTRIBUTE_PARTITION].validPartitionId || !this.getValue()) { return } if (this.webViewImpl.guestInstanceId == null) { @@ -171,12 +176,14 @@ class SrcAttribute extends WebViewAttribute { } // Navigate to |this.src|. - const opts = {} - const httpreferrer = this.webViewImpl.attributes[webViewConstants.ATTRIBUTE_HTTPREFERRER].getValue() + const opts: Record = {} + + const httpreferrer = this.webViewImpl.attributes[WEB_VIEW_CONSTANTS.ATTRIBUTE_HTTPREFERRER].getValue() if (httpreferrer) { opts.httpReferrer = httpreferrer } - const useragent = this.webViewImpl.attributes[webViewConstants.ATTRIBUTE_USERAGENT].getValue() + + const useragent = this.webViewImpl.attributes[WEB_VIEW_CONSTANTS.ATTRIBUTE_USERAGENT].getValue() if (useragent) { opts.userAgent = useragent } @@ -191,69 +198,72 @@ class SrcAttribute extends WebViewAttribute { // Attribute specifies HTTP referrer. class HttpReferrerAttribute extends WebViewAttribute { - constructor (webViewImpl) { - super(webViewConstants.ATTRIBUTE_HTTPREFERRER, webViewImpl) + constructor (webViewImpl: WebViewImpl) { + super(WEB_VIEW_CONSTANTS.ATTRIBUTE_HTTPREFERRER, webViewImpl) } } // Attribute specifies user agent class UserAgentAttribute extends WebViewAttribute { - constructor (webViewImpl) { - super(webViewConstants.ATTRIBUTE_USERAGENT, webViewImpl) + constructor (webViewImpl: WebViewImpl) { + super(WEB_VIEW_CONSTANTS.ATTRIBUTE_USERAGENT, webViewImpl) } } // Attribute that set preload script. class PreloadAttribute extends WebViewAttribute { - constructor (webViewImpl) { - super(webViewConstants.ATTRIBUTE_PRELOAD, webViewImpl) + constructor (webViewImpl: WebViewImpl) { + super(WEB_VIEW_CONSTANTS.ATTRIBUTE_PRELOAD, webViewImpl) } - getValue () { + public getValue () { if (!this.webViewImpl.webviewNode.hasAttribute(this.name)) { return this.value } + let preload = resolveURL(this.webViewImpl.webviewNode.getAttribute(this.name)) const protocol = preload.substr(0, 5) + if (protocol !== 'file:') { - console.error(webViewConstants.ERROR_MSG_INVALID_PRELOAD_ATTRIBUTE) + console.error(WEB_VIEW_CONSTANTS.ERROR_MSG_INVALID_PRELOAD_ATTRIBUTE) preload = '' } + return preload } } // Attribute that specifies the blink features to be enabled. class BlinkFeaturesAttribute extends WebViewAttribute { - constructor (webViewImpl) { - super(webViewConstants.ATTRIBUTE_BLINKFEATURES, webViewImpl) + constructor (webViewImpl: WebViewImpl) { + super(WEB_VIEW_CONSTANTS.ATTRIBUTE_BLINKFEATURES, webViewImpl) } } // Attribute that specifies the blink features to be disabled. class DisableBlinkFeaturesAttribute extends WebViewAttribute { - constructor (webViewImpl) { - super(webViewConstants.ATTRIBUTE_DISABLEBLINKFEATURES, webViewImpl) + constructor (webViewImpl: WebViewImpl) { + super(WEB_VIEW_CONSTANTS.ATTRIBUTE_DISABLEBLINKFEATURES, webViewImpl) } } // Attribute that specifies the web preferences to be enabled. class WebPreferencesAttribute extends WebViewAttribute { - constructor (webViewImpl) { - super(webViewConstants.ATTRIBUTE_WEBPREFERENCES, webViewImpl) + constructor (webViewImpl: WebViewImpl) { + super(WEB_VIEW_CONSTANTS.ATTRIBUTE_WEBPREFERENCES, webViewImpl) } } class EnableRemoteModuleAttribute extends WebViewAttribute { - constructor (webViewImpl) { - super(webViewConstants.ATTRIBUTE_ENABLEREMOTEMODULE, webViewImpl) + constructor (webViewImpl: WebViewImpl) { + super(WEB_VIEW_CONSTANTS.ATTRIBUTE_ENABLEREMOTEMODULE, webViewImpl) } - getValue () { + public getValue () { return this.webViewImpl.webviewNode.getAttribute(this.name) !== 'false' } - setValue (value) { + public setValue (value: any) { this.webViewImpl.webviewNode.setAttribute(this.name, value ? 'true' : 'false') } } @@ -261,17 +271,17 @@ class EnableRemoteModuleAttribute extends WebViewAttribute { // Sets up all of the webview attributes. WebViewImpl.prototype.setupWebViewAttributes = function () { this.attributes = {} - this.attributes[webViewConstants.ATTRIBUTE_PARTITION] = new PartitionAttribute(this) - this.attributes[webViewConstants.ATTRIBUTE_SRC] = new SrcAttribute(this) - this.attributes[webViewConstants.ATTRIBUTE_HTTPREFERRER] = new HttpReferrerAttribute(this) - this.attributes[webViewConstants.ATTRIBUTE_USERAGENT] = new UserAgentAttribute(this) - this.attributes[webViewConstants.ATTRIBUTE_NODEINTEGRATION] = new BooleanAttribute(webViewConstants.ATTRIBUTE_NODEINTEGRATION, this) - this.attributes[webViewConstants.ATTRIBUTE_PLUGINS] = new BooleanAttribute(webViewConstants.ATTRIBUTE_PLUGINS, this) - this.attributes[webViewConstants.ATTRIBUTE_DISABLEWEBSECURITY] = new BooleanAttribute(webViewConstants.ATTRIBUTE_DISABLEWEBSECURITY, this) - this.attributes[webViewConstants.ATTRIBUTE_ALLOWPOPUPS] = new BooleanAttribute(webViewConstants.ATTRIBUTE_ALLOWPOPUPS, this) - this.attributes[webViewConstants.ATTRIBUTE_ENABLEREMOTEMODULE] = new EnableRemoteModuleAttribute(this) - this.attributes[webViewConstants.ATTRIBUTE_PRELOAD] = new PreloadAttribute(this) - this.attributes[webViewConstants.ATTRIBUTE_BLINKFEATURES] = new BlinkFeaturesAttribute(this) - this.attributes[webViewConstants.ATTRIBUTE_DISABLEBLINKFEATURES] = new DisableBlinkFeaturesAttribute(this) - this.attributes[webViewConstants.ATTRIBUTE_WEBPREFERENCES] = new WebPreferencesAttribute(this) + this.attributes[WEB_VIEW_CONSTANTS.ATTRIBUTE_PARTITION] = new PartitionAttribute(this) + this.attributes[WEB_VIEW_CONSTANTS.ATTRIBUTE_SRC] = new SrcAttribute(this) + this.attributes[WEB_VIEW_CONSTANTS.ATTRIBUTE_HTTPREFERRER] = new HttpReferrerAttribute(this) + this.attributes[WEB_VIEW_CONSTANTS.ATTRIBUTE_USERAGENT] = new UserAgentAttribute(this) + this.attributes[WEB_VIEW_CONSTANTS.ATTRIBUTE_NODEINTEGRATION] = new BooleanAttribute(WEB_VIEW_CONSTANTS.ATTRIBUTE_NODEINTEGRATION, this) + this.attributes[WEB_VIEW_CONSTANTS.ATTRIBUTE_PLUGINS] = new BooleanAttribute(WEB_VIEW_CONSTANTS.ATTRIBUTE_PLUGINS, this) + this.attributes[WEB_VIEW_CONSTANTS.ATTRIBUTE_DISABLEWEBSECURITY] = new BooleanAttribute(WEB_VIEW_CONSTANTS.ATTRIBUTE_DISABLEWEBSECURITY, this) + this.attributes[WEB_VIEW_CONSTANTS.ATTRIBUTE_ALLOWPOPUPS] = new BooleanAttribute(WEB_VIEW_CONSTANTS.ATTRIBUTE_ALLOWPOPUPS, this) + this.attributes[WEB_VIEW_CONSTANTS.ATTRIBUTE_ENABLEREMOTEMODULE] = new EnableRemoteModuleAttribute(this) + this.attributes[WEB_VIEW_CONSTANTS.ATTRIBUTE_PRELOAD] = new PreloadAttribute(this) + this.attributes[WEB_VIEW_CONSTANTS.ATTRIBUTE_BLINKFEATURES] = new BlinkFeaturesAttribute(this) + this.attributes[WEB_VIEW_CONSTANTS.ATTRIBUTE_DISABLEBLINKFEATURES] = new DisableBlinkFeaturesAttribute(this) + this.attributes[WEB_VIEW_CONSTANTS.ATTRIBUTE_WEBPREFERENCES] = new WebPreferencesAttribute(this) } diff --git a/lib/renderer/web-view/web-view-constants.js b/lib/renderer/web-view/web-view-constants.js deleted file mode 100644 index 459aafaca28b1..0000000000000 --- a/lib/renderer/web-view/web-view-constants.js +++ /dev/null @@ -1,28 +0,0 @@ -'use strict' - -module.exports = { - // Attributes. - ATTRIBUTE_NAME: 'name', - ATTRIBUTE_PARTITION: 'partition', - ATTRIBUTE_SRC: 'src', - ATTRIBUTE_HTTPREFERRER: 'httpreferrer', - ATTRIBUTE_NODEINTEGRATION: 'nodeintegration', - ATTRIBUTE_ENABLEREMOTEMODULE: 'enableremotemodule', - ATTRIBUTE_PLUGINS: 'plugins', - ATTRIBUTE_DISABLEWEBSECURITY: 'disablewebsecurity', - ATTRIBUTE_ALLOWPOPUPS: 'allowpopups', - ATTRIBUTE_PRELOAD: 'preload', - ATTRIBUTE_USERAGENT: 'useragent', - ATTRIBUTE_BLINKFEATURES: 'blinkfeatures', - ATTRIBUTE_DISABLEBLINKFEATURES: 'disableblinkfeatures', - ATTRIBUTE_WEBPREFERENCES: 'webpreferences', - - // Internal attribute. - ATTRIBUTE_INTERNALINSTANCEID: 'internalinstanceid', - - // Error messages. - ERROR_MSG_ALREADY_NAVIGATED: 'The object has already navigated, so its partition cannot be changed.', - ERROR_MSG_CANNOT_INJECT_SCRIPT: ': ' + 'Script cannot be injected into content until the page has loaded.', - ERROR_MSG_INVALID_PARTITION_ATTRIBUTE: 'Invalid partition attribute.', - ERROR_MSG_INVALID_PRELOAD_ATTRIBUTE: 'Only "file:" protocol is supported in "preload" attribute.' -} diff --git a/lib/renderer/web-view/web-view-constants.ts b/lib/renderer/web-view/web-view-constants.ts new file mode 100644 index 0000000000000..6a31dbc0d003a --- /dev/null +++ b/lib/renderer/web-view/web-view-constants.ts @@ -0,0 +1,26 @@ +export const enum WEB_VIEW_CONSTANTS { + // Attributes. + ATTRIBUTE_NAME = 'name', + ATTRIBUTE_PARTITION = 'partition', + ATTRIBUTE_SRC = 'src', + ATTRIBUTE_HTTPREFERRER = 'httpreferrer', + ATTRIBUTE_NODEINTEGRATION = 'nodeintegration', + ATTRIBUTE_ENABLEREMOTEMODULE = 'enableremotemodule', + ATTRIBUTE_PLUGINS = 'plugins', + ATTRIBUTE_DISABLEWEBSECURITY = 'disablewebsecurity', + ATTRIBUTE_ALLOWPOPUPS = 'allowpopups', + ATTRIBUTE_PRELOAD = 'preload', + ATTRIBUTE_USERAGENT = 'useragent', + ATTRIBUTE_BLINKFEATURES = 'blinkfeatures', + ATTRIBUTE_DISABLEBLINKFEATURES = 'disableblinkfeatures', + ATTRIBUTE_WEBPREFERENCES = 'webpreferences', + + // Internal attribute. + ATTRIBUTE_INTERNALINSTANCEID = 'internalinstanceid', + + // Error messages. + ERROR_MSG_ALREADY_NAVIGATED = 'The object has already navigated, so its partition cannot be changed.', + ERROR_MSG_CANNOT_INJECT_SCRIPT = ' = ' + 'Script cannot be injected into content until the page has loaded.', + ERROR_MSG_INVALID_PARTITION_ATTRIBUTE = 'Invalid partition attribute.', + ERROR_MSG_INVALID_PRELOAD_ATTRIBUTE = 'Only "file:" protocol is supported in "preload" attribute.' +} diff --git a/lib/renderer/web-view/web-view-element.js b/lib/renderer/web-view/web-view-element.ts similarity index 52% rename from lib/renderer/web-view/web-view-element.js rename to lib/renderer/web-view/web-view-element.ts index beb0a34d0f880..26ac4e0badf16 100644 --- a/lib/renderer/web-view/web-view-element.js +++ b/lib/renderer/web-view/web-view-element.ts @@ -1,5 +1,3 @@ -'use strict' - // When using context isolation, the WebViewElement and the custom element // methods have to be defined in the main world to be able to be registered. // @@ -10,27 +8,30 @@ // which runs in browserify environment instead of Node environment, all native // modules must be passed from outside, all included files must be plain JS. -const webViewConstants = require('@electron/internal/renderer/web-view/web-view-constants') +import { WEB_VIEW_CONSTANTS } from '@electron/internal/renderer/web-view/web-view-constants' +import { WebViewImpl, webViewImplModule } from '@electron/internal/renderer/web-view/web-view-impl' // Return a WebViewElement class that is defined in this context. -const defineWebViewElement = (v8Util, webViewImpl) => { +const defineWebViewElement = (v8Util: NodeJS.V8UtilBinding, webViewImpl: typeof webViewImplModule) => { const { guestViewInternal, WebViewImpl } = webViewImpl return class WebViewElement extends HTMLElement { + public internalInstanceId?: number; + static get observedAttributes () { return [ - webViewConstants.ATTRIBUTE_PARTITION, - webViewConstants.ATTRIBUTE_SRC, - webViewConstants.ATTRIBUTE_HTTPREFERRER, - webViewConstants.ATTRIBUTE_USERAGENT, - webViewConstants.ATTRIBUTE_NODEINTEGRATION, - webViewConstants.ATTRIBUTE_PLUGINS, - webViewConstants.ATTRIBUTE_DISABLEWEBSECURITY, - webViewConstants.ATTRIBUTE_ALLOWPOPUPS, - webViewConstants.ATTRIBUTE_ENABLEREMOTEMODULE, - webViewConstants.ATTRIBUTE_PRELOAD, - webViewConstants.ATTRIBUTE_BLINKFEATURES, - webViewConstants.ATTRIBUTE_DISABLEBLINKFEATURES, - webViewConstants.ATTRIBUTE_WEBPREFERENCES + WEB_VIEW_CONSTANTS.ATTRIBUTE_PARTITION, + WEB_VIEW_CONSTANTS.ATTRIBUTE_SRC, + WEB_VIEW_CONSTANTS.ATTRIBUTE_HTTPREFERRER, + WEB_VIEW_CONSTANTS.ATTRIBUTE_USERAGENT, + WEB_VIEW_CONSTANTS.ATTRIBUTE_NODEINTEGRATION, + WEB_VIEW_CONSTANTS.ATTRIBUTE_PLUGINS, + WEB_VIEW_CONSTANTS.ATTRIBUTE_DISABLEWEBSECURITY, + WEB_VIEW_CONSTANTS.ATTRIBUTE_ALLOWPOPUPS, + WEB_VIEW_CONSTANTS.ATTRIBUTE_ENABLEREMOTEMODULE, + WEB_VIEW_CONSTANTS.ATTRIBUTE_PRELOAD, + WEB_VIEW_CONSTANTS.ATTRIBUTE_BLINKFEATURES, + WEB_VIEW_CONSTANTS.ATTRIBUTE_DISABLEBLINKFEATURES, + WEB_VIEW_CONSTANTS.ATTRIBUTE_WEBPREFERENCES ] } @@ -40,26 +41,26 @@ const defineWebViewElement = (v8Util, webViewImpl) => { } connectedCallback () { - const internal = v8Util.getHiddenValue(this, 'internal') + const internal = v8Util.getHiddenValue(this, 'internal') if (!internal) { return } if (!internal.elementAttached) { guestViewInternal.registerEvents(internal, internal.viewInstanceId) internal.elementAttached = true - internal.attributes[webViewConstants.ATTRIBUTE_SRC].parse() + internal.attributes[WEB_VIEW_CONSTANTS.ATTRIBUTE_SRC].parse() } } - attributeChangedCallback (name, oldValue, newValue) { - const internal = v8Util.getHiddenValue(this, 'internal') + attributeChangedCallback (name: string, oldValue: any, newValue: any) { + const internal = v8Util.getHiddenValue(this, 'internal') if (internal) { internal.handleWebviewAttributeMutation(name, oldValue, newValue) } } disconnectedCallback () { - const internal = v8Util.getHiddenValue(this, 'internal') + const internal = v8Util.getHiddenValue(this, 'internal') if (!internal) { return } @@ -72,15 +73,18 @@ const defineWebViewElement = (v8Util, webViewImpl) => { } // Register custom element. -const registerWebViewElement = (v8Util, webViewImpl) => { - const WebViewElement = defineWebViewElement(v8Util, webViewImpl) +const registerWebViewElement = (v8Util: NodeJS.V8UtilBinding, webViewImpl: typeof webViewImplModule) => { + // I wish eslint wasn't so stupid, but it is + // eslint-disable-next-line + const WebViewElement = defineWebViewElement(v8Util, webViewImpl) as unknown as typeof ElectronInternal.WebViewElement webViewImpl.setupMethods(WebViewElement) // The customElements.define has to be called in a special scope. - webViewImpl.webFrame.allowGuestViewElementDefinition(window, () => { - window.customElements.define('webview', WebViewElement) - window.WebView = WebViewElement + const webFrame = webViewImpl.webFrame as ElectronInternal.WebFrameInternal + webFrame.allowGuestViewElementDefinition(window, () => { + window.customElements.define('webview', WebViewElement); + (window as any).WebView = WebViewElement // Delete the callbacks so developers cannot call them and produce unexpected // behavior. @@ -90,21 +94,24 @@ const registerWebViewElement = (v8Util, webViewImpl) => { // Now that |observedAttributes| has been retrieved, we can hide it from // user code as well. - delete WebViewElement.observedAttributes + // TypeScript is concerned that we're deleting a read-only attribute + delete (WebViewElement as any).observedAttributes }) } // Prepare to register the element. -const setupWebView = (v8Util, webViewImpl) => { +export const setupWebView = (v8Util: NodeJS.V8UtilBinding, webViewImpl: typeof webViewImplModule) => { const useCapture = true - window.addEventListener('readystatechange', function listener (event) { + const listener = (event: Event) => { if (document.readyState === 'loading') { return } + webViewImpl.setupAttributes() registerWebViewElement(v8Util, webViewImpl) + window.removeEventListener(event.type, listener, useCapture) - }, useCapture) -} + } -module.exports = { setupWebView } + window.addEventListener('readystatechange', listener, useCapture) +} diff --git a/lib/renderer/web-view/web-view-impl.js b/lib/renderer/web-view/web-view-impl.ts similarity index 66% rename from lib/renderer/web-view/web-view-impl.js rename to lib/renderer/web-view/web-view-impl.ts index ccbdb4cf7d348..d607c0a8b6387 100644 --- a/lib/renderer/web-view/web-view-impl.js +++ b/lib/renderer/web-view/web-view-impl.ts @@ -1,16 +1,15 @@ -'use strict' +import { deprecate, webFrame } from 'electron' -const { webFrame, deprecate } = require('electron') - -const v8Util = process.atomBinding('v8_util') -const ipcRendererUtils = require('@electron/internal/renderer/ipc-renderer-internal-utils') -const guestViewInternal = require('@electron/internal/renderer/web-view/guest-view-internal') -const webViewConstants = require('@electron/internal/renderer/web-view/web-view-constants') -const { +import * as ipcRendererUtils from '@electron/internal/renderer/ipc-renderer-internal-utils' +import * as guestViewInternal from '@electron/internal/renderer/web-view/guest-view-internal' +import { WEB_VIEW_CONSTANTS } from '@electron/internal/renderer/web-view/web-view-constants' +import { syncMethods, asyncCallbackMethods, asyncPromiseMethods -} = require('@electron/internal/common/web-view-methods') +} from '@electron/internal/common/web-view-methods' + +const v8Util = process.atomBinding('v8_util') // ID generator. let nextId = 0 @@ -20,16 +19,25 @@ const getNextId = function () { } // Represents the internal state of the WebView node. -class WebViewImpl { - constructor (webviewNode) { - this.webviewNode = webviewNode - this.elementAttached = false - this.beforeFirstNavigation = true - this.hasFocus = false - - // on* Event handlers. - this.on = {} - +export class WebViewImpl { + public beforeFirstNavigation = true + public elementAttached = false + public guestInstanceId?: number + public hasFocus = false + public internalInstanceId?: number; + public resizeObserver?: ResizeObserver; + public userAgentOverride?: string; + public viewInstanceId: number + + // on* Event handlers. + public on: Record = {} + public internalElement: HTMLIFrameElement + + // Replaced in web-view-attributes + public attributes: Record = {} + public setupWebViewAttributes (): void {} + + constructor (public webviewNode: HTMLElement) { // Create internal iframe element. this.internalElement = this.createInternalElement() const shadowRoot = this.webviewNode.attachShadow({ mode: 'open' }) @@ -70,22 +78,17 @@ class WebViewImpl { } this.beforeFirstNavigation = true - this.attributes[webViewConstants.ATTRIBUTE_PARTITION].validPartitionId = true + this.attributes[WEB_VIEW_CONSTANTS.ATTRIBUTE_PARTITION].validPartitionId = true // Since attachment swaps a local frame for a remote frame, we need our // internal iframe element to be local again before we can reattach. const newFrame = this.createInternalElement() const oldFrame = this.internalElement this.internalElement = newFrame - oldFrame.parentNode.replaceChild(newFrame, oldFrame) - } - // Sets the .request property. - setRequestPropertyOnWebViewNode (request) { - Object.defineProperty(this.webviewNode, 'request', { - value: request, - enumerable: true - }) + if (oldFrame && oldFrame.parentNode) { + oldFrame.parentNode.replaceChild(newFrame, oldFrame) + } } // This observer monitors mutations to attributes of the and @@ -93,7 +96,7 @@ class WebViewImpl { // a BrowserPlugin property will update the corresponding BrowserPlugin // attribute, if necessary. See BrowserPlugin::UpdateDOMAttribute for more // details. - handleWebviewAttributeMutation (attributeName, oldValue, newValue) { + handleWebviewAttributeMutation (attributeName: string, oldValue: any, newValue: any) { if (!this.attributes[attributeName] || this.attributes[attributeName].ignoreMutation) { return } @@ -103,7 +106,7 @@ class WebViewImpl { } onElementResize () { - const resizeEvent = new Event('resize') + const resizeEvent = new Event('resize') as ElectronInternal.WebFrameResizeEvent resizeEvent.newWidth = this.webviewNode.clientWidth resizeEvent.newHeight = this.webviewNode.clientHeight this.dispatchEvent(resizeEvent) @@ -120,13 +123,13 @@ class WebViewImpl { this.attachGuestInstance(guestViewInternal.createGuestSync(this.buildParams())) } - dispatchEvent (webViewEvent) { + dispatchEvent (webViewEvent: Electron.Event) { this.webviewNode.dispatchEvent(webViewEvent) } // Adds an 'on' property on the webview, which can be used to set/unset // an event handler. - setupEventProperty (eventName) { + setupEventProperty (eventName: string) { const propertyName = `on${eventName.toLowerCase()}` return Object.defineProperty(this.webviewNode, propertyName, { get: () => { @@ -146,14 +149,14 @@ class WebViewImpl { } // Updates state upon loadcommit. - onLoadCommit (webViewEvent) { - const oldValue = this.webviewNode.getAttribute(webViewConstants.ATTRIBUTE_SRC) + onLoadCommit (webViewEvent: ElectronInternal.WebViewEvent) { + const oldValue = this.webviewNode.getAttribute(WEB_VIEW_CONSTANTS.ATTRIBUTE_SRC) const newValue = webViewEvent.url if (webViewEvent.isMainFrame && (oldValue !== newValue)) { // Touching the src attribute triggers a navigation. To avoid // triggering a page reload on every guest-initiated navigation, // we do not handle this mutation. - this.attributes[webViewConstants.ATTRIBUTE_SRC].setValueIgnoreMutation(newValue) + this.attributes[WEB_VIEW_CONSTANTS.ATTRIBUTE_SRC].setValueIgnoreMutation(newValue) } } @@ -166,52 +169,66 @@ class WebViewImpl { } } - onAttach (storagePartitionId) { - return this.attributes[webViewConstants.ATTRIBUTE_PARTITION].setValue(storagePartitionId) + onAttach (storagePartitionId: number) { + return this.attributes[WEB_VIEW_CONSTANTS.ATTRIBUTE_PARTITION].setValue(storagePartitionId) } buildParams () { - const params = { + const params: Record = { instanceId: this.viewInstanceId, userAgentOverride: this.userAgentOverride } + for (const attributeName in this.attributes) { if (this.attributes.hasOwnProperty(attributeName)) { params[attributeName] = this.attributes[attributeName].getValue() } } + return params } - attachGuestInstance (guestInstanceId) { + attachGuestInstance (guestInstanceId: number) { if (!this.elementAttached) { // The element could be detached before we got response from browser. return } this.internalInstanceId = getNextId() this.guestInstanceId = guestInstanceId - guestViewInternal.attachGuest(this.internalInstanceId, this.guestInstanceId, this.buildParams(), this.internalElement.contentWindow) + + guestViewInternal.attachGuest( + this.internalInstanceId, + this.guestInstanceId, + this.buildParams(), + this.internalElement.contentWindow! + ) + // ResizeObserver is a browser global not recognized by "standard". /* globals ResizeObserver */ // TODO(zcbenz): Should we deprecate the "resize" event? Wait, it is not // even documented. - this.resizeObserver = new ResizeObserver(this.onElementResize.bind(this)).observe(this.internalElement) + this.resizeObserver = new ResizeObserver(this.onElementResize.bind(this)) + this.resizeObserver.observe(this.internalElement) } } -const setupAttributes = () => { +export const setupAttributes = () => { require('@electron/internal/renderer/web-view/web-view-attributes') } -const setupMethods = (WebViewElement) => { +// I wish eslint wasn't so stupid, but it is +// eslint-disable-next-line +export const setupMethods = (WebViewElement: typeof ElectronInternal.WebViewElement) => { // WebContents associated with this webview. WebViewElement.prototype.getWebContents = function () { const { getRemote } = require('@electron/internal/renderer/remote') const getGuestWebContents = getRemote('getGuestWebContents') - const internal = v8Util.getHiddenValue(this, 'internal') + const internal = v8Util.getHiddenValue(this, 'internal') + if (!internal.guestInstanceId) { internal.createGuestSync() } + return getGuestWebContents(internal.guestInstanceId) } @@ -220,8 +237,8 @@ const setupMethods = (WebViewElement) => { this.contentWindow.focus() } - const getGuestInstanceId = function (self) { - const internal = v8Util.getHiddenValue(self, 'internal') + const getGuestInstanceId = function (self: any) { + const internal = v8Util.getHiddenValue(self, 'internal') if (!internal.guestInstanceId) { throw new Error('The WebView must be attached to the DOM and the dom-ready event emitted before this method can be called.') } @@ -229,34 +246,41 @@ const setupMethods = (WebViewElement) => { } // Forward proto.foo* method calls to WebViewImpl.foo*. - const createBlockHandler = function (method) { - return function (...args) { + const createBlockHandler = function (method: string) { + return function (this: any, ...args: Array) { return ipcRendererUtils.invokeSync('ELECTRON_GUEST_VIEW_MANAGER_CALL', getGuestInstanceId(this), method, args) } } + for (const method of syncMethods) { - WebViewElement.prototype[method] = createBlockHandler(method) + (WebViewElement.prototype as Record)[method] = createBlockHandler(method) } - const createNonBlockHandler = function (method) { - return function (...args) { + const createNonBlockHandler = function (method: string) { + return function (this: any, ...args: Array) { ipcRendererUtils.invoke('ELECTRON_GUEST_VIEW_MANAGER_CALL', getGuestInstanceId(this), method, args) } } for (const method of asyncCallbackMethods) { - WebViewElement.prototype[method] = createNonBlockHandler(method) + (WebViewElement.prototype as Record)[method] = createNonBlockHandler(method) } - const createPromiseHandler = function (method) { - return function (...args) { + const createPromiseHandler = function (method: string) { + return function (this: any, ...args: Array) { return ipcRendererUtils.invoke('ELECTRON_GUEST_VIEW_MANAGER_CALL', getGuestInstanceId(this), method, args) } } for (const method of asyncPromiseMethods) { - WebViewElement.prototype[method] = deprecate.promisify(createPromiseHandler(method)) + (WebViewElement.prototype as Record)[method] = deprecate.promisify(createPromiseHandler(method)) } } -module.exports = { setupAttributes, setupMethods, guestViewInternal, webFrame, WebViewImpl } +export const webViewImplModule = { + setupAttributes, + setupMethods, + guestViewInternal, + webFrame, + WebViewImpl +} diff --git a/lib/renderer/web-view/web-view-init.js b/lib/renderer/web-view/web-view-init.ts similarity index 67% rename from lib/renderer/web-view/web-view-init.js rename to lib/renderer/web-view/web-view-init.ts index a57aacb6ae7b4..5e2c2fe152623 100644 --- a/lib/renderer/web-view/web-view-init.js +++ b/lib/renderer/web-view/web-view-init.ts @@ -1,9 +1,8 @@ -'use strict' +import { ipcRendererInternal } from '@electron/internal/renderer/ipc-renderer-internal' -const { ipcRendererInternal } = require('@electron/internal/renderer/ipc-renderer-internal') const v8Util = process.atomBinding('v8_util') -function handleFocusBlur (guestInstanceId) { +function handleFocusBlur (guestInstanceId: number) { // Note that while Chromium content APIs have observer for focus/blur, they // unfortunately do not work for webview. @@ -16,15 +15,17 @@ function handleFocusBlur (guestInstanceId) { }) } -module.exports = function (contextIsolation, webviewTag, guestInstanceId) { +export function webViewInit ( + contextIsolation: boolean, webviewTag: ElectronInternal.WebViewElement, guestInstanceId: number +) { // Don't allow recursive ``. if (webviewTag && guestInstanceId == null) { - const webViewImpl = require('@electron/internal/renderer/web-view/web-view-impl') + const { webViewImplModule } = require('@electron/internal/renderer/web-view/web-view-impl') if (contextIsolation) { - v8Util.setHiddenValue(window, 'web-view-impl', webViewImpl) + v8Util.setHiddenValue(window, 'web-view-impl', webViewImplModule) } else { const { setupWebView } = require('@electron/internal/renderer/web-view/web-view-element') - setupWebView(v8Util, webViewImpl) + setupWebView(v8Util, webViewImplModule) } } diff --git a/lib/sandboxed_renderer/init.js b/lib/sandboxed_renderer/init.js index d9bf399004999..7537276e40f76 100644 --- a/lib/sandboxed_renderer/init.js +++ b/lib/sandboxed_renderer/init.js @@ -116,7 +116,8 @@ const guestInstanceId = binding.guestInstanceId && parseInt(binding.guestInstanc // Load webview tag implementation. if (process.isMainFrame) { - require('@electron/internal/renderer/web-view/web-view-init')(contextIsolation, isWebViewTagEnabled, guestInstanceId) + const { webViewInit } = require('@electron/internal/renderer/web-view/web-view-init') + webViewInit(contextIsolation, isWebViewTagEnabled, guestInstanceId) } const errorUtils = require('@electron/internal/common/error-utils') diff --git a/typings/internal-ambient.d.ts b/typings/internal-ambient.d.ts index 130c181958f64..8c5f53281d34d 100644 --- a/typings/internal-ambient.d.ts +++ b/typings/internal-ambient.d.ts @@ -62,5 +62,56 @@ declare interface Window { FileSystemWorkspaceBinding: { completeURL: (project: string, path: string) => string; } - } + }; + ResizeObserver: ResizeObserver; +} + +/** + * The ResizeObserver interface is used to observe changes to Element's content + * rect. + * + * It is modeled after MutationObserver and IntersectionObserver. + */ +declare class ResizeObserver { + constructor (callback: ResizeObserverCallback); + + /** + * Adds target to the list of observed elements. + */ + observe: (target: Element) => void; + + /** + * Removes target from the list of observed elements. + */ + unobserve: (target: Element) => void; + + /** + * Clears both the observationTargets and activeTargets lists. + */ + disconnect: () => void; +} + +/** + * This callback delivers ResizeObserver's notifications. It is invoked by a + * broadcast active observations algorithm. + */ +interface ResizeObserverCallback { + (entries: ResizeObserverEntry[], observer: ResizeObserver): void; } + +interface ResizeObserverEntry { + /** + * @param target The Element whose size has changed. + */ + new (target: Element): ResizeObserverEntry; + + /** + * The Element whose size has changed. + */ + readonly target: Element; + + /** + * Element's content rect when ResizeObserverCallback is invoked. + */ + readonly contentRect: DOMRectReadOnly; +} \ No newline at end of file diff --git a/typings/internal-electron.d.ts b/typings/internal-electron.d.ts index cc2566b936923..6a73f6d8cca65 100644 --- a/typings/internal-electron.d.ts +++ b/typings/internal-electron.d.ts @@ -80,6 +80,34 @@ declare namespace ElectronInternal { on(channel: string, listener: (event: IpcMainInternalEvent, ...args: any[]) => void): this; once(channel: string, listener: (event: IpcMainInternalEvent, ...args: any[]) => void): this; } + + interface WebFrameInternal extends Electron.WebFrame { + getWebFrameId(window: Window): number; + allowGuestViewElementDefinition(window: Window, context: any): void; + } + + interface WebFrameResizeEvent extends Electron.Event { + newWidth: number; + newHeight: number; + } + + interface WebViewEvent extends Event { + url: string; + isMainFrame: boolean; + } + + abstract class WebViewElement extends HTMLElement { + static observedAttributes: Array; + + public contentWindow: Window; + + public connectedCallback(): void; + public attributeChangedCallback(): void; + public disconnectedCallback(): void; + + // Created in web-view-impl + public getWebContents(): Electron.WebContents; + } } declare namespace Chrome { From ac88b3ead5e1a7ba89b6b96d46439754c45fcb42 Mon Sep 17 00:00:00 2001 From: Samuel Maddock Date: Thu, 7 Mar 2019 18:29:37 -0500 Subject: [PATCH 21/22] feat: add 'disableHtmlFullscreenWindowResize' option to webPreferences (#17203) This option allows users to prevent the window from resizing when the HTML5 FullScreen API is used. --- atom/browser/common_web_contents_delegate.cc | 12 ++++++++++- atom/browser/web_contents_preferences.cc | 4 ++++ atom/common/options_switches.cc | 8 +++++++ atom/common/options_switches.h | 2 ++ docs/api/browser-window.md | 3 +++ docs/api/web-contents.md | 8 +++++++ spec/api-browser-window-spec.js | 22 ++++++++++++++++++++ 7 files changed, 58 insertions(+), 1 deletion(-) diff --git a/atom/browser/common_web_contents_delegate.cc b/atom/browser/common_web_contents_delegate.cc index dae1ec367d67a..07b5a5b7e5ec5 100644 --- a/atom/browser/common_web_contents_delegate.cc +++ b/atom/browser/common_web_contents_delegate.cc @@ -619,7 +619,17 @@ void CommonWebContentsDelegate::SetHtmlApiFullscreen(bool enter_fullscreen) { return; } - owner_window_->SetFullScreen(enter_fullscreen); + // Set fullscreen on window if allowed. + auto* web_preferences = WebContentsPreferences::From(GetWebContents()); + bool html_fullscreenable = + web_preferences ? !web_preferences->IsEnabled( + options::kDisableHtmlFullscreenWindowResize) + : true; + + if (html_fullscreenable) { + owner_window_->SetFullScreen(enter_fullscreen); + } + html_fullscreen_ = enter_fullscreen; native_fullscreen_ = false; } diff --git a/atom/browser/web_contents_preferences.cc b/atom/browser/web_contents_preferences.cc index c2713f9bb31b2..260cad9a79493 100644 --- a/atom/browser/web_contents_preferences.cc +++ b/atom/browser/web_contents_preferences.cc @@ -122,6 +122,7 @@ WebContentsPreferences::WebContentsPreferences( SetDefaultBoolIfUndefined(options::kNodeIntegration, false); SetDefaultBoolIfUndefined(options::kNodeIntegrationInSubFrames, false); SetDefaultBoolIfUndefined(options::kNodeIntegrationInWorker, false); + SetDefaultBoolIfUndefined(options::kDisableHtmlFullscreenWindowResize, false); SetDefaultBoolIfUndefined(options::kWebviewTag, false); SetDefaultBoolIfUndefined(options::kSandbox, false); SetDefaultBoolIfUndefined(options::kNativeWindowOpen, false); @@ -371,6 +372,9 @@ void WebContentsPreferences::AppendCommandLineSwitches( if (IsEnabled(options::kNodeIntegrationInSubFrames)) command_line->AppendSwitch(switches::kNodeIntegrationInSubFrames); + if (IsEnabled(options::kDisableHtmlFullscreenWindowResize)) + command_line->AppendSwitch(switches::kDisableHtmlFullscreenWindowResize); + // We are appending args to a webContents so let's save the current state // of our preferences object so that during the lifetime of the WebContents // we can fetch the options used to initally configure the WebContents diff --git a/atom/common/options_switches.cc b/atom/common/options_switches.cc index a01681ca1b5c4..b22631651d877 100644 --- a/atom/common/options_switches.cc +++ b/atom/common/options_switches.cc @@ -156,6 +156,10 @@ const char kOffscreen[] = "offscreen"; const char kNodeIntegrationInSubFrames[] = "nodeIntegrationInSubFrames"; +// Disable window resizing when HTML Fullscreen API is activated. +const char kDisableHtmlFullscreenWindowResize[] = + "disableHtmlFullscreenWindowResize"; + } // namespace options namespace switches { @@ -220,6 +224,10 @@ const char kNodeIntegrationInWorker[] = "node-integration-in-worker"; // environments will be created in sub-frames. const char kNodeIntegrationInSubFrames[] = "node-integration-in-subframes"; +// Disable window resizing when HTML Fullscreen API is activated. +const char kDisableHtmlFullscreenWindowResize[] = + "disable-html-fullscreen-window-resize"; + // Widevine options // Path to Widevine CDM binaries. const char kWidevineCdmPath[] = "widevine-cdm-path"; diff --git a/atom/common/options_switches.h b/atom/common/options_switches.h index 80aafa3db62bf..3b999faa8e8b3 100644 --- a/atom/common/options_switches.h +++ b/atom/common/options_switches.h @@ -76,6 +76,7 @@ extern const char kWebSecurity[]; extern const char kAllowRunningInsecureContent[]; extern const char kOffscreen[]; extern const char kNodeIntegrationInSubFrames[]; +extern const char kDisableHtmlFullscreenWindowResize[]; } // namespace options @@ -111,6 +112,7 @@ extern const char kNativeWindowOpen[]; extern const char kNodeIntegrationInWorker[]; extern const char kWebviewTag[]; extern const char kNodeIntegrationInSubFrames[]; +extern const char kDisableHtmlFullscreenWindowResize[]; extern const char kWidevineCdmPath[]; extern const char kWidevineCdmVersion[]; diff --git a/docs/api/browser-window.md b/docs/api/browser-window.md index b5dc64d81d269..7f31f397941e3 100644 --- a/docs/api/browser-window.md +++ b/docs/api/browser-window.md @@ -379,6 +379,9 @@ It creates a new `BrowserWindow` with native properties as set by the `options`. content in the window, can be `no-user-gesture-required`, `user-gesture-required`, `document-user-activation-required`. Defaults to `no-user-gesture-required`. + * `disableHtmlFullscreenWindowResize` Boolean (optional) - Whether to + prevent the window from resizing when entering HTML Fullscreen. Default + is `false`. When setting minimum or maximum window size with `minWidth`/`maxWidth`/ `minHeight`/`maxHeight`, it only constrains the users. It won't prevent you from diff --git a/docs/api/web-contents.md b/docs/api/web-contents.md index e4972bfa5593a..f457921090c28 100644 --- a/docs/api/web-contents.md +++ b/docs/api/web-contents.md @@ -361,6 +361,14 @@ win.webContents.on('before-input-event', (event, input) => { }) ``` +#### Event: 'enter-html-full-screen' + +Emitted when the window enters a full-screen state triggered by HTML API. + +#### Event: 'leave-html-full-screen' + +Emitted when the window leaves a full-screen state triggered by HTML API. + #### Event: 'devtools-opened' Emitted when DevTools is opened. diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index 8b83847041d5f..28748ed7e8514 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -2097,6 +2097,28 @@ describe('BrowserWindow module', () => { expect(typeofProcess).to.eql('undefined') }) }) + + describe('"disableHtmlFullscreenWindowResize" option', () => { + it('prevents window from resizing when set', (done) => { + w.destroy() + w = new BrowserWindow({ + show: false, + webPreferences: { + disableHtmlFullscreenWindowResize: true + } + }) + w.webContents.once('did-finish-load', () => { + const size = w.getSize() + w.webContents.once('enter-html-full-screen', () => { + const newSize = w.getSize() + expect(newSize).to.deep.equal(size) + done() + }) + w.webContents.executeJavaScript('document.body.webkitRequestFullscreen()', true) + }) + w.loadURL('about:blank') + }) + }) }) describe('nativeWindowOpen + contextIsolation options', () => { From 5791a2a9ec4ecdb95ad4dc8187aff778788f8d84 Mon Sep 17 00:00:00 2001 From: Milan Burda Date: Fri, 8 Mar 2019 00:31:25 +0100 Subject: [PATCH 22/22] refactor: use ipcRendererUtils.invoke / ipcMainUtils.handle for desktopCapturer.getSources() (#16619) --- lib/browser/desktop-capturer.js | 56 +++++++++++++--------------- lib/renderer/api/desktop-capturer.js | 25 +++---------- 2 files changed, 31 insertions(+), 50 deletions(-) diff --git a/lib/browser/desktop-capturer.js b/lib/browser/desktop-capturer.js index 6f5ef9e3bdf7f..0211b1505d6cf 100644 --- a/lib/browser/desktop-capturer.js +++ b/lib/browser/desktop-capturer.js @@ -1,6 +1,6 @@ 'use strict' -const { ipcMainInternal } = require('@electron/internal/browser/ipc-main-internal') +const ipcMainUtils = require('@electron/internal/browser/ipc-main-internal-utils') const { desktopCapturer } = process.atomBinding('desktop_capturer') const eventBinding = process.atomBinding('event') @@ -10,37 +10,34 @@ const deepEqual = (a, b) => JSON.stringify(a) === JSON.stringify(b) // A queue for holding all requests from renderer process. let requestsQueue = [] -const electronSources = 'ELECTRON_BROWSER_DESKTOP_CAPTURER_GET_SOURCES' -const capturerResult = (id) => `ELECTRON_RENDERER_DESKTOP_CAPTURER_RESULT_${id}` - -ipcMainInternal.on(electronSources, (event, captureWindow, captureScreen, thumbnailSize, fetchWindowIcons, id) => { +ipcMainUtils.handle('ELECTRON_BROWSER_DESKTOP_CAPTURER_GET_SOURCES', (event, captureWindow, captureScreen, thumbnailSize, fetchWindowIcons) => { const customEvent = eventBinding.createWithSender(event.sender) event.sender.emit('desktop-capturer-get-sources', customEvent) if (customEvent.defaultPrevented) { - event._replyInternal(capturerResult(id), []) - return + return [] } - const request = { - id, - options: { - captureWindow, - captureScreen, - thumbnailSize, - fetchWindowIcons - }, - event - } - requestsQueue.push(request) - if (requestsQueue.length === 1) { - desktopCapturer.startHandling(captureWindow, captureScreen, thumbnailSize, fetchWindowIcons) - } + return new Promise((resolve, reject) => { + const request = { + options: { + captureWindow, + captureScreen, + thumbnailSize, + fetchWindowIcons + }, + resolve + } + requestsQueue.push(request) + if (requestsQueue.length === 1) { + desktopCapturer.startHandling(captureWindow, captureScreen, thumbnailSize, fetchWindowIcons) + } - // If the WebContents is destroyed before receiving result, just remove the - // reference from requestsQueue to make the module not send the result to it. - event.sender.once('destroyed', () => { - request.event = null + // If the WebContents is destroyed before receiving result, just remove the + // reference from requestsQueue to make the module not send the result to it. + event.sender.once('destroyed', () => { + request.resolve = null + }) }) }) @@ -59,16 +56,15 @@ desktopCapturer.emit = (event, name, sources, fetchWindowIcons) => { } }) - if (handledRequest.event) { - handledRequest.event._replyInternal(capturerResult(handledRequest.id), result) + if (handledRequest.resolve) { + handledRequest.resolve(result) } // Check the queue to see whether there is another identical request & handle requestsQueue.forEach(request => { - const event = request.event if (deepEqual(handledRequest.options, request.options)) { - if (event) { - event._replyInternal(capturerResult(request.id), result) + if (request.resolve) { + request.resolve(result) } } else { unhandledRequestsQueue.push(request) diff --git a/lib/renderer/api/desktop-capturer.js b/lib/renderer/api/desktop-capturer.js index 816fa2394b30a..c1f2c220b3448 100644 --- a/lib/renderer/api/desktop-capturer.js +++ b/lib/renderer/api/desktop-capturer.js @@ -1,15 +1,7 @@ 'use strict' const { nativeImage, deprecate } = require('electron') -const { ipcRendererInternal } = require('@electron/internal/renderer/ipc-renderer-internal') - -const includes = [].includes -let currentId = 0 - -const incrementId = () => { - currentId += 1 - return currentId -} +const ipcRendererUtils = require('@electron/internal/renderer/ipc-renderer-internal-utils') // |options.types| can't be empty and must be an array function isValid (options) { @@ -31,8 +23,8 @@ const getSources = (options) => { return new Promise((resolve, reject) => { if (!isValid(options)) throw new Error('Invalid options') - const captureWindow = includes.call(options.types, 'window') - const captureScreen = includes.call(options.types, 'screen') + const captureWindow = options.types.includes('window') + const captureScreen = options.types.includes('screen') if (options.thumbnailSize == null) { options.thumbnailSize = { @@ -44,15 +36,8 @@ const getSources = (options) => { options.fetchWindowIcons = false } - const id = incrementId() - ipcRendererInternal.send('ELECTRON_BROWSER_DESKTOP_CAPTURER_GET_SOURCES', captureWindow, captureScreen, options.thumbnailSize, options.fetchWindowIcons, id) - return ipcRendererInternal.once(`ELECTRON_RENDERER_DESKTOP_CAPTURER_RESULT_${id}`, (event, sources) => { - try { - resolve(mapSources(sources)) - } catch (error) { - reject(error) - } - }) + ipcRendererUtils.invoke('ELECTRON_BROWSER_DESKTOP_CAPTURER_GET_SOURCES', captureWindow, captureScreen, options.thumbnailSize, options.fetchWindowIcons) + .then(sources => resolve(mapSources(sources)), reject) }) }