Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(161)

Issue 803253003: Revert of Bind Window focus ability to the ExecutionContext instead of the process. (Closed)

Created:
6 years ago by amineer_google
Modified:
6 years ago
CC:
jochen (gone - plz use gerrit), blink-reviews, blink-reviews-dom_chromium.org, dglazkov+blink, eae+blinkwatch, falken, horo+watch_chromium.org, jsbell+serviceworker_chromium.org, kinuko+serviceworker, michaeln, nhiroki, rwlbuis, serviceworker-reviews, sof, tzik
Base URL:
https://chromium.googlesource.com/chromium/blink.git@sw_client_focus
Project:
blink
Visibility:
Public.

Description

Revert of Bind Window focus ability to the ExecutionContext instead of the process. (patchset #3 id:40001 of https://codereview.chromium.org/777483004/) Reason for revert: Crashing, see bug 441968 Original issue's description: > Bind Window focus ability to the ExecutionContext instead of the process. > > WindowFocusAllowedIndicator is using a static variable that allows > the entire process to focus a window. In addition, it is entirely > stack based which prevents asynchronous methods to use it properly. > > By binding the window focus ability to the ExecutionContext, > we can allow asynchronous methods but also, reduces the chances > of unrelated scripts to focus windows when they should not be > able to. Those two concerns are slightly related because an > ExecutionContext will now be able to focus a window for a > long period of time, thus, we do not want other ExecutionContext > to be able to do that in the meantime. > > This is part of a three-sided CL: > Part 1: <this> > Part 2: https://codereview.chromium.org/786403003 > Part 3: https://codereview.chromium.org/722423004 > > BUG=440740 > > Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=186849 TBR=jochen@chromium.org,mkwst@chromium.org,mlamouri@chromium.org NOTREECHECKS=true NOTRY=true BUG=440740 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=187209

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+15 lines, -151 lines) Patch
M LayoutTests/http/tests/serviceworker/client-focus.html View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/core.gypi View 1 chunk +0 lines, -2 lines 0 comments Download
M Source/core/dom/ExecutionContext.h View 2 chunks +0 lines, -10 lines 0 comments Download
M Source/core/dom/ExecutionContext.cpp View 3 chunks +0 lines, -22 lines 0 comments Download
D Source/core/dom/ScopedWindowFocusAllowedIndicator.h View 1 chunk +0 lines, -24 lines 0 comments Download
D Source/core/dom/ScopedWindowFocusAllowedIndicator.cpp View 1 chunk +0 lines, -26 lines 0 comments Download
M Source/core/frame/LocalDOMWindow.cpp View 1 chunk +8 lines, -7 lines 0 comments Download
M Source/modules/notifications/Notification.cpp View 2 chunks +2 lines, -2 lines 0 comments Download
M Source/modules/serviceworkers/ServiceWorkerClient.cpp View 2 chunks +3 lines, -6 lines 0 comments Download
M Source/modules/serviceworkers/WaitUntilObserver.cpp View 2 chunks +0 lines, -11 lines 0 comments Download
M Source/web/WebScopedWindowFocusAllowedIndicator.cpp View 2 chunks +0 lines, -11 lines 0 comments Download
M Source/web/tests/WebScopedWindowFocusAllowedIndicatorTest.cpp View 2 chunks +1 line, -20 lines 0 comments Download
M public/web/WebScopedWindowFocusAllowedIndicator.h View 1 chunk +0 lines, -9 lines 0 comments Download

Messages

Total messages: 9 (3 generated)
amineer_google
Created Revert of Bind Window focus ability to the ExecutionContext instead of the process.
6 years ago (2014-12-15 22:57:47 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/803253003/1
6 years ago (2014-12-15 22:58:45 UTC) #2
commit-bot: I haz the power
No LGTM from a valid reviewer yet. Only full committers are accepted. Even if an ...
6 years ago (2014-12-15 22:58:47 UTC) #4
Ken Rockot(use gerrit already)
lgtm pending the revert of https://codereview.chromium.org/786403003 in chromium
6 years ago (2014-12-15 22:59:56 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/803253003/1
6 years ago (2014-12-16 02:38:52 UTC) #8
commit-bot: I haz the power
6 years ago (2014-12-16 02:40:27 UTC) #9
Message was sent while issue was closed.
Committed patchset #1 (id:1) as
https://src.chromium.org/viewvc/blink?view=rev&revision=187209

Powered by Google App Engine
This is Rietveld 408576698