Skip to content
This repository has been archived by the owner on Jan 4, 2019. It is now read-only.

Commit

Permalink
fix webkit bug with release of context menu node
Browse files Browse the repository at this point in the history
always call SetShowingContextMenu/NotifyContextMenuClosed
fix brave/browser-laptop#10563
also fixes flash context menu items not working
  • Loading branch information
bridiver committed Oct 4, 2017
1 parent 7f036a9 commit b80850f
Show file tree
Hide file tree
Showing 9 changed files with 84 additions and 27 deletions.
5 changes: 4 additions & 1 deletion atom/browser/api/atom_api_menu.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ class Menu : public mate::TrackableObject<Menu>,
public AtomMenuModel::Delegate,
public MenuObserver {
public:
typedef const base::Callback<void(int)>& CloseCallback;

static mate::WrappableBase* New(mate::Arguments* args);

static void BuildPrototype(v8::Isolate* isolate,
Expand Down Expand Up @@ -63,7 +65,8 @@ class Menu : public mate::TrackableObject<Menu>,
void MenuWillShow(ui::SimpleMenuModel* source) override;

virtual void PopupAt(
Window* window, int x, int y, int positioning_item) = 0;
Window* window, int x, int y, int positioning_item,
CloseCallback callback) = 0;
virtual void ClosePopupAt(int32_t window_id) = 0;

std::unique_ptr<AtomMenuModel> model_;
Expand Down
6 changes: 4 additions & 2 deletions atom/browser/api/atom_api_menu_mac.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,11 @@ class MenuMac : public Menu {
MenuMac(v8::Isolate* isolate, v8::Local<v8::Object> wrapper);

void PopupAt(
Window* window, int x, int y, int positioning_item) override;
Window* window, int x, int y, int positioning_item,
const base::Callback<void(int)>& callback) override;
void PopupOnUI(const base::WeakPtr<NativeWindow>& native_window,
int32_t window_id, int x, int y, int positioning_item);
int32_t window_id, int x, int y, int positioning_item,
CloseCallback callback);
void ClosePopupAt(int32_t window_id) override;

private:
Expand Down
11 changes: 6 additions & 5 deletions atom/browser/api/atom_api_menu_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -28,29 +28,30 @@
}

void MenuMac::PopupAt(
Window* window, int x, int y, int positioning_item) {
Window* window, int x, int y, int positioning_item,
CloseCallback callback) {
NativeWindow* native_window = window->window();
if (!native_window)
return;

auto popup = base::Bind(&MenuMac::PopupOnUI, weak_factory_.GetWeakPtr(),
native_window->GetWeakPtr(), window->ID(), x, y,
positioning_item);
positioning_item, callback);

BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, popup);
}

void MenuMac::PopupOnUI(const base::WeakPtr<NativeWindow>& native_window,
int32_t window_id, int x, int y, int positioning_item) {
int32_t window_id, int x, int y, int positioning_item,
CloseCallback callback) {
if (!native_window)
return;
brightray::InspectableWebContents* web_contents =
native_window->inspectable_web_contents();
if (!web_contents || !web_contents->GetWebContents())
return;

auto close_callback = base::Bind(&MenuMac::ClosePopupAt,
weak_factory_.GetWeakPtr(), window_id);
auto close_callback = base::Bind(callback, window_id);
popup_controllers_[window_id] = base::scoped_nsobject<AtomMenuController>(
[[AtomMenuController alloc] initWithModel:model()
useDefaultAccelerator:NO]);
Expand Down
6 changes: 3 additions & 3 deletions atom/browser/api/atom_api_menu_views.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ MenuViews::MenuViews(v8::Isolate* isolate, v8::Local<v8::Object> wrapper)
}

