881797 - [Activity] Generalize Activity Handler throttling

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: General, defect)

Description

[Mike Pennisi [:jugglinmike]]

Incoming Activities may occur at high frequency in response to accidental user input. The Messaging application currently protects against this case for the "new" Activity specifically via a software lock. Because the problem exists for any arbitrary Activity, this logic should be generalized as a throttle for each Activity handler.

Comment 1

[Julien Wajsberg [:julienw]]

I think this is more general than Gaia, and may need to be done in Gecko.

Vivien, I'm sure you have useful thoughts here :)

Comment 2

[Mike Pennisi [:jugglinmike]]

Stepping off of this bug to focus on higher-priority Messaging work

Comment 3

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to Julien Wajsberg [:julienw] (PTO 15th -> 28th July) from comment #1)
> I think this is more general than Gaia, and may need to be done in Gecko.
> 
> Vivien, I'm sure you have useful thoughts here :)

That makes sense to me. Instead of managing that at the app level or at the platform level I wonder if this can be managed at the system app level. The code responsible for activities lives in apps/system/js/activities.js

Activities are supposed to be response to user input. So this code could reject any activities that comes less than 1sec after the initial one. Or it could deny them if they are a displayed activity chooser and a new activity is popping up.

Let me CC Alive on the bug.

Comment 4

[Julien Wajsberg [:julienw]]

I was thinking about doing this in Gecko because, well, this is probably desirable in other platforms than B2G, right ?

Comment 5

[(inactive after 6/18) Alive Kuo [:alive]]

I tend to do this in gecko because the user input rejection is done in gecko, too.
Do half of things in gaia and another half in gecko doesn't sound good to me.

I could try to patch activity in gecko if :fabrice is too busy.

Comment 6

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to Vivien Nicolas (:vingtetun) (:21) from comment #3)
> 
> Activities are supposed to be response to user input. So this code could
> reject any activities that comes less than 1sec after the initial one. Or it
> could deny them if they are a displayed activity chooser and a new activity
> is popping up.

I think we should just deny the following too fast activity request. But AFAIK currently in the activity-choice, gaia has no way to deny but only have to choose one of the choices. So this should be done in gecko. Or both.

Comment 7

[(inactive after 6/18) Alive Kuo [:alive]]

Reading activity.c and it suprised me that "NoUserInput" decision is coming from the page visibility...
====
 bool isActive;
  window->GetDocShell()->GetIsActive(&isActive);

  if (!isActive &&
      !nsContentUtils::IsChromeDoc(document)) {
    nsCOMPtr<nsIDOMRequestService> rs =
      do_GetService("@mozilla.org/dom/dom-request-service;1");
    rs->FireErrorAsync(static_cast<DOMRequest*>(this),
                       NS_LITERAL_STRING("NotUserInput"));

    nsCOMPtr<nsIConsoleService> console(
      do_GetService("@mozilla.org/consoleservice;1"));
    NS_ENSURE_TRUE(console, NS_OK);

    nsString message =
      NS_LITERAL_STRING("Can start activity from non user input or chrome code");
    console->LogStringMessage(message.get());

    return NS_OK;
  }
===

That means if I don't set the visibility correctly, some "background" page could still invoke web activity.

Anyway, instead of put a timer here in activity.c, how about let gaia could cancel the request via activity-choice mozChromeEvent event?

Fabrice, any thought?

Comment 8

[(inactive after 6/18) Alive Kuo [:alive]]

Attachment: Patch v1: Prevent too quick MozActivity Creation

Patch v1: Set a timer to prevent quick MozActivity instances.

Comment 9

[[:fabrice] Fabrice Desré]

Comment on attachment 786211 [details] [diff] [review]
Patch v1: Prevent too quick MozActivity Creation

Review of attachment 786211 [details] [diff] [review]:
-----------------------------------------------------------------

Activity.cpp lives in the child process, so two (or more) child processes could still compete and trigger several activities in less than 1 second.

So unless I misunderstood the intention and you're only interested in activities from a single process, you should look into ActivityService.jsm : this code runs as a singleton in the parent and could fire back errors in case of rapid succession of activities. I would rather not rely on the glue code (ie. gaia) for this kind of policy.

Comment 10

[BMO Automation]

Firefox OS is not being worked on