871374 - In process check is failed for all apps instead of only browser app.

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::System, defect)

Description

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

See https://bugzilla.mozilla.org/show_bug.cgi?id=856546#c24

STR:
1. Open an app(not browser app)
2. Call from another phone

Expected:
Except for browser app, all other app visibility shouldn't be turned off immediately.

Actual:
All apps becomes white immediate when call incomes.

Note:
the contentWindow check is true for not only browser app.
I'm not sure if this is a gecko bug.

An alternative workaround in gaia is move the browserManifestUrl in createFrame and check it instead of using contentWindow.


+++ This bug was initially created as a clone of Bug #845661 +++

1. Title : Alarm is not shown
2. Precondition : Set an alarm.
3. Tester's Action :
                   1) Open any page in browser 
                   2) click on the url bar and wait for the alarm to go on.
                   3) It is observed that only alarm sound is heard. the alarm UI is not visible

4. Detailed Symptom (ENG.) : It is observed that only alarm sound is heard. the alarm UI is not 

visible
5. Gaia Version : 6916e18d1f72936e892543cf4a104a7d457f78ad
6. Expected : Alarm UI should be shown.
7.Reproducibility: Y
1)Frequency Rate : 100%

Comment 2

[Justin Lebar (not reading bugmail)]

To make sure I understand how the bug's summary relates to the STR here: Gaia has code which checks whether an app is in-process or not (by checking whether contentWindow is non-null), and you observe that contentWindow is non-null for all apps, not just the browser?

Could you check whether the app in question is actually running OOP (via b2g-ps)?  If so, I'd be curious to know what the contentWindow you're getting represents; for example, can you access contentWindow.location?

Comment 3

[Leo]

There are many regressions caused by the patch of bug 845661.

The dup Bug 871413 is one of them and here is another one.

Title: White screen is displayed shortly after phonecall ends.
1. Call 1st phone from 2nd phone.
2. End the phonecall on 1st phone.
3. Observe white screen displayed shortly after dial app is closed.

And if I revert the following change in window_manager.js, it does not happen.

        case 'attentionscreenshow':
        var detail = evt.detail;
        if (detail && detail.origin && detail.origin != displayedApp) {
          attentionScreenTimer = setTimeout(function setVisibility() {
            if (inlineActivityFrames.length) {
              setVisibilityForInlineActivity(false);
            } else {
              setVisibilityForCurrentApp(false);
            }
          }, 3000);

          // Instantly blur the frame in order to ensure hiding the keyboard
          var app = runningApps[displayedApp];
          if (app) {
            if ('contentWindow' in app.iframe) {
              // Bug 845661 - Attention screen does not appears when
              // the url bar input is focused.
              // Calling app.iframe.blur() on an in-process window
              // seems to triggers heavy tasks that froze the main
              // process for a while and seems to expose a gecko
              // repaint issue.
              // So since the only in-process frame is the browser app
              // let's switch it's visibility as soon as possible when
              // there is an attention screen and delegate the
              // responsibility to blur the possible focused elements
              // itself.
              app.iframe.setVisible(false);
            } else {
              app.iframe.blur();
            }
          }
        }
        break;

Comment 4

[Leo]

Attachment: screenshot

Another regression by the same code above in my comment.

Fixed white card in Cardview
1. Open Clock app on 1st phone.
2. Send SMS from 2nd phone to 1st phone.
3. Bring down notification ared on 1st phone.
4. Select SMS to start SMS app.
5. Hold 'Home' button to bring up Cardview. (The cardview must start from the SMS app.)
6. Observe there is a white card in the center, which stays fixed and doesn't move.

Comment 5

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