void MenuViews::PopupAt(
Window* window, int x, int y, int positioning_item) {
Window* window, int x, int y, int positioning_item,
CloseCallback callback) {
NativeWindow* native_window = static_cast<NativeWindow*>(window->window());
if (!native_window)
return;
Expand All @@ -50,8 +51,7 @@ void MenuViews::PopupAt(

// Show the menu.
int32_t window_id = window->ID();
auto close_callback = base::Bind(
&MenuViews::ClosePopupAt, weak_factory_.GetWeakPtr(), window_id);
auto close_callback = base::Bind(callback, window_id);
menu_runners_[window_id] = std::unique_ptr<MenuRunner>(new MenuRunner(
model(), flags, close_callback));
menu_runners_[window_id]->RunMenuAt(
Expand Down
20 changes: 17 additions & 3 deletions atom/browser/api/atom_api_web_contents.cc
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@
#include "content/public/browser/storage_partition.h"
#include "content/public/browser/web_contents.h"
#include "content/public/child/v8_value_converter.h"
#include "content/public/common/context_menu_params.h"
#include "content/public/common/window_container_type.mojom.h"
#include "native_mate/dictionary.h"
#include "native_mate/object_template_builder.h"
Expand Down Expand Up @@ -977,19 +976,32 @@ void WebContents::RendererResponsive(content::WebContents* source) {
owner_window()->RendererResponsive(source);
}

bool WebContents::OnContextMenuShow() {
if (web_contents()->IsShowingContextMenu())
return false;

web_contents()->SetShowingContextMenu(true);
return true;
}

void WebContents::OnContextMenuClose() {
web_contents()->SetShowingContextMenu(false);
web_contents()->NotifyContextMenuClosed(context_menu_params_.custom_context);
}

bool WebContents::HandleContextMenu(const content::ContextMenuParams& params) {
context_menu_params_ = content::ContextMenuParams(params);

#if BUILDFLAG(ENABLE_PLUGINS)
if (params.custom_context.request_id &&
!params.custom_context.is_pepper_menu) {
Emit("enable-pepper-menu", std::make_pair(params, web_contents()));
web_contents()->NotifyContextMenuClosed(params.custom_context);
return true;
}
#endif

if (params.custom_context.is_pepper_menu) {
Emit("pepper-context-menu", std::make_pair(params, web_contents()));
web_contents()->NotifyContextMenuClosed(params.custom_context);
} else {
// For forgetting custom dictionary option
content::ContextMenuParams new_params(params);
Expand Down Expand Up @@ -2521,6 +2533,8 @@ void WebContents::BuildPrototype(v8::Isolate* isolate,
&WebContents::EnablePreferredSizeMode)
.SetMethod("authorizePlugin",
&WebContents::AuthorizePlugin)
.SetMethod("onContextMenuShow", &WebContents::OnContextMenuShow)
.SetMethod("onContextMenuClose", &WebContents::OnContextMenuClose)
#if BUILDFLAG(ENABLE_EXTENSIONS)
.SetMethod("executeScriptInTab", &WebContents::ExecuteScriptInTab)
.SetMethod("isBackgroundPage", &WebContents::IsBackgroundPage)
Expand Down
6 changes: 6 additions & 0 deletions atom/browser/api/atom_api_web_contents.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "content/common/view_messages.h"
#include "content/public/browser/service_worker_context.h"
#include "content/public/browser/web_contents_observer.h"
#include "content/public/common/context_menu_params.h"
#include "content/public/common/favicon_url.h"
#include "extensions/features/features.h"
#include "native_mate/handle.h"
Expand Down Expand Up @@ -426,6 +427,8 @@ class WebContents : public mate::TrackableObject<WebContents>,
content::WebContents* source,
const content::WebContentsUnresponsiveState& unresponsive_state) override;
void RendererResponsive(content::WebContents* source) override;
bool OnContextMenuShow();
void OnContextMenuClose();
bool HandleContextMenu(const content::ContextMenuParams& params) override;
bool OnGoToEntryOffset(int offset) override;
void FindReply(content::WebContents* web_contents,
Expand Down Expand Up @@ -553,6 +556,9 @@ class WebContents : public mate::TrackableObject<WebContents>,

guest_view::GuestViewBase* guest_delegate_; // not owned

// the context menu params for the current context menu;
content::ContextMenuParams context_menu_params_;

std::unique_ptr<base::MemoryPressureListener> memory_pressure_listener_;
DISALLOW_COPY_AND_ASSIGN(WebContents);
};
Expand Down
43 changes: 31 additions & 12 deletions lib/browser/api/menu.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,17 +143,36 @@ Menu.prototype._init = function () {
}
}

Menu.prototype.popup = function (window, x, y, positioningItem) {
Menu.prototype.popup = function (sender, x, y, positioningItem) {
// menu.popup(x, y, positioningItem)
if (typeof window !== 'object' || window.constructor !== BrowserWindow) {
let win
if (typeof sender === 'object') {
if (sender.constructor === BrowserWindow) {
win = sender
sender = win.webContents
} else {
const embedder = sender.hostWebContents || sender
win = BrowserWindow.fromWebContents(embedder)
}
} else {
// Shift.
positioningItem = y
y = x
x = window
window = BrowserWindow.getFocusedWindow()
x = win
win = BrowserWindow.getFocusedWindow()
sender = win.webContents
}

if (win == null || win.isDestroyed()) {
return
}

// menu.popup(window, {})
this.sourceWebContents = sender
if (this.sourceWebContents == null || this.sourceWebContents.isDestroyed()) {
return
}

// menu.popup(win, {})
if (x != null && typeof x === 'object') {
const options = x
x = options.x
Expand All @@ -168,15 +187,15 @@ Menu.prototype.popup = function (window, x, y, positioningItem) {
// Default to not highlighting any item.
if (typeof positioningItem !== 'number') positioningItem = -1

this.popupAt(window, x, y, positioningItem)
if (this.sourceWebContents.onContextMenuShow()) {
this.popupAt(win, x, y, positioningItem, this.closePopup.bind(this))
}
}

Menu.prototype.closePopup = function (window) {
if (window == null || window.constructor !== BrowserWindow) {
window = BrowserWindow.getFocusedWindow()
}
if (window != null) {
this.closePopupAt(window.id)
Menu.prototype.closePopup = function (window_id) {
this.closePopupAt(window_id)
if (!this.sourceWebContents.isDestroyed()) {
this.sourceWebContents.onContextMenuClose()
}
}

Expand Down
2 changes: 1 addition & 1 deletion lib/browser/api/web-contents.js
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ WebContents.prototype._init = function () {
this.on('pepper-context-menu', function (event, params) {
// Access Menu via electron.Menu to prevent circular require
const menu = electron.Menu.buildFromTemplate(params.menu)
menu.popup(params.x, params.y)
menu.popup(this, params.x, params.y)
})

// The devtools requests the webContents to reload.
Expand Down
12 changes: 12 additions & 0 deletions patches/master_patch.patch
Original file line number Diff line number Diff line change
Expand Up @@ -2182,6 +2182,18 @@ index 5fbf9daf8b150748327b60b8a1c0e64a308985df..d9a520b1db4c331bd2beceb2dc29cf02
IsAutoplayAllowedPerSettings()) {
return false;
}
diff --git a/third_party/WebKit/Source/web/WebViewImpl.cpp b/third_party/WebKit/Source/web/WebViewImpl.cpp
index f7fc7dd686a5f25e7c6fdabd760fedc7a4a2ccfe..f2ffd987885cb032357c4176d0e4394b060b577e 100644
--- a/third_party/WebKit/Source/web/WebViewImpl.cpp
+++ b/third_party/WebKit/Source/web/WebViewImpl.cpp
@@ -3457,6 +3457,7 @@ void WebViewImpl::DidCloseContextMenu() {
LocalFrame* frame = page_->GetFocusController().FocusedFrame();
if (frame)
frame->Selection().SetCaretBlinkingSuspended(false);
+ page_->GetContextMenuController().ClearContextMenu();
}

void WebViewImpl::HidePopups() {
diff --git a/third_party/boringssl/BUILD.generated.gni b/third_party/boringssl/BUILD.generated.gni
index fab23921a1432cd0605e73190555797915589656..33f7699cbc6bd9ebdea0bd9a829b1f6b13e87aa3 100644
--- a/third_party/boringssl/BUILD.generated.gni
Expand Down

0 comments on commit b80850f

Please sign in to comment.