619875 - Load about:home when having a custom home page that crashes in content.

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[ahoza]

Device: Nokia N900
BuildID: Mozilla/5.0 (Maemo; Linux armv7l;rv:2.0b8pre) Gecko/20101214
Firefox/4.08pre Fennec/4.0b3 

Steps to reproduce:

1. Open Fennec.
2. Set yahoo.com as home page.
3. Go to about:config and set plugin.disable=true
(From a more general point of view, steps 2 and 3 say the following: set as home page a page that sometimes/ most of the times crashes in content)
4. Restart browser.
5. When crash tab dialog is displayed, select "Close tab".

Expected results: 
As there is only one tab opened, when selecting "Close tab" the browser should close. But after restart, the tab will crash again and the browser will close again and this cycle can repeat indefinitely.
A better approach is the following: when custom home page crashes in content and there isn't any other tab opened, load about:home if one chooses : "Close tab" in crash dialog. Alternatively, reset home page to about:home

Actual results:
Browser tries to load the current home page, which is the one that crashes in content. So if I choose <n> times "Close tab" in crash dialog, the tab will crash for <n> times (infinite loop).

Comment 1

[Tony Chung [:tchung]]

adriana, any chance you can get a crash report?   can you try this on a previous nightly?

Comment 2

[ahoza]

