Allow creating new windows from a notificationclick event in SW.

Allow creating new windows from a notificationclick event in SW.

This is re-using ExecutionContext windowfocus tokens but generalize them
for window interactions.

In addition, the token has to be used in the 10s after granting,
otherwise it is lost.

BUG=437151
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=189530

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=189557

[mlamouri (slow - plz ping), 2015-01-27 16:46:18]

mlamouri@chromium.org changed reviewers:
+ jochen@chromium.org

[mlamouri (slow - plz ping), 2015-01-27 16:46:18]

Jochen, can you have a look? :)

[jochen (gone - plz use gerrit), 2015-01-29 14:39:19]

instead of using a delayed task, could you just record the last time it was set,
and only return true of isWindowInteractionAllowed() if there are tokens and the
last access time is not more than 10s in the past?

[mlamouri (slow - plz ping), 2015-01-29 15:30:10]

On 2015/01/29 at 14:39:19, jochen wrote:
> instead of using a delayed task, could you just record the last time it was
set, and only return true of isWindowInteractionAllowed() if there are tokens
and the last access time is not more than 10s in the past?

Doesn't that mean that an attacker would be able to open multiple windows at the
same time if he/she is able to keep N notificationclick alives and open all the
windows after the Nth arrives?

[mlamouri (slow - plz ping), 2015-02-02 15:52:21]

Jochen, could you PTAL?

[jochen (gone - plz use gerrit), 2015-02-04 12:44:10]

On 2015/01/29 at 15:30:10, mlamouri wrote:
> On 2015/01/29 at 14:39:19, jochen wrote:
> > instead of using a delayed task, could you just record the last time it was
set, and only return true of isWindowInteractionAllowed() if there are tokens
and the last access time is not more than 10s in the past?
> 
> Doesn't that mean that an attacker would be able to open multiple windows at
the same time if he/she is able to keep N notificationclick alives and open all
the windows after the Nth arrives?

I don't understand exactly what you mean?

the attacker can only collect them if the user clicks on the notification every
10s

[mlamouri (slow - plz ping), 2015-02-04 14:22:38]

After discussing offline with jochen@, he agreed to keep the current design.
Using a timestamp would not work if there is an event spinning indefinitely
(waitUntil() is kept alive as long as the promises linked to it are not
resolved) and then another event comes, update the timestamp, allowing both
events to interact with a window, even if the first one was more than 10s old.

I will update the patch with some tests though.

[mlamouri (slow - plz ping), 2015-02-04 16:13:20]

mlamouri@chromium.org changed reviewers:
+ mkwst@chromium.org

[mlamouri (slow - plz ping), 2015-02-04 16:13:21]

After investigation, I will need to write some LayoutTests because unit tests
will be hard for this. Mostly because of the SWGlobalScope.

Unfortunately, the feature is not yet entirely finished and given that
LayoutTests are integration test, I would have to write them later.

mkwst@, how does that sound?

[jochen (gone - plz use gerrit), 2015-02-04 17:05:51]

assuming you can get it to pass tests and will have layout tests really soon now
this looks good

[mlamouri (slow - plz ping), 2015-02-04 17:14:02]

Rebased again (previous one was on Blink revision in Chromium).

[mlamouri (slow - plz ping), 2015-02-04 17:52:22]

On 2015/02/04 at 17:05:51, jochen wrote:
> assuming you can get it to pass tests and will have layout tests really soon
now this looks good

Strictly and highly WIP tests: https://codereview.chromium.org/896043004
Otherwise the try seems happy so far.

[Mike West, 2015-02-04 18:43:43]

LGTM.

[mlamouri (slow - plz ping), 2015-02-04 19:48:03]

The CQ bit was checked by mlamouri@chromium.org

[commit-bot: I haz the power, 2015-02-04 19:49:14]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/866983004/40001

[commit-bot: I haz the power, 2015-02-04 20:51:05]

Committed patchset #3 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=189530

[mlamouri (slow - plz ping), 2015-02-05 10:35:52]

New patchsets have been uploaded after l-g-t-m from mkwst@chromium.org

[mlamouri (slow - plz ping), 2015-02-05 10:37:13]

Rebasing and relanding because it was reverted as a result of collateral damage
from https://codereview.chromium.org/901663005/

[mlamouri (slow - plz ping), 2015-02-05 10:37:16]

The CQ bit was checked by mlamouri@chromium.org

[commit-bot: I haz the power, 2015-02-05 10:38:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/866983004/60001

[pfeldman, 2015-02-05 13:47:07]

Sorry, this was reverted since it was blocking the breakage revert:
https://codereview.chromium.org/901663005/. Please rebaseline it and land. Sorry
for inconvenience.

[commit-bot: I haz the power, 2015-02-05 15:11:21]

Committed patchset #4 (id:60001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=189557