(In reply to Justin Lebar [:jlebar] from comment #2)
> Could you check whether the app in question is actually running OOP (via
> b2g-ps)?  If so, I'd be curious to know what the contentWindow you're
> getting represents; for example, can you access contentWindow.location?

contentWindow.location = "app://browser.gaiamobile.org/index.html" for browser app(in process)
for other app, print contentWindow.location throws error.

Is this to say we should check 'document' in iframe.contentWindow instead of 'contentWindow' in iframe? (Confirmed this works.)

Comment 6

[Justin Lebar (not reading bugmail)]

> Is this to say we should check 'document' in iframe.contentWindow instead of 'contentWindow' in 
> iframe? (Confirmed this works.)

If it works, it works, but we should figure out why this behavior changed (assuming it did change).

Comment 7

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

(In reply to Justin Lebar [:jlebar] from comment #6)
> > Is this to say we should check 'document' in iframe.contentWindow instead of 'contentWindow' in 
> > iframe? (Confirmed this works.)
> 
> If it works, it works, but we should figure out why this behavior changed
> (assuming it did change).

I take a look into browserElement files but don't see we do something special to iframe.contentWindow there..
I wonder my solution works because the contentWindow is never removed from the mozbrowser iframe (that's why 'contentWindow' in iframe works) but only the attributes cannot be accessed because it's running in another process.

But I am not familiar enough with gecko so this is just a rough guess.

Comment 8

[Justin Lebar (not reading bugmail)]

> I take a look into browserElement files but don't see we do something special to 
> iframe.contentWindow there..

Right; it should be null automagically.

nsGenericHTMLFrameElement::GetContentWindow() calls nsFrameLoader::GetDocShell which calls MaybeCreateDocShell(), which calls ShouldUseRemoteProcess().  If ShouldUseRemoteProcess returns true, we set mRemoteFrame to true and don't create a docshell.  Therefore GetContentWindow() returns null.

Something is not going as I expect if you're seeing a non-null contentWindow.  It's quite odd...

Comment 9

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

(In reply to Justin Lebar [:jlebar] from comment #8)
> > I take a look into browserElement files but don't see we do something special to 
> > iframe.contentWindow there..
> 
> Right; it should be null automagically.
> 
> nsGenericHTMLFrameElement::GetContentWindow() calls
> nsFrameLoader::GetDocShell which calls MaybeCreateDocShell(), which calls
> ShouldUseRemoteProcess().  If ShouldUseRemoteProcess returns true, we set
> mRemoteFrame to true and don't create a docshell.  Therefore
> GetContentWindow() returns null.
> 
> Something is not going as I expect if you're seeing a non-null
> contentWindow.  It's quite odd...

OK I see.
The problem should be:
|iframe.contentWindow = null;| doesn't imply |!'contentWindow' in iframe| unless we |delete iframe.contentWindow| in gecko.

Comment 10

[Justin Lebar (not reading bugmail)]

> |iframe.contentWindow = null;| doesn't imply |!'contentWindow' in iframe| unless we |delete 
> iframe.contentWindow| in gecko.

Correct; I think the check should be |iframe.contentWindow !== null|.

Comment 11

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

Attachment: https://github.com/mozilla-b2g/gaia/pull/9710

Patch v1: fix the failed check to iframe.contentWindow.

Comment 12

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

master

https://github.com/mozilla-b2g/gaia/commit/ba2a59237f8b1ec180f70b9a4f9f833b878c34ab

Comment 14

[Leo]

(In reply to Alive Kuo [:alive] from comment #12)
> master
> 
> https://github.com/mozilla-b2g/gaia/commit/
> ba2a59237f8b1ec180f70b9a4f9f833b878c34ab

How about uplift?

Comment 15

[Leo]

(In reply to Alive Kuo [:alive] from comment #11)
> Created attachment 748676 [details]
> https://github.com/mozilla-b2g/gaia/pull/9710
> 
> Patch v1: fix the failed check to iframe.contentWindow.

This patch only fixed the dup bug (https://bugzilla.mozilla.org/show_bug.cgi?id=871413#c0).

The white screen issue when phonecall ends still exists. 
(https://bugzilla.mozilla.org/show_bug.cgi?id=871374#c3)

Also, the cardview issue also still exists.
(https://bugzilla.mozilla.org/show_bug.cgi?id=871374#c4)

Comment 16

[Leo]

To Alive, 

Should we create a new bug for the two other issues, which I mentioned above at Comment 15?

Comment 17

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

(In reply to Leo from comment #16)
> To Alive, 
> 
> Should we create a new bug for the two other issues, which I mentioned above
> at Comment 15?

Sorry I miss your comment.
Yes, "iff" the patch doesn't solve your bug.

Comment 18

[Leo]

(In reply to Alive Kuo [:alive] from comment #17)
> Sorry I miss your comment.
> Yes, "iff" the patch doesn't solve your bug.

Ok, I have created Bug 871919 and 871920. Thanks!

Comment 19

[John Ford [:jhford] CET/CEST Berlin Time]

Uplifted ba2a59237f8b1ec180f70b9a4f9f833b878c34ab to:
v1-train: 270adef58a01e30eee92fb8d7df95f46e69e7b6a
v1.0.1: e2187565da22df0c85b2eb501ced4efd342b7bc2

Comment 20

[Leo]

(In reply to Alive Kuo [:alive] from comment #11)
> Created attachment 748676 [details]
> https://github.com/mozilla-b2g/gaia/pull/9710
> 
> Patch v1: fix the failed check to iframe.contentWindow.

I tested with latest v1-train now and I still see the following problem, which is described in the dup Bug 871413.

1. Title : [System] White screen is displayed shortly on incoming call.
2. Precondition : Run any app (e.g. Clock) except homescreen, browser, dialer.
3. Tester's Action : Use 2nd phone to call 1st phone.
4. Detailed Symptom (ENG.) : On 1st phone, it displays white empty screen shortly before displaying the incoming call screen.
5. Expected : No white screen inbetween should be displayed.