(In reply to comment #1)
> adriana, any chance you can get a crash report?   can you try this on a
> previous nightly?

the tab crashes due to flash content. I get the same crash report as the one in bug 617703.

http://crash-stats.mozilla.com/report/index/f1bc6018-35b6-439d-87da-686e82101220

But this bug is not about the crash. It is about how the things are handled when having only one tab opened and that tab crashes. In this situation the browser tries to load current home page, if one chooses "Close tab" in crash dialog. But if the tab that crashed is the current home page, browser will try to load this page again. It will give another crash and so on.

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: patch

This patch uses the default (not user) homepage to fallback to when closing a crashed tab. It also changes Browser.getHomePage to return the user or default homepage.

Comment 4

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

Comment on attachment 499284 [details] [diff] [review]
patch

>diff --git a/chrome/content/browser.js b/chrome/content/browser.js
>-  getHomePage: function () {
>+  getHomePage: function (aOptions) {
>+    aOptions = aOptions || { useDefault: false };
>+
>     let url = "about:home";
>     try {
>-      url = Services.prefs.getComplexValue("browser.startup.homepage", Ci.nsIPrefLocalizedString).data;
>+      if (aOptions.useDefault) {
>+        let defaultPrefs = Services.prefs.getDefaultBranch(null);
>+        url = defaultPrefs.getComplexValue("browser.startup.homepage", Ci.nsIPrefLocalizedString).data;
>+      } else {
>+        url = Services.prefs.getComplexValue("browser.startup.homepage", Ci.nsIPrefLocalizedString).data;
>+      }
>     } catch (e) { }
> 
>     return url;
>   },

try {
  let prefs = aOptions.useDefault ? Services.prefs.getDefaultBranch(null) : Services.prefs;
  url = prefs.getComplexValue("browser.startup.homepage", Ci.nsIPrefLocalizedString).data;
}
catch(e) { }

 
>   /**
>    * Handle cert exception event bubbling up from content.
>    */
>   _handleCertException: function _handleCertException(aMessage) {
>     let json = aMessage.json;
>     if (json.action == "leave") {
>       // Get the start page from the *default* pref branch, not the user's
>-      let defaultPrefs = Services.prefs.getDefaultBranch(null);
>-      let url = "about:blank";
>-      try {
>-        url = defaultPrefs.getComplexValue("browser.startup.homepage", Ci.nsIPrefLocalizedString).data;
>-        // If url is a pipe-delimited set of pages, just take the first one.
>-        if (url.indexOf("|") != -1)
>-          url = url.split("|")[0];
>-      } catch (e) { /* Fall back on about blank */ }
>-
>+      let url = Browser.getHomePage({ useDefault: true });

Why do we not support "|" anymore?

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to comment #4)
> Comment on attachment 499284 [details] [diff] [review]
> patch

> try {
>   let prefs = aOptions.useDefault ? Services.prefs.getDefaultBranch(null) :
> Services.prefs;
>   url = prefs.getComplexValue("browser.startup.homepage",
> Ci.nsIPrefLocalizedString).data;
> }
> catch(e) { }

Nice. I'll update

> >-        // If url is a pipe-delimited set of pages, just take the first one.
> >-        if (url.indexOf("|") != -1)
> >-          url = url.split("|")[0];
> >-      } catch (e) { /* Fall back on about blank */ }
> >-
> >+      let url = Browser.getHomePage({ useDefault: true });
> 
> Why do we not support "|" anymore?

Fennec never really did. Browser.getHomePage never supported it.

Comment 6

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

Comment on attachment 499284 [details] [diff] [review]
patch

r+ with previous comments addressed

Comment 7

[Mark Finkle (:mfinkle) (use needinfo?)]

pushed:
http://hg.mozilla.org/mobile-browser/rev/d7f5ac2f69c1

Comment 8

[Martijn Wargers (dead)]

I followed the steps to reproduce.
Step 3 has to be plugin.disable=false instead of true, btw.

I don't think I get the expected results, so I'm reopening this bug for now, until someone can explain me this (after which this bug can be closed and verified and/or a new bug can be file about what I'm seeing)

My understanding of what should be happening, e.g. expected result:
- After tapping "Close tab" in the crash dialog, about:home should have been loaded. (that's what I understand from comment 3)

Actual result:
- After tapping "Close tab" in the crash dialog, an blank page appears with the title "Yahoo!" in there (but no url beneath it).

Can someone answer if my expected result is the actual expected result?

I tested this, using:
Mozilla/5.0 (Maemo; Linux armv7l; rv:2.0b9pre) Gecko/20101229 Firefox/4.0b9pre Fennec/4.0b4pre ID:20101229011248
On Android, I get redirected to the mobile version of Yahoo, which doesn't appear to have Flash. The Droid doesn't seem to crash easily with Flash, so it's more difficult to test, anyhow

Comment 9

[Mark Finkle (:mfinkle) (use needinfo?)]

This bug is not really about Flash. It's about crashing on your custom startpage.

Previously, if you "Close tab" in the crash dialog, we would try to open your custom startpage again - the same one that crashed.

Now, we should be trying to open "about:home" if you choose "Close tab"

I tested this by actually killing the child process and making sure the about:home was loaded if I picked "Close tab"

Comment 10

[Mark Finkle (:mfinkle) (use needinfo?)]

Setting plugin.disable=false and crashing in Flash may not be caught correctly at all. I did not test using a bad Flash site.

Comment 11

[Martijn Wargers (dead)]

(In reply to comment #9)
> I tested this by actually killing the child process and making sure the
> about:home was loaded if I picked "Close tab"

I tested this on Maemo again by killing the child process. I get the same result.
What I did:
- I set http://nu.nl as the custom home page
- I opened up the terminal, did "sudo gainroot", then "ps" to view the list of processes.
- I then used "kill ****" to kill the PID of the plugin-container process.
- I got the crash dialog and tapped on "Close tab"

I got the same result again as mentioned in comment 8. I get a blank page, but with the old page title still in there of the tab that was crashing.

I don't know how to kill the plugin-container process on Android.

Comment 12

[Martijn Wargers (dead)]

Ok, I filed bug 623251 for the issue that I'm seeing. It would be too confusing to continue here, since a patch that supposedly fixed this, is already checked in.
I'll just close this bug again as resolved->fixed, but I can't verify this bug at this point.

Comment 13

[ahoza]

Verified as fixed with buildId: Mozilla /5.0 (Maemo;Linux armv7l;rv:2.0b9pre)Gecko/20110106 Firefox/4.0b9pre Fennec /4.0b4pre using Nokia N900