Closed
Bug 1365643
Opened 7 years ago
Closed 7 years ago
about:newtab pages should run in content processes for activity-stream but not tiles
Categories
(Firefox :: Activity Streams: General, defect)
Firefox
Activity Streams: General
Tracking
()
RESOLVED
FIXED
Firefox 55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: dmosedale, Assigned: dmosedale)
References
Details
Attachments
(2 files)
Activity Stream is currently preffed-off, soon to be (gradually) preffed on. AS is e10s-capable, the old code (Tiles) is not. So until Tiles is phased out, about:newtab should return URI_MUST_LOAD_IN_CHILD when AS is preffed on, and should not return it when AS is preffed off. Patch forthcoming.
Comment hidden (mozreview-request) |
Comment 2•7 years ago
|
||
mozreview-review |
Comment on attachment 8868674 [details] Bug 1365643 - Temporarily back out Activity-Stream content-process move (c17e14e79d1a), https://reviewboard.mozilla.org/r/140282/#review144626
Attachment #8868674 -
Flags: review?(edilee) → review+
Pushed by dmosedale@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/c17e14e79d1a make activity stream, but not tiles, load in the child, r=Mardak
![]() |
||
Comment 4•7 years ago
|
||
Backed out for eslint failures in test_getURIFlags.js: https://hg.mozilla.org/integration/autoland/rev/563a164d255a85725b56a9cf8d453bec07370501 Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=c17e14e79d1adedbbd8297126b3af0076417ea69&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=100509338&repo=autoland [task 2017-05-19T18:03:58.788284Z] TEST-UNEXPECTED-ERROR | /home/worker/checkouts/gecko/browser/components/about/test/unit/test_getURIFlags.js:22:3 | 'ok' is not defined. (no-undef) [task 2017-05-19T18:03:58.788366Z] TEST-UNEXPECTED-ERROR | /home/worker/checkouts/gecko/browser/components/about/test/unit/test_getURIFlags.js:32:3 | 'ok' is not defined. (no-undef)
Flags: needinfo?(dmose)
Comment hidden (mozreview-request) |
Assignee | ||
Comment 6•7 years ago
|
||
A .eslint.js file was missing. I've added on, verified that it works, and repushed via autoland.
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(dmose)
Pushed by dmosedale@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/86b4df3b51cb make activity stream, but not tiles, load in the child, r=Mardak
Comment 9•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/86b4df3b51cb
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Assignee | ||
Comment 10•7 years ago
|
||
Re-opening. After discussion with :mconley, :gabor, :selena, and others, we're going to temporarily back this out until a solution for bug 1372664 is ready.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8868674 -
Flags: review+ → review?(edilee)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8868674 -
Flags: review?(edilee)
Assignee | ||
Updated•7 years ago
|
Attachment #8868674 -
Flags: review?(edilee)
Assignee | ||
Updated•7 years ago
|
Attachment #8868674 -
Flags: review?(edilee)
Assignee | ||
Comment 15•7 years ago
|
||
All the try tests that I ran passed; this is ready to land once it gets r+
Comment 16•7 years ago
|
||
mozreview-review |
Comment on attachment 8880523 [details] Bug 1365643 - temporarily disable preload patch until the rest of this bug relands, https://reviewboard.mozilla.org/r/151856/#review157228
Attachment #8880523 -
Flags: review?(edilee) → review+
Comment 17•7 years ago
|
||
Pushed by dmosedale@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/9377a56fe004 Temporarily back out Activity-Stream content-process move (c17e14e79d1a), r=Mardak https://hg.mozilla.org/integration/autoland/rev/c5e8b08a4635 temporarily disable preload patch until the rest of this bug relands, r=Mardak
Had to back out this backout for frequent failures like https://treeherder.mozilla.org/logviewer.html#?job_id=109655630&repo=autoland https://hg.mozilla.org/integration/autoland/rev/a6e97576ead7
Flags: needinfo?(dmose)
Assignee | ||
Comment 19•7 years ago
|
||
I'm a little confused about why this was backed out: looking at those failures on Treeherder, they are auto-matched to a known existing Treeherder failure: <https://bugzilla.mozilla.org/show_bug.cgi?id=1326337>. :KWierso, can you shed some light on this?
Flags: needinfo?(dmose) → needinfo?(wkocher)
Assignee | ||
Comment 20•7 years ago
|
||
OK, looking at the stack more closely, I see that although it's triggered from the same test, the crashes that triggered this backout are a bit different than the one in 1326337. However, the top of the C++ part of the stack looks very very similar to bug 1372597, both have mozilla::widget::JumpListBuilder::DoCommitListBuild as the first non-system code on the stack of the crashing thread. My original backout (that has now itself been backed out) had two pieces: the one that effected the code itself is conditionalized on activity-stream being turned on by default in mozilla-central, which it is not. It seems extremely unlikely that the code changes made by my original backout somehow caused the problem. However, my original backout also disables a test, and so it seems more plausible that disabling the test changes the test suite characteristics in a way that makes some underlying race condition (likely the same one documented in 1372597) easier to hit on Windows. It seems somewhat (not sure how much) likely that the race condition was introduced during the time the original patch was landed (5/22) and before I attempted the backout (6/23).
Assignee | ||
Comment 21•7 years ago
|
||
There were recent changes (on June 12th, in the time frame I speculated about above) in DoCommitListBuild from bug 1354143. :wcp, could the changes from that bug be causing the crashes here and in bug 1372597?
Flags: needinfo?(wpan)
Assignee | ||
Comment 22•7 years ago
|
||
:jimm, :wcp, I'm speculating that this crash is likely caused by the landing in bug 1354143. It's currently preventing us from doing the last things necessary to pref activity stream on in Nightly. Does looking at this stack give you any thoughts on whether my hypothesis is right? INFO - Crash dump filename: c:\users\cltbld\appdata\local\temp\tmpz4wbv7.mozrunner\minidumps\95626bc0-44e0-411d-9c22-c8d496c02f90.dmp 13:21:41 INFO - Operating system: Windows NT 13:21:41 INFO - 6.1.7601 Service Pack 1 13:21:41 INFO - CPU: x86 13:21:41 INFO - GenuineIntel family 6 model 62 stepping 4 13:21:41 INFO - 8 CPUs 13:21:41 INFO - 13:21:41 INFO - GPU: UNKNOWN 13:21:41 INFO - 13:21:41 INFO - Crash reason: EXCEPTION_ACCESS_VIOLATION_READ 13:21:41 INFO - Crash address: 0x0 13:21:41 INFO - Process uptime: 151 seconds 13:21:41 INFO - 13:21:41 INFO - Thread 68 (crashed) 13:21:41 INFO - 0 shell32.dll!SetLinkFlags + 0x15 13:21:41 INFO - eip = 0x76e1bfcf esp = 0x12fff2ac ebp = 0x12fff2b8 ebx = 0x00000000 13:21:41 INFO - esi = 0x00200000 edi = 0x00371de8 eax = 0x00000000 ecx = 0x00000000 13:21:41 INFO - edx = 0x3a32395f efl = 0x00010206 13:21:41 INFO - Found by: given as instruction pointer in context 13:21:41 INFO - 1 shell32.dll!CDestinationCategory::_WriteDests(IStream *) + 0x5f 13:21:41 INFO - eip = 0x76dfac70 esp = 0x12fff2c0 ebp = 0x12fff2e8 ebx = 0x46000000 13:21:41 INFO - Found by: call frame info 13:21:41 INFO - 2 shell32.dll!CDestinationCategory::_WriteCustomCategory(IStream *) + 0x22 13:21:41 INFO - eip = 0x76de846f esp = 0x12fff2f0 ebp = 0x12fff2f8 ebx = 0x2943a960 13:21:41 INFO - Found by: call frame info 13:21:41 INFO - 3 shell32.dll!CEnumFolderItemVerbs::Reset() + 0x17 13:21:41 INFO - eip = 0x76de8443 esp = 0x12fff300 ebp = 0x12fff310 13:21:41 INFO - Found by: call frame info 13:21:41 INFO - 4 shell32.dll!CDestinationList::_WriteListToFile(unsigned short const *) + 0xb3 13:21:41 INFO - eip = 0x76dfab54 esp = 0x12fff318 ebp = 0x12fff33c 13:21:41 INFO - Found by: previous frame's frame pointer 13:21:41 INFO - 5 shell32.dll!CDestinationList::_WriteList() + 0x3e 13:21:41 INFO - eip = 0x76dfaa4d esp = 0x12fff344 ebp = 0x12fff55c ebx = 0x003336e0 13:21:41 INFO - Found by: call frame info 13:21:41 INFO - 6 shell32.dll!CDestinationList::CommitList() + 0x77 13:21:41 INFO - eip = 0x76dfa995 esp = 0x12fff564 ebp = 0x12fff574 13:21:41 INFO - Found by: call frame info 13:21:41 INFO - 7 xul.dll!mozilla::widget::JumpListBuilder::DoCommitListBuild(RefPtr<mozilla::widget::detail::DoneCommitListBuildCallback>) [JumpListBuilder.cpp:c5e8b08a4635 : 494 + 0x9] 13:21:41 INFO - eip = 0x5d5bf093 esp = 0x12fff57c ebp = 0x12fff584 13:21:41 INFO - Found by: previous frame's frame pointer 13:21:41 INFO - 8 xul.dll!mozilla::detail::RunnableMethodArguments<RefPtr<mozilla::widget::detail::DoneCommitListBuildCallback> >::applyImpl<mozilla::widget::JumpListBuilder,void ( mozilla::widget::JumpListBuilder::*)(RefPtr<mozilla::widget::detail::DoneCommitListBuildCallback>),StoreRefPtrPassByPtr<mozilla::widget::detail::DoneCommitListBuildCallback>,0>(mozilla::widget::JumpListBuilder *,void ( mozilla::widget::JumpListBuilder::*)(RefPtr<mozilla::widget::detail::DoneCommitListBuildCallback>),mozilla::Tuple<StoreRefPtrPassByPtr<mozilla::widget::detail::DoneCommitListBuildCallback> > &,mozilla::IndexSequence<0>) [nsThreadUtils.h:c5e8b08a4635 : 1131 + 0x1d] 13:21:41 INFO - eip = 0x5d5be8db esp = 0x12fff58c ebp = 0x12fff590 13:21:41 INFO - Found by: call frame info 13:21:41 INFO - 9 xul.dll!mozilla::detail::RunnableMethodImpl<mozilla::widget::JumpListBuilder * const,void ( mozilla::widget::JumpListBuilder::*)(RefPtr<mozilla::widget::detail::DoneCommitListBuildCallback>),0,0,RefPtr<mozilla::widget::detail::DoneCommitListBuildCallback> >::Run() [nsThreadUtils.h:c5e8b08a4635 : 1180 + 0x15] 13:21:41 INFO - eip = 0x5d5bf6b7 esp = 0x12fff598 ebp = 0x12fff5ac 13:21:41 INFO - Found by: call frame info 13:21:41 INFO - 10 xul.dll!nsThread::ProcessNextEvent(bool,bool *) [nsThread.cpp:c5e8b08a4635 : 1421 + 0x6] 13:21:41 INFO - eip = 0x5c5f7243 esp = 0x12fff5b4 ebp = 0x12fff6b8 13:21:41 INFO - Found by: call frame info 13:21:41 INFO - 11 nss3.dll!_MD_CURRENT_THREAD [w95thred.c:c5e8b08a4635 : 317 + 0xc] 13:21:41 INFO - eip = 0x724cfa10 esp = 0x12fff5d0 ebp = 0x12fff6b8 13:21:41 INFO - Found by: stack scanning 13:21:41 INFO - 12 xul.dll!nsThreadManager::GetCurrentThread() [nsThreadManager.cpp:c5e8b08a4635 : 234 + 0x9] 13:21:41 INFO - eip = 0x5c5f4cb7 esp = 0x12fff5e0 ebp = 0x12fff6b8 13:21:41 INFO - Found by: call frame info 13:21:41 INFO - 13 xul.dll!nsThreadManager::GetCurrentThread(nsIThread * *) [nsThreadManager.cpp:c5e8b08a4635 : 337 + 0x6] 13:21:41 INFO - eip = 0x5c5f4d41 esp = 0x12fff5e8 ebp = 0x12fff6b8 13:21:41 INFO - Found by: call frame info 13:21:41 INFO - 14 xul.dll!NS_ProcessNextEvent(nsIThread *,bool) [nsThreadUtils.cpp:c5e8b08a4635 : 471 + 0xd] 13:21:41 INFO - eip = 0x5c5f69b8 esp = 0x12fff6c0 ebp = 0x12fff6d4 13:21:41 INFO - Found by: call frame info 13:21:41 INFO - 15 xul.dll!mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate *) [MessagePump.cpp:c5e8b08a4635 : 338 + 0x8] 13:21:41 INFO - eip = 0x5c88b1cc esp = 0x12fff6dc ebp = 0x12fff6fc 13:21:41 INFO - Found by: call frame info 13:21:41 INFO - 16 xul.dll!MessageLoop::RunHandler() [message_loop.cc:c5e8b08a4635 : 313 + 0x8] 13:21:41 INFO - eip = 0x5c875a97 esp = 0x12fff704 ebp = 0x12fff734 13:21:41 INFO - Found by: call frame info 13:21:41 INFO - 17 xul.dll!MessageLoop::Run() [message_loop.cc:c5e8b08a4635 : 293 + 0x7] 13:21:41 INFO - eip = 0x5c87584d esp = 0x12fff73c ebp = 0x12fff754 13:21:41 INFO - Found by: call frame info 13:21:41 INFO - 18 xul.dll!nsThread::ThreadFunc(void *) [nsThread.cpp:c5e8b08a4635 : 503 + 0x7] 13:21:41 INFO - eip = 0x5c5f94c8 esp = 0x12fff75c ebp = 0x12fff778 13:21:41 INFO - Found by: call frame info 13:21:41 INFO - 19 nss3.dll!_PR_NativeRunThread [pruthr.c:c5e8b08a4635 : 397 + 0x6] 13:21:41 INFO - eip = 0x724da6a8 esp = 0x12fff780 ebp = 0x12fff794 13:21:41 INFO - Found by: call frame info 13:21:41 INFO - 20 nss3.dll!pr_root [w95thred.c:c5e8b08a4635 : 95 + 0xa] 13:21:41 INFO - eip = 0x724cfcbe esp = 0x12fff79c ebp = 0x12fff7a0 13:21:41 INFO - Found by: call frame info 13:21:41 INFO - 21 ucrtbase.dll!_o__CIpow + 0x4f 13:21:41 INFO - eip = 0x7272d5ef esp = 0x12fff7a8 ebp = 0x12fff7dc 13:21:41 INFO - Found by: call frame info 13:21:41 INFO - 22 kernel32.dll!BaseThreadInitThunk + 0x12 13:21:41 INFO - eip = 0x76bc3c45 esp = 0x12fff7e4 ebp = 0x12fff7e8 13:21:41 INFO - Found by: call frame info 13:21:41 INFO - 23 mozglue.dll!patched_BaseThreadInitThunk [WindowsDllBlocklist.cpp:c5e8b08a4635 : 847 + 0x15] 13:21:41 INFO - eip = 0x7237265f esp = 0x12fff7f0 ebp = 0x12fff81c 13:21:41 INFO - Found by: call frame info 13:21:41 INFO - 24 ntdll.dll!__RtlUserThreadStart + 0x27 13:21:41 INFO - eip = 0x779b37f5 esp = 0x12fff824 ebp = 0x12fff85c 13:21:41 INFO - Found by: call frame info 13:21:41 INFO - 25 ntdll.dll!_RtlUserThreadStart + 0x1b 13:21:41 INFO - eip = 0x779b37c8 esp = 0x12fff864 ebp = 0x12fff874 13:21:41 INFO - Found by: call frame info
Flags: needinfo?(jmathies)
Tried on a Windows 10 machine but didn't reproduce this crash. Probably Windows 7 only? aCallback should holds a strong reference to the JumpListBuilder, and it passed those assertion, so I guess at least those pointers are valid during the call. Maybe ICDestinationList is not usable at the time, e.g. shutting down the whole application?
Flags: needinfo?(wpan)
Assignee | ||
Comment 24•7 years ago
|
||
After extremely helpful discussions with :gabor and :mconley, a new plan has emerged. We are no longer going to attempt to back this out at all, and will instead leave about:newtab in a content process. Thanks for all the help, y'all!
Status: REOPENED → RESOLVED
Closed: 7 years ago → 7 years ago
Flags: needinfo?(wkocher)
Flags: needinfo?(jmathies)
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•