Closed Bug 1381359 Opened 2 years ago Closed 2 years ago

Crash in java.lang.IllegalStateException: Tried changing pinned state before it''s determined. at org.mozilla.gecko.home.HomeFragment$ToggleASPinTask.<init>(HomeFragment.java)

Categories

(Firefox for Android :: General, defect, P1, critical)

ARM
Android
defect

Tracking

()

RESOLVED FIXED
Firefox 57
Tracking Status
fennec + ---
firefox55 --- unaffected
firefox56 + verified
firefox57 --- fixed

People

(Reporter: njn, Assigned: Grisha)

References

Details

(Keywords: crash, topcrash, Whiteboard: [MobileAS])

Crash Data

Attachments

(2 files)

This bug was filed from the Socorro interface and is 
report bp-8800fa74-6043-4ad0-aa0d-1fbf70170716.
=============================================================

This is the Fennec #1 topcrash in Nightly 20170714131541.

Here's the top part of the stack trace:

> java.lang.IllegalStateException: Tried changing pinned state before it's determined.
> at org.mozilla.gecko.home.HomeFragment$ToggleASPinTask.<init>(HomeFragment.java:461)
> at org.mozilla.gecko.home.HomeFragment.onContextItemSelected(HomeFragment.java:367)
> at org.mozilla.gecko.home.TopSitesPanel.onContextItemSelected(TopSitesPanel.java:369)
> at android.support.v4.app.Fragment.performContextItemSelected(Unknown Source)
> at android.support.v4.app.FragmentManagerImpl.dispatchContextItemSelected(Unknown Source)
> at android.support.v4.app.FragmentController.dispatchContextItemSelected(Unknown Source)
> at android.support.v4.app.FragmentActivity.onMenuItemSelected(Unknown Source)
> at android.support.v7.app.AppCompatActivity.onMenuItemSelected(Unknown Source)
> at android.support.v7.view.WindowCallbackWrapper.onMenuItemSelected(Unknown Source)
> at com.android.internal.policy.impl.PhoneWindow$DialogMenuCallback.onMenuItemSelected(PhoneWindow.java:4770)
> at com.android.internal.view.menu.MenuBuilder.dispatchMenuItemSelected(MenuBuilder.java:761)
> at com.android.internal.view.menu.MenuItemImpl.invoke(MenuItemImpl.java:152)
> at com.android.internal.view.menu.MenuBuilder.performItemAction(MenuBuilder.java:904)
> at com.android.internal.view.menu.MenuBuilder.performItemAction(MenuBuilder.java:894)
> at com.android.internal.view.menu.MenuDialogHelper.onClick(MenuDialogHelper.java:180)
> at com.android.internal.app.AlertController$AlertParams$3.onItemClick(AlertController.java:1106)
> at android.widget.AdapterView.performItemClick(AdapterView.java:305)
> at android.widget.AbsListView.performItemClick(AbsListView.java:1166)
> at android.widget.AbsListView$PerformClick.run(AbsListView.java:3073)
> at android.widget.AbsListView.onTouchUp(AbsListView.java:3907)
> at android.widget.AbsListView.onTouchEvent(AbsListView.java:3666)
> at android.view.View.dispatchTouchEvent(View.java:8744)

First occurrence is in the July 14 Nightly, which suggests this regression range:

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=30ea2905130e85f9e1d8d56fa3097901eec6514b&tochange=400584289c8fa44060d6443a3288ef57cf73ff4e

Bug 1377286 is probably the cause. Grisha, can you please investigate?
Flags: needinfo?(gkruglov)
Depends on: 1377286
tracking-fennec: --- → ?
Hardware: Unspecified → ARM
Version: unspecified → Trunk
Grisha, I was going to add "[MobileAS]" in whiteboard but then think it's good to ask in advance. Is it truly related to Bug 1377286? If not please let us know since it's FE related. Thanks.
Assignee: nobody → gkruglov
Status: NEW → ASSIGNED
Flags: needinfo?(gkruglov)
Priority: -- → P1
Whiteboard: [MobileAS]
On Nightly 56 there are 2 "Pin site" options in the Top sites context menu. The second one, located under the Remove option, triggers this crash.

build: Nightly 56.0a1 (2017-07-19).
(In reply to Oana Horvath from comment #2)
> On Nightly 56 there are 2 "Pin site" options in the Top sites context menu.
> The second one, located under the Remove option, triggers this crash.
> 
> build: Nightly 56.0a1 (2017-07-19).

note: Need to mention that it's on the old Top sites panel, with Activity stream off.
[traige@0726] +, already taken.
tracking-fennec: ? → +
This signature is ranked #4 in top-crashers.
Keywords: topcrash
Activity Stream is enabled by default for 57, without ability to disable it to access classic top sites. So a fix here would be a direct uplift.
Blocks: 1377286
No longer depends on: 1377286
Approval Request Comment
[Feature/Bug causing the regression]: Bug 1377286

[User impact if declined]: Users will see a duplicate "Pin Site" item on the Top Sites context menu. If they interact with it, their browser will crash.

[Is this code covered by automated tests?]: No.

[Has the fix been verified in Nightly?]: This patch is a no-op on nightly, since the classic Top Sites are disabled there.

[Needs manual test from QE? If yes, steps to reproduce]: In beta, open regular (non-Activity Stream) top sites. Long press on any item in the top sites grid. You should only see one Pin Site item, and pressing that item should work.

[List of other uplifts needed for the feature/fix]: N/A

[Is the change risky?]: No

[Why is the change risky/not risky?]: Very simple change, simply turning off a menu item for context menu on one panel.

[String changes made/needed]: N/A
Attachment #8896450 - Flags: approval-mozilla-beta?
Attachment #8896448 - Flags: review?(s.kaspari) → review?(liuche)
Comment on attachment 8896448 [details]
Bug 1381359 - Hide AS Pin Site item from classic Top Sites context menu

https://reviewboard.mozilla.org/r/167698/#review172942

lgtm, disabling activity stream pin in non-AS panel. home_as_pin is only present in home_contextmenu and TopSitesPanel is the only non-AS file to use that context menu, so there shouldn't anywhere else to disable it.
Attachment #8896448 - Flags: review?(liuche) → review+
Pushed by gkruglov@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a19e930e2793
Hide AS Pin Site item from classic Top Sites context menu r=liuche
https://hg.mozilla.org/mozilla-central/rev/a19e930e2793
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
[Tracking Requested - why for this release]:
Comment on attachment 8896450 [details] [diff] [review]
hide-as-pin-top-sites-beta.patch

Fix a top crash. Beta56+. Should be in 56.0b3.
Attachment #8896450 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Track 56+ as top crash.
Verified as fixed on the latest Beta build (56.0b3).
Tested on a Samsung Galaxy S8 (Android 7.0) and on a Nexus 6P (Android 8.0).
You need to log in before you can comment on or make changes to this bug.