Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
319633007:
Move WebScreenOrientationClient to WebFrameClient. (Closed)
|
Created:
6 years, 6 months ago by mlamouri (slow - plz ping) Modified:
6 years, 6 months ago CC:
blink-reviews, jamesr, dglazkov+blink, abarth-chromium, oilpan-reviews Base URL:
https://chromium.googlesource.com/chromium/blink.git@master Visibility:
Public. |
DescriptionMove WebScreenOrientationClient to WebFrameClient.
Requested by jam@, because of WebViewClient being doomed to a painful death.
BUG=162827
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=175923
Patch Set 1 #
Total comments: 2
Patch Set 2 : rebase #
Total comments: 1
Patch Set 3 : fix oilpan build #
Total comments: 6
Patch Set 4 : on top of cl 323873007 #Patch Set 5 : don't send the events multiple times #
Total comments: 2
Patch Set 6 : magic #Patch Set 7 : update comment #Patch Set 8 : m_client null check #Messages
Total messages: 36 (0 generated)
https://codereview.chromium.org/319633007/diff/1/Source/modules/screen_orient... File Source/modules/screen_orientation/ScreenOrientationController.cpp (left): https://codereview.chromium.org/319633007/diff/1/Source/modules/screen_orient... Source/modules/screen_orientation/ScreenOrientationController.cpp:80: LocalFrame* mainFrame = page()->mainFrame(); Why don't we keep doing this? Why do we keep the LocalFrame as a member now?
https://codereview.chromium.org/319633007/diff/1/Source/modules/screen_orient... File Source/modules/screen_orientation/ScreenOrientationController.cpp (left): https://codereview.chromium.org/319633007/diff/1/Source/modules/screen_orient... Source/modules/screen_orientation/ScreenOrientationController.cpp:80: LocalFrame* mainFrame = page()->mainFrame(); On 2014/06/09 17:29:29, Chris Dumez wrote: > Why don't we keep doing this? Why do we keep the LocalFrame as a member now? Because page->mainFrame() might not be the same as m_frame. Actually, we might want to no longer propagate the event to the child frame given that every frame might end up getting |pageVisibilyChanged| called.
+oilpan-reviews https://codereview.chromium.org/319633007/diff/20001/Source/modules/screen_or... File Source/modules/screen_orientation/ScreenOrientationController.h (right): https://codereview.chromium.org/319633007/diff/20001/Source/modules/screen_or... Source/modules/screen_orientation/ScreenOrientationController.h:22: class ScreenOrientationController FINAL : public NoBaseWillBeGarbageCollectedFinalized<ScreenOrientationController>, public WillBeHeapSupplement<LocalFrame>, public PageLifecycleObserver { LocalFrame isn't using oilpan, so you'll have to undo the oilpan related changes to these files
I've updated the CL to fix oilpan build. PTAL.
https://codereview.chromium.org/319633007/diff/40001/Source/modules/screen_or... File Source/modules/screen_orientation/ScreenOrientationController.cpp (right): https://codereview.chromium.org/319633007/diff/40001/Source/modules/screen_or... Source/modules/screen_orientation/ScreenOrientationController.cpp:28: Supplement<LocalFrame>::provideTo(frame, supplementName(), adoptPtr(controller)); This is not going to work, OwnPtr<>'ing something that's controlled by the garbage collector. I think you need to pull ScreenOrientationController off the heap (i.e., drop NoBaseWillBeGarbageCollectedFinalized<>). https://codereview.chromium.org/319633007/diff/40001/Source/modules/screen_or... File Source/modules/screen_orientation/ScreenOrientationController.h (right): https://codereview.chromium.org/319633007/diff/40001/Source/modules/screen_or... Source/modules/screen_orientation/ScreenOrientationController.h:37: explicit ScreenOrientationController(LocalFrame&, blink::WebScreenOrientationClient*); nit: remove explicit.
CL https://codereview.chromium.org/323873007/ makes LocalFrame HeapSupplementable again using a persistent/GC-root. This means that all of the supplements are on-heap GC managed objects. My inlined comments should be the necessary changes to make this CL compatible with the change to LocalFrame. https://codereview.chromium.org/319633007/diff/40001/Source/modules/screen_or... File Source/modules/screen_orientation/ScreenOrientationController.cpp (right): https://codereview.chromium.org/319633007/diff/40001/Source/modules/screen_or... Source/modules/screen_orientation/ScreenOrientationController.cpp:28: Supplement<LocalFrame>::provideTo(frame, supplementName(), adoptPtr(controller)); On 2014/06/09 21:18:58, sof wrote: > This is not going to work, OwnPtr<>'ing something that's controlled by the > garbage collector. > > I think you need to pull ScreenOrientationController off the heap (i.e., drop > NoBaseWillBeGarbageCollectedFinalized<>). Now that LocalFrame supplements are on-heap, this should be: WillBeHeapSupplement<LocalFrame>::provideTo(frame, supplementName(), adoptPtrWillBeNoop(controller)); https://codereview.chromium.org/319633007/diff/40001/Source/modules/screen_or... Source/modules/screen_orientation/ScreenOrientationController.cpp:33: return *static_cast<ScreenOrientationController*>(Supplement<LocalFrame>::from(frame, supplementName())); WillBeHeapSupplement<LocalFrame> https://codereview.chromium.org/319633007/diff/40001/Source/modules/screen_or... File Source/modules/screen_orientation/ScreenOrientationController.h (left): https://codereview.chromium.org/319633007/diff/40001/Source/modules/screen_or... Source/modules/screen_orientation/ScreenOrientationController.h:24: WILL_BE_USING_GARBAGE_COLLECTED_MIXIN(ScreenOrientationController); revert https://codereview.chromium.org/319633007/diff/40001/Source/modules/screen_or... File Source/modules/screen_orientation/ScreenOrientationController.h (right): https://codereview.chromium.org/319633007/diff/40001/Source/modules/screen_or... Source/modules/screen_orientation/ScreenOrientationController.h:22: class ScreenOrientationController FINAL : public NoBaseWillBeGarbageCollectedFinalized<ScreenOrientationController>, public Supplement<LocalFrame>, public PageLifecycleObserver { WillBeHeapSupplement<LocalFrame>
Ok, so basically with https://codereview.chromium.org/323873007/ Supplement<LocalFrame> will be oilpan-compatible. I've reverted the changes from rev 3.
Chris, I've fixed the issue you pointed regarding the events. And added a test for it. PTAL.
lgtm with nit. https://codereview.chromium.org/319633007/diff/80001/Source/modules/screen_or... File Source/modules/screen_orientation/ScreenOrientationController.cpp (right): https://codereview.chromium.org/319633007/diff/80001/Source/modules/screen_or... Source/modules/screen_orientation/ScreenOrientationController.cpp:86: if (m_frame.localFrameRoot() == &m_frame && oldOrientation != orientation()) nit: this '&' should not be needed due to DEFINE_COMPARISON_OPERATORS_WITH_REFERENCES_REFCOUNTED() magic I think.
https://codereview.chromium.org/319633007/diff/80001/Source/modules/screen_or... File Source/modules/screen_orientation/ScreenOrientationController.cpp (right): https://codereview.chromium.org/319633007/diff/80001/Source/modules/screen_or... Source/modules/screen_orientation/ScreenOrientationController.cpp:86: if (m_frame.localFrameRoot() == &m_frame && oldOrientation != orientation()) On 2014/06/10 14:48:48, Chris Dumez wrote: > nit: this '&' should not be needed due to > DEFINE_COMPARISON_OPERATORS_WITH_REFERENCES_REFCOUNTED() magic I think. This is working. The name of that macro can't be more true :)
The comment in https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit... refers to WebView, but that's not correct any more - can you update that comment as well? Can you request a lock from any frame? If so lgtm
On 2014/06/10 13:43:22, Mounir Lamouri wrote: > Ok, so basically with https://codereview.chromium.org/323873007/ > Supplement<LocalFrame> will be oilpan-compatible. I've reverted the changes from > rev 3. Yes. All LocalFrame supplements need to be on-heap for the oilpan build, so for the time being the transition type should be WillBeHeapSupplement<LocalFrame> (which is Supplement<LocalFrame> in the non-oilpan build and HeapSupplement<LocalFrame> in the oilpan build). Oilpan related changes lgtm.
On 2014/06/10 18:07:14, jamesr wrote: > The comment in > https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit... > refers to WebView, but that's not correct any more - can you update that comment > as well? Done. > Can you request a lock from any frame? If so lgtm Yes. Unless they are being blocked by sandboxing rules (HTML sandboxed attribute) but this is handled before the client is being called. Thanks for the quick reviews! :)
The CQ bit was checked by mlamouri@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mlamouri@chromium.org/319633007/120001
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: linux_blink_dbg on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/1...) linux_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/1...) mac_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/11005) win_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/11481)
The CQ bit was checked by mlamouri@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mlamouri@chromium.org/319633007/140001
Message was sent while issue was closed.
Change committed as 175923
Message was sent while issue was closed.
LGTM in terms of oilpan.
Message was sent while issue was closed.
seeing some new flaky oilpan crashers for the past 12 hours, http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40... any chance they're due to this? (Missing view() null check in ScreenOrientationController::orientation(), perhaps?)
Message was sent while issue was closed.
On 2014/06/11 16:05:39, sof wrote: > seeing some new flaky oilpan crashers for the past 12 hours, > > > http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40... > > any chance they're due to this? (Missing view() null check in > ScreenOrientationController::orientation(), perhaps?) Thanks for looking into this. As far as I see the stack trace, it seems that m_frame is gone. Thread 0 (crashed) 0 content_shell!google_breakpad::ExceptionHandler::SimulateSignalDelivery(int) [exception_handler.cc : 440 + 0x0] rax = 0x000000000abe2040 rdx = 0x0000000000000007 rcx = 0x00000fffee031000 rbx = 0x00000fffee03104c rsi = 0x0000000000000000 rdi = 0x00007fff70188320 rbp = 0x00007fff701887b0 rsp = 0x00007fff70188260 r8 = 0x0000000000000080 r9 = 0x0101010101010101 r10 = 0x0000000000547057 r11 = 0x000000000abe6590 r12 = 0x00007fff70188320 r13 = 0x00007fff70188260 r14 = 0x000060c00008a1c0 r15 = 0x00007fff70188280 rip = 0x00000000052b8b80 Found by: given as instruction pointer in context 1 content_shell!~ScopedInErrorReport [asan_report.cc : 570 + 0x9] rbx = 0x00007fff701890c0 rbp = 0x000000000784fc1e rsp = 0x00007fff701887c0 r12 = 0x0000000007850ad4 r13 = 0x00007fff701890c0 r14 = 0x0000000007850ae1 r15 = 0x0000000007847ed0 rip = 0x0000000000537f38 Found by: call frame info 2 content_shell!__asan_report_error [asan_report.cc : 891 + 0x5] rbx = 0x00007fff701890c0 rbp = 0x000000000784fc1e rsp = 0x00007fff701887d0 r12 = 0x0000000007850ad4 r13 = 0x00007fff701890c0 r14 = 0x0000000007850ae1 r15 = 0x0000000007847ed0 rip = 0x0000000000537ab1 Found by: call frame info 3 content_shell!__asan_report_load8 [asan_rtl.cc : 369 + 0x1c] rbx = 0x00007fd7600d2510 rbp = 0x00007fff70189120 rsp = 0x00007fff70189110 r12 = 0x000060700009b630 r13 = 0x00007fff70189240 r14 = 0x00000ffaec01a4a2 r15 = 0x00007fd7600d2504 rip = 0x00000000005386b7 Found by: call frame info 4 content_shell!WebCore::LocalFrame::view() const [RefPtr.h : 56 + 0x5] rbx = 0x00007fd7600d2510 rbp = 0x00007fff70189130 rsp = 0x00007fff70189130 r12 = 0x000060700009b630 r13 = 0x00007fff70189240 r14 = 0x00000ffaec01a4a2 r15 = 0x00007fd7600d2504 rip = 0x0000000001ff22d5 Found by: call frame info 5 content_shell!WebCore::ScreenOrientationController::orientation() const [ScreenOrientationController.cpp : 104 + 0x1d] rbx = 0x00007fd7600d2510 rbp = 0x00007fff70189150 rsp = 0x00007fff70189140 r12 = 0x000060700009b630 r13 = 0x00007fff70189240 r14 = 0x00000ffaec01a4a2 r15 = 0x00007fd7600d2504 rip = 0x0000000003b8c341 Found by: call frame info ^^^ The crash is happening when ScreenOrientationController accessed m_frame.view(), not when ScreenOrientationController used the view() value somewhere.
Message was sent while issue was closed.
On 2014/06/11 16:12:44, haraken wrote: > ^^^ The crash is happening when ScreenOrientationController accessed > m_frame.view(), not when ScreenOrientationController used the view() value > somewhere. Hmm, maybe m_frame should be RefPtr<LocalFrame> instead?
Message was sent while issue was closed.
On 2014/06/11 16:27:45, Mounir Lamouri wrote: > On 2014/06/11 16:12:44, haraken wrote: > > ^^^ The crash is happening when ScreenOrientationController accessed > > m_frame.view(), not when ScreenOrientationController used the view() value > > somewhere. > > Hmm, maybe m_frame should be RefPtr<LocalFrame> instead? I suspect the reason why this is only manifesting itself with Oilpan is the use of a Persistent supplementable for LocalFrame (https://codereview.chromium.org/323873007/), which will prolong the lifetime of the supplements until the next Oilpan GC strikes. i.e., the Supplementable object is not finalized at the same time as the Supplements. Hence, when the m_frame is reset to 0 as part of frame detachment in WebLocalFrameImpl::setFrame(), the LocalFrame is deref'ed and destructed. But as far as the (heap based) supplements go, only the Persistent to them is destroyed, not the heap objects it refers to. Like ScreenOrientationController. That's my current theory - I don't quite understand what brings about the subsequent access to the supplement though.
Message was sent while issue was closed.
On 2014/06/11 20:37:56, sof wrote: > On 2014/06/11 16:27:45, Mounir Lamouri wrote: > > On 2014/06/11 16:12:44, haraken wrote: > > > ^^^ The crash is happening when ScreenOrientationController accessed > > > m_frame.view(), not when ScreenOrientationController used the view() value > > > somewhere. > > > > Hmm, maybe m_frame should be RefPtr<LocalFrame> instead? > > I suspect the reason why this is only manifesting itself with Oilpan is the use > of a Persistent supplementable for LocalFrame > (https://codereview.chromium.org/323873007/), which will prolong the lifetime of > the supplements until the next Oilpan GC strikes. i.e., the Supplementable > object is not finalized at the same time as the Supplements. > > Hence, when the m_frame is reset to 0 as part of frame detachment in > WebLocalFrameImpl::setFrame(), the LocalFrame is deref'ed and destructed. But as > far as the (heap based) supplements go, only the Persistent to them is > destroyed, not the heap objects it refers to. Like ScreenOrientationController. > > That's my current theory - I don't quite understand what brings about the > subsequent access to the supplement though. I agree with your theory. It's nasty that supplementing objects don't die with the supplementable object (except for an intentional case). hmm, that's a tough issue. The only solution I can come up with is to move the LocalFrame to the heap, but I understand that's a substantial amount of work due to the prompt finalization issue...
Message was sent while issue was closed.
On 2014/06/12 01:02:29, haraken wrote: > On 2014/06/11 20:37:56, sof wrote: > > On 2014/06/11 16:27:45, Mounir Lamouri wrote: > > > On 2014/06/11 16:12:44, haraken wrote: > > > > ^^^ The crash is happening when ScreenOrientationController accessed > > > > m_frame.view(), not when ScreenOrientationController used the view() value > > > > somewhere. > > > > > > Hmm, maybe m_frame should be RefPtr<LocalFrame> instead? > > > > I suspect the reason why this is only manifesting itself with Oilpan is the > use > > of a Persistent supplementable for LocalFrame > > (https://codereview.chromium.org/323873007/), which will prolong the lifetime > of > > the supplements until the next Oilpan GC strikes. i.e., the Supplementable > > object is not finalized at the same time as the Supplements. > > > > Hence, when the m_frame is reset to 0 as part of frame detachment in > > WebLocalFrameImpl::setFrame(), the LocalFrame is deref'ed and destructed. But > as > > far as the (heap based) supplements go, only the Persistent to them is > > destroyed, not the heap objects it refers to. Like > ScreenOrientationController. > > > > That's my current theory - I don't quite understand what brings about the > > subsequent access to the supplement though. > > I agree with your theory. > > It's nasty that supplementing objects don't die with the supplementable object > (except for an intentional case). > Not unexpected behavior of Persistents. > hmm, that's a tough issue. The only solution I can come up with is to move the > LocalFrame to the heap, but I understand that's a substantial amount of work due > to the prompt finalization issue... What's the motivation for doing this Page -> LocalFrame shift?
Message was sent while issue was closed.
On 2014/06/12 05:44:33, sof wrote: > On 2014/06/12 01:02:29, haraken wrote: > > On 2014/06/11 20:37:56, sof wrote: > > > On 2014/06/11 16:27:45, Mounir Lamouri wrote: > > > > On 2014/06/11 16:12:44, haraken wrote: > > > > > ^^^ The crash is happening when ScreenOrientationController accessed > > > > > m_frame.view(), not when ScreenOrientationController used the view() > value > > > > > somewhere. > > > > > > > > Hmm, maybe m_frame should be RefPtr<LocalFrame> instead? > > > > > > I suspect the reason why this is only manifesting itself with Oilpan is the > > use > > > of a Persistent supplementable for LocalFrame > > > (https://codereview.chromium.org/323873007/), which will prolong the > lifetime > > of > > > the supplements until the next Oilpan GC strikes. i.e., the Supplementable > > > object is not finalized at the same time as the Supplements. > > > > > > Hence, when the m_frame is reset to 0 as part of frame detachment in > > > WebLocalFrameImpl::setFrame(), the LocalFrame is deref'ed and destructed. > But > > as > > > far as the (heap based) supplements go, only the Persistent to them is > > > destroyed, not the heap objects it refers to. Like > > ScreenOrientationController. > > > > > > That's my current theory - I don't quite understand what brings about the > > > subsequent access to the supplement though. > > > > I agree with your theory. > > > > It's nasty that supplementing objects don't die with the supplementable object > > (except for an intentional case). > > > > Not unexpected behavior of Persistents. > > > hmm, that's a tough issue. The only solution I can come up with is to move the > > LocalFrame to the heap, but I understand that's a substantial amount of work > due > > to the prompt finalization issue... > > What's the motivation for doing this Page -> LocalFrame shift? It's for site-isolation. When the site isolation is done, Page will be gone. jam@ would know better.
Message was sent while issue was closed.
On 2014/06/11 16:12:44, haraken wrote: > On 2014/06/11 16:05:39, sof wrote: > > seeing some new flaky oilpan crashers for the past 12 hours, > > > > > > > http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40... > > > > any chance they're due to this? (Missing view() null check in > > ScreenOrientationController::orientation(), perhaps?) > > Thanks for looking into this. As far as I see the stack trace, it seems that > m_frame is gone. > > Thread 0 (crashed) > 0 content_shell!google_breakpad::ExceptionHandler::SimulateSignalDelivery(int) > [exception_handler.cc : 440 + 0x0] > rax = 0x000000000abe2040 rdx = 0x0000000000000007 > rcx = 0x00000fffee031000 rbx = 0x00000fffee03104c > rsi = 0x0000000000000000 rdi = 0x00007fff70188320 > rbp = 0x00007fff701887b0 rsp = 0x00007fff70188260 > r8 = 0x0000000000000080 r9 = 0x0101010101010101 > r10 = 0x0000000000547057 r11 = 0x000000000abe6590 > r12 = 0x00007fff70188320 r13 = 0x00007fff70188260 > r14 = 0x000060c00008a1c0 r15 = 0x00007fff70188280 > rip = 0x00000000052b8b80 > Found by: given as instruction pointer in context > 1 content_shell!~ScopedInErrorReport [asan_report.cc : 570 + 0x9] > rbx = 0x00007fff701890c0 rbp = 0x000000000784fc1e > rsp = 0x00007fff701887c0 r12 = 0x0000000007850ad4 > r13 = 0x00007fff701890c0 r14 = 0x0000000007850ae1 > r15 = 0x0000000007847ed0 rip = 0x0000000000537f38 > Found by: call frame info > 2 content_shell!__asan_report_error [asan_report.cc : 891 + 0x5] > rbx = 0x00007fff701890c0 rbp = 0x000000000784fc1e > rsp = 0x00007fff701887d0 r12 = 0x0000000007850ad4 > r13 = 0x00007fff701890c0 r14 = 0x0000000007850ae1 > r15 = 0x0000000007847ed0 rip = 0x0000000000537ab1 > Found by: call frame info > 3 content_shell!__asan_report_load8 [asan_rtl.cc : 369 + 0x1c] > rbx = 0x00007fd7600d2510 rbp = 0x00007fff70189120 > rsp = 0x00007fff70189110 r12 = 0x000060700009b630 > r13 = 0x00007fff70189240 r14 = 0x00000ffaec01a4a2 > r15 = 0x00007fd7600d2504 rip = 0x00000000005386b7 > Found by: call frame info > 4 content_shell!WebCore::LocalFrame::view() const [RefPtr.h : 56 + 0x5] > rbx = 0x00007fd7600d2510 rbp = 0x00007fff70189130 > rsp = 0x00007fff70189130 r12 = 0x000060700009b630 > r13 = 0x00007fff70189240 r14 = 0x00000ffaec01a4a2 > r15 = 0x00007fd7600d2504 rip = 0x0000000001ff22d5 > Found by: call frame info > 5 content_shell!WebCore::ScreenOrientationController::orientation() const > [ScreenOrientationController.cpp : 104 + 0x1d] > rbx = 0x00007fd7600d2510 rbp = 0x00007fff70189150 > rsp = 0x00007fff70189140 r12 = 0x000060700009b630 > r13 = 0x00007fff70189240 r14 = 0x00000ffaec01a4a2 > r15 = 0x00007fd7600d2504 rip = 0x0000000003b8c341 > Found by: call frame info > > ^^^ The crash is happening when ScreenOrientationController accessed > m_frame.view(), not when ScreenOrientationController used the view() value > somewhere. Haraken, what is the rest of this stack trace? Where is this access to ScreenOrientationController::orientation comming from? Is it coming from pageVisibilityChanged? Since these objects don't die together we probably have to explicitly propagate a notification with Oilpan. Maybe we can propagate a notification so that this orientation access never happens?
Message was sent while issue was closed.
On 2014/06/12 06:28:39, Mads Ager (chromium) wrote: > On 2014/06/11 16:12:44, haraken wrote: > > On 2014/06/11 16:05:39, sof wrote: > > > seeing some new flaky oilpan crashers for the past 12 hours, > > > > > > > > > > > > http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40... > > > > > > any chance they're due to this? (Missing view() null check in > > > ScreenOrientationController::orientation(), perhaps?) > > > > Thanks for looking into this. As far as I see the stack trace, it seems that > > m_frame is gone. > > > > Thread 0 (crashed) > > 0 > content_shell!google_breakpad::ExceptionHandler::SimulateSignalDelivery(int) > > [exception_handler.cc : 440 + 0x0] > > rax = 0x000000000abe2040 rdx = 0x0000000000000007 > > rcx = 0x00000fffee031000 rbx = 0x00000fffee03104c > > rsi = 0x0000000000000000 rdi = 0x00007fff70188320 > > rbp = 0x00007fff701887b0 rsp = 0x00007fff70188260 > > r8 = 0x0000000000000080 r9 = 0x0101010101010101 > > r10 = 0x0000000000547057 r11 = 0x000000000abe6590 > > r12 = 0x00007fff70188320 r13 = 0x00007fff70188260 > > r14 = 0x000060c00008a1c0 r15 = 0x00007fff70188280 > > rip = 0x00000000052b8b80 > > Found by: given as instruction pointer in context > > 1 content_shell!~ScopedInErrorReport [asan_report.cc : 570 + 0x9] > > rbx = 0x00007fff701890c0 rbp = 0x000000000784fc1e > > rsp = 0x00007fff701887c0 r12 = 0x0000000007850ad4 > > r13 = 0x00007fff701890c0 r14 = 0x0000000007850ae1 > > r15 = 0x0000000007847ed0 rip = 0x0000000000537f38 > > Found by: call frame info > > 2 content_shell!__asan_report_error [asan_report.cc : 891 + 0x5] > > rbx = 0x00007fff701890c0 rbp = 0x000000000784fc1e > > rsp = 0x00007fff701887d0 r12 = 0x0000000007850ad4 > > r13 = 0x00007fff701890c0 r14 = 0x0000000007850ae1 > > r15 = 0x0000000007847ed0 rip = 0x0000000000537ab1 > > Found by: call frame info > > 3 content_shell!__asan_report_load8 [asan_rtl.cc : 369 + 0x1c] > > rbx = 0x00007fd7600d2510 rbp = 0x00007fff70189120 > > rsp = 0x00007fff70189110 r12 = 0x000060700009b630 > > r13 = 0x00007fff70189240 r14 = 0x00000ffaec01a4a2 > > r15 = 0x00007fd7600d2504 rip = 0x00000000005386b7 > > Found by: call frame info > > 4 content_shell!WebCore::LocalFrame::view() const [RefPtr.h : 56 + 0x5] > > rbx = 0x00007fd7600d2510 rbp = 0x00007fff70189130 > > rsp = 0x00007fff70189130 r12 = 0x000060700009b630 > > r13 = 0x00007fff70189240 r14 = 0x00000ffaec01a4a2 > > r15 = 0x00007fd7600d2504 rip = 0x0000000001ff22d5 > > Found by: call frame info > > 5 content_shell!WebCore::ScreenOrientationController::orientation() const > > [ScreenOrientationController.cpp : 104 + 0x1d] > > rbx = 0x00007fd7600d2510 rbp = 0x00007fff70189150 > > rsp = 0x00007fff70189140 r12 = 0x000060700009b630 > > r13 = 0x00007fff70189240 r14 = 0x00000ffaec01a4a2 > > r15 = 0x00007fd7600d2504 rip = 0x0000000003b8c341 > > Found by: call frame info > > > > ^^^ The crash is happening when ScreenOrientationController accessed > > m_frame.view(), not when ScreenOrientationController used the view() value > > somewhere. > > Haraken, what is the rest of this stack trace? Where is this access to > ScreenOrientationController::orientation comming from? Is it coming from > pageVisibilityChanged? Since these objects don't die together we probably have > to explicitly propagate a notification with Oilpan. Maybe we can propagate a > notification so that this orientation access never happens? Just right now the flakiness dashboard is not working and I cannot get a crash log... http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40...
Message was sent while issue was closed.
On 2014/06/12 07:13:32, haraken wrote: > On 2014/06/12 06:28:39, Mads Ager (chromium) wrote: > > On 2014/06/11 16:12:44, haraken wrote: > > > On 2014/06/11 16:05:39, sof wrote: > > > > seeing some new flaky oilpan crashers for the past 12 hours, > > > > > > > > > > > > > > > > > > http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40... > > > > > > > > any chance they're due to this? (Missing view() null check in > > > > ScreenOrientationController::orientation(), perhaps?) > > > > > > Thanks for looking into this. As far as I see the stack trace, it seems that > > > m_frame is gone. > > > > > > Thread 0 (crashed) > > > 0 > > content_shell!google_breakpad::ExceptionHandler::SimulateSignalDelivery(int) > > > [exception_handler.cc : 440 + 0x0] > > > rax = 0x000000000abe2040 rdx = 0x0000000000000007 > > > rcx = 0x00000fffee031000 rbx = 0x00000fffee03104c > > > rsi = 0x0000000000000000 rdi = 0x00007fff70188320 > > > rbp = 0x00007fff701887b0 rsp = 0x00007fff70188260 > > > r8 = 0x0000000000000080 r9 = 0x0101010101010101 > > > r10 = 0x0000000000547057 r11 = 0x000000000abe6590 > > > r12 = 0x00007fff70188320 r13 = 0x00007fff70188260 > > > r14 = 0x000060c00008a1c0 r15 = 0x00007fff70188280 > > > rip = 0x00000000052b8b80 > > > Found by: given as instruction pointer in context > > > 1 content_shell!~ScopedInErrorReport [asan_report.cc : 570 + 0x9] > > > rbx = 0x00007fff701890c0 rbp = 0x000000000784fc1e > > > rsp = 0x00007fff701887c0 r12 = 0x0000000007850ad4 > > > r13 = 0x00007fff701890c0 r14 = 0x0000000007850ae1 > > > r15 = 0x0000000007847ed0 rip = 0x0000000000537f38 > > > Found by: call frame info > > > 2 content_shell!__asan_report_error [asan_report.cc : 891 + 0x5] > > > rbx = 0x00007fff701890c0 rbp = 0x000000000784fc1e > > > rsp = 0x00007fff701887d0 r12 = 0x0000000007850ad4 > > > r13 = 0x00007fff701890c0 r14 = 0x0000000007850ae1 > > > r15 = 0x0000000007847ed0 rip = 0x0000000000537ab1 > > > Found by: call frame info > > > 3 content_shell!__asan_report_load8 [asan_rtl.cc : 369 + 0x1c] > > > rbx = 0x00007fd7600d2510 rbp = 0x00007fff70189120 > > > rsp = 0x00007fff70189110 r12 = 0x000060700009b630 > > > r13 = 0x00007fff70189240 r14 = 0x00000ffaec01a4a2 > > > r15 = 0x00007fd7600d2504 rip = 0x00000000005386b7 > > > Found by: call frame info > > > 4 content_shell!WebCore::LocalFrame::view() const [RefPtr.h : 56 + 0x5] > > > rbx = 0x00007fd7600d2510 rbp = 0x00007fff70189130 > > > rsp = 0x00007fff70189130 r12 = 0x000060700009b630 > > > r13 = 0x00007fff70189240 r14 = 0x00000ffaec01a4a2 > > > r15 = 0x00007fd7600d2504 rip = 0x0000000001ff22d5 > > > Found by: call frame info > > > 5 content_shell!WebCore::ScreenOrientationController::orientation() const > > > [ScreenOrientationController.cpp : 104 + 0x1d] > > > rbx = 0x00007fd7600d2510 rbp = 0x00007fff70189150 > > > rsp = 0x00007fff70189140 r12 = 0x000060700009b630 > > > r13 = 0x00007fff70189240 r14 = 0x00000ffaec01a4a2 > > > r15 = 0x00007fd7600d2504 rip = 0x0000000003b8c341 > > > Found by: call frame info > > > > > > ^^^ The crash is happening when ScreenOrientationController accessed > > > m_frame.view(), not when ScreenOrientationController used the view() value > > > somewhere. > > > > Haraken, what is the rest of this stack trace? Where is this access to > > ScreenOrientationController::orientation comming from? Is it coming from > > pageVisibilityChanged? Since these objects don't die together we probably have > > to explicitly propagate a notification with Oilpan. Maybe we can propagate a > > notification so that this orientation access never happens? > > Just right now the flakiness dashboard is not working and I cannot get a crash > log... > > http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40... https://storage.googleapis.com/chromium-layout-test-archives/WebKit_Win_Oilpa...
Message was sent while issue was closed.
On 2014/06/12 07:16:15, sof wrote: > On 2014/06/12 07:13:32, haraken wrote: > > On 2014/06/12 06:28:39, Mads Ager (chromium) wrote: > > > On 2014/06/11 16:12:44, haraken wrote: > > > > On 2014/06/11 16:05:39, sof wrote: > > > > > seeing some new flaky oilpan crashers for the past 12 hours, > > > > > > > > > > > > > > > > > > > > > > > > > http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40... > > > > > > > > > > any chance they're due to this? (Missing view() null check in > > > > > ScreenOrientationController::orientation(), perhaps?) > > > > > > > > Thanks for looking into this. As far as I see the stack trace, it seems > that > > > > m_frame is gone. > > > > > > > > Thread 0 (crashed) > > > > 0 > > > content_shell!google_breakpad::ExceptionHandler::SimulateSignalDelivery(int) > > > > [exception_handler.cc : 440 + 0x0] > > > > rax = 0x000000000abe2040 rdx = 0x0000000000000007 > > > > rcx = 0x00000fffee031000 rbx = 0x00000fffee03104c > > > > rsi = 0x0000000000000000 rdi = 0x00007fff70188320 > > > > rbp = 0x00007fff701887b0 rsp = 0x00007fff70188260 > > > > r8 = 0x0000000000000080 r9 = 0x0101010101010101 > > > > r10 = 0x0000000000547057 r11 = 0x000000000abe6590 > > > > r12 = 0x00007fff70188320 r13 = 0x00007fff70188260 > > > > r14 = 0x000060c00008a1c0 r15 = 0x00007fff70188280 > > > > rip = 0x00000000052b8b80 > > > > Found by: given as instruction pointer in context > > > > 1 content_shell!~ScopedInErrorReport [asan_report.cc : 570 + 0x9] > > > > rbx = 0x00007fff701890c0 rbp = 0x000000000784fc1e > > > > rsp = 0x00007fff701887c0 r12 = 0x0000000007850ad4 > > > > r13 = 0x00007fff701890c0 r14 = 0x0000000007850ae1 > > > > r15 = 0x0000000007847ed0 rip = 0x0000000000537f38 > > > > Found by: call frame info > > > > 2 content_shell!__asan_report_error [asan_report.cc : 891 + 0x5] > > > > rbx = 0x00007fff701890c0 rbp = 0x000000000784fc1e > > > > rsp = 0x00007fff701887d0 r12 = 0x0000000007850ad4 > > > > r13 = 0x00007fff701890c0 r14 = 0x0000000007850ae1 > > > > r15 = 0x0000000007847ed0 rip = 0x0000000000537ab1 > > > > Found by: call frame info > > > > 3 content_shell!__asan_report_load8 [asan_rtl.cc : 369 + 0x1c] > > > > rbx = 0x00007fd7600d2510 rbp = 0x00007fff70189120 > > > > rsp = 0x00007fff70189110 r12 = 0x000060700009b630 > > > > r13 = 0x00007fff70189240 r14 = 0x00000ffaec01a4a2 > > > > r15 = 0x00007fd7600d2504 rip = 0x00000000005386b7 > > > > Found by: call frame info > > > > 4 content_shell!WebCore::LocalFrame::view() const [RefPtr.h : 56 + 0x5] > > > > rbx = 0x00007fd7600d2510 rbp = 0x00007fff70189130 > > > > rsp = 0x00007fff70189130 r12 = 0x000060700009b630 > > > > r13 = 0x00007fff70189240 r14 = 0x00000ffaec01a4a2 > > > > r15 = 0x00007fd7600d2504 rip = 0x0000000001ff22d5 > > > > Found by: call frame info > > > > 5 content_shell!WebCore::ScreenOrientationController::orientation() > const > > > > [ScreenOrientationController.cpp : 104 + 0x1d] > > > > rbx = 0x00007fd7600d2510 rbp = 0x00007fff70189150 > > > > rsp = 0x00007fff70189140 r12 = 0x000060700009b630 > > > > r13 = 0x00007fff70189240 r14 = 0x00000ffaec01a4a2 > > > > r15 = 0x00007fd7600d2504 rip = 0x0000000003b8c341 > > > > Found by: call frame info > > > > > > > > ^^^ The crash is happening when ScreenOrientationController accessed > > > > m_frame.view(), not when ScreenOrientationController used the view() value > > > > somewhere. > > > > > > Haraken, what is the rest of this stack trace? Where is this access to > > > ScreenOrientationController::orientation comming from? Is it coming from > > > pageVisibilityChanged? Since these objects don't die together we probably > have > > > to explicitly propagate a notification with Oilpan. Maybe we can propagate a > > > notification so that this orientation access never happens? > > > > Just right now the flakiness dashboard is not working and I cannot get a crash > > log... > > > > > http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40... > > https://storage.googleapis.com/chromium-layout-test-archives/WebKit_Win_Oilpa... Reproduction steps that work for me: gdb --args ./out/Debug/content_shell --single-process --no-sandbox --dump-render-tree - wait for it to print READY and then paste in: fast/dom/DeviceMotion/page-visibility.html fast/dom/DeviceMotion/page-visibility.html fast/dom/DeviceOrientation/page-visibility.html fast/events/page-visibility-iframe-propagation-test.html jquery/manipulation.html screen_orientation/page-visibility.html That leads to: WebCore::Widget::parent (this=0xcdcdcdcdcdcdcdcd) at ../../third_party/WebKit/Source/platform/Widget.h:96 96 Widget* parent() const { return m_parent; } (gdb) where #0 WebCore::Widget::parent (this=0xcdcdcdcdcdcdcdcd) at ../../third_party/WebKit/Source/platform/Widget.h:96 #1 0x00007ffff6ada4ad in WebCore::Widget::root (this=0xcdcdcdcdcdcdcdcd) at ../../third_party/WebKit/Source/platform/Widget.cpp:60 #2 0x00007ffff6acb360 in WebCore::toHostWindow (widget=0xcdcdcdcdcdcdcdcd) at ../../third_party/WebKit/Source/platform/PlatformScreen.cpp:45 #3 0x00007ffff6acb5f5 in WebCore::screenOrientationType ( widget=0xcdcdcdcdcdcdcdcd) at ../../third_party/WebKit/Source/platform/PlatformScreen.cpp:101 #4 0x00007fffeb6f4b52 in WebCore::ScreenOrientationController::orientation ( this=0x7fffc6efb4a8) So it would seem that the widget has been deallocated at this point. I'll keep digging but thought I would post the reproduction steps.
Message was sent while issue was closed.
On 2014/06/12 08:31:00, Mads Ager (chromium) wrote: > On 2014/06/12 07:16:15, sof wrote: > > On 2014/06/12 07:13:32, haraken wrote: > > > On 2014/06/12 06:28:39, Mads Ager (chromium) wrote: > > > > On 2014/06/11 16:12:44, haraken wrote: > > > > > On 2014/06/11 16:05:39, sof wrote: > > > > > > seeing some new flaky oilpan crashers for the past 12 hours, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40... > > > > > > > > > > > > any chance they're due to this? (Missing view() null check in > > > > > > ScreenOrientationController::orientation(), perhaps?) > > > > > > > > > > Thanks for looking into this. As far as I see the stack trace, it seems > > that > > > > > m_frame is gone. > > > > > > > > > > Thread 0 (crashed) > > > > > 0 > > > > > content_shell!google_breakpad::ExceptionHandler::SimulateSignalDelivery(int) > > > > > [exception_handler.cc : 440 + 0x0] > > > > > rax = 0x000000000abe2040 rdx = 0x0000000000000007 > > > > > rcx = 0x00000fffee031000 rbx = 0x00000fffee03104c > > > > > rsi = 0x0000000000000000 rdi = 0x00007fff70188320 > > > > > rbp = 0x00007fff701887b0 rsp = 0x00007fff70188260 > > > > > r8 = 0x0000000000000080 r9 = 0x0101010101010101 > > > > > r10 = 0x0000000000547057 r11 = 0x000000000abe6590 > > > > > r12 = 0x00007fff70188320 r13 = 0x00007fff70188260 > > > > > r14 = 0x000060c00008a1c0 r15 = 0x00007fff70188280 > > > > > rip = 0x00000000052b8b80 > > > > > Found by: given as instruction pointer in context > > > > > 1 content_shell!~ScopedInErrorReport [asan_report.cc : 570 + 0x9] > > > > > rbx = 0x00007fff701890c0 rbp = 0x000000000784fc1e > > > > > rsp = 0x00007fff701887c0 r12 = 0x0000000007850ad4 > > > > > r13 = 0x00007fff701890c0 r14 = 0x0000000007850ae1 > > > > > r15 = 0x0000000007847ed0 rip = 0x0000000000537f38 > > > > > Found by: call frame info > > > > > 2 content_shell!__asan_report_error [asan_report.cc : 891 + 0x5] > > > > > rbx = 0x00007fff701890c0 rbp = 0x000000000784fc1e > > > > > rsp = 0x00007fff701887d0 r12 = 0x0000000007850ad4 > > > > > r13 = 0x00007fff701890c0 r14 = 0x0000000007850ae1 > > > > > r15 = 0x0000000007847ed0 rip = 0x0000000000537ab1 > > > > > Found by: call frame info > > > > > 3 content_shell!__asan_report_load8 [asan_rtl.cc : 369 + 0x1c] > > > > > rbx = 0x00007fd7600d2510 rbp = 0x00007fff70189120 > > > > > rsp = 0x00007fff70189110 r12 = 0x000060700009b630 > > > > > r13 = 0x00007fff70189240 r14 = 0x00000ffaec01a4a2 > > > > > r15 = 0x00007fd7600d2504 rip = 0x00000000005386b7 > > > > > Found by: call frame info > > > > > 4 content_shell!WebCore::LocalFrame::view() const [RefPtr.h : 56 + > 0x5] > > > > > rbx = 0x00007fd7600d2510 rbp = 0x00007fff70189130 > > > > > rsp = 0x00007fff70189130 r12 = 0x000060700009b630 > > > > > r13 = 0x00007fff70189240 r14 = 0x00000ffaec01a4a2 > > > > > r15 = 0x00007fd7600d2504 rip = 0x0000000001ff22d5 > > > > > Found by: call frame info > > > > > 5 content_shell!WebCore::ScreenOrientationController::orientation() > > const > > > > > [ScreenOrientationController.cpp : 104 + 0x1d] > > > > > rbx = 0x00007fd7600d2510 rbp = 0x00007fff70189150 > > > > > rsp = 0x00007fff70189140 r12 = 0x000060700009b630 > > > > > r13 = 0x00007fff70189240 r14 = 0x00000ffaec01a4a2 > > > > > r15 = 0x00007fd7600d2504 rip = 0x0000000003b8c341 > > > > > Found by: call frame info > > > > > > > > > > ^^^ The crash is happening when ScreenOrientationController accessed > > > > > m_frame.view(), not when ScreenOrientationController used the view() > value > > > > > somewhere. > > > > > > > > Haraken, what is the rest of this stack trace? Where is this access to > > > > ScreenOrientationController::orientation comming from? Is it coming from > > > > pageVisibilityChanged? Since these objects don't die together we probably > > have > > > > to explicitly propagate a notification with Oilpan. Maybe we can propagate > a > > > > notification so that this orientation access never happens? > > > > > > Just right now the flakiness dashboard is not working and I cannot get a > crash > > > log... > > > > > > > > > http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40... > > > > > https://storage.googleapis.com/chromium-layout-test-archives/WebKit_Win_Oilpa... > > Reproduction steps that work for me: > > gdb --args ./out/Debug/content_shell --single-process --no-sandbox > --dump-render-tree - > > wait for it to print READY and then paste in: > > fast/dom/DeviceMotion/page-visibility.html > fast/dom/DeviceMotion/page-visibility.html > fast/dom/DeviceOrientation/page-visibility.html > fast/events/page-visibility-iframe-propagation-test.html > jquery/manipulation.html > screen_orientation/page-visibility.html > > That leads to: > > WebCore::Widget::parent (this=0xcdcdcdcdcdcdcdcd) > at ../../third_party/WebKit/Source/platform/Widget.h:96 > 96 Widget* parent() const { return m_parent; } > (gdb) where > #0 WebCore::Widget::parent (this=0xcdcdcdcdcdcdcdcd) > at ../../third_party/WebKit/Source/platform/Widget.h:96 > #1 0x00007ffff6ada4ad in WebCore::Widget::root (this=0xcdcdcdcdcdcdcdcd) > at ../../third_party/WebKit/Source/platform/Widget.cpp:60 > #2 0x00007ffff6acb360 in WebCore::toHostWindow (widget=0xcdcdcdcdcdcdcdcd) > at ../../third_party/WebKit/Source/platform/PlatformScreen.cpp:45 > #3 0x00007ffff6acb5f5 in WebCore::screenOrientationType ( > widget=0xcdcdcdcdcdcdcdcd) > at ../../third_party/WebKit/Source/platform/PlatformScreen.cpp:101 > #4 0x00007fffeb6f4b52 in WebCore::ScreenOrientationController::orientation ( > this=0x7fffc6efb4a8) > > So it would seem that the widget has been deallocated at this point. I'll keep > digging but thought I would post the reproduction steps. Right, so what is happening is that the frame is dead but that does not kill the supplements because they are in the heap. Therefore, the ScreenOrientationController supplement is still regsitered as a page lifecycle observer with oilpan and therefore we get this notification with a dead frame only in the oilpan build. With oilpan we need to explicitly unregister the ScreenOrientationController as a page life-cycle observer when the LocalFrame is going down.
Message was sent while issue was closed.
On 2014/06/12 08:41:50, Mads Ager (chromium) wrote: > On 2014/06/12 08:31:00, Mads Ager (chromium) wrote: > > On 2014/06/12 07:16:15, sof wrote: > > > On 2014/06/12 07:13:32, haraken wrote: > > > > On 2014/06/12 06:28:39, Mads Ager (chromium) wrote: > > > > > On 2014/06/11 16:12:44, haraken wrote: > > > > > > On 2014/06/11 16:05:39, sof wrote: > > > > > > > seeing some new flaky oilpan crashers for the past 12 hours, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40... > > > > > > > > > > > > > > any chance they're due to this? (Missing view() null check in > > > > > > > ScreenOrientationController::orientation(), perhaps?) > > > > > > > > > > > > Thanks for looking into this. As far as I see the stack trace, it > seems > > > that > > > > > > m_frame is gone. > > > > > > > > > > > > Thread 0 (crashed) > > > > > > 0 > > > > > > > content_shell!google_breakpad::ExceptionHandler::SimulateSignalDelivery(int) > > > > > > [exception_handler.cc : 440 + 0x0] > > > > > > rax = 0x000000000abe2040 rdx = 0x0000000000000007 > > > > > > rcx = 0x00000fffee031000 rbx = 0x00000fffee03104c > > > > > > rsi = 0x0000000000000000 rdi = 0x00007fff70188320 > > > > > > rbp = 0x00007fff701887b0 rsp = 0x00007fff70188260 > > > > > > r8 = 0x0000000000000080 r9 = 0x0101010101010101 > > > > > > r10 = 0x0000000000547057 r11 = 0x000000000abe6590 > > > > > > r12 = 0x00007fff70188320 r13 = 0x00007fff70188260 > > > > > > r14 = 0x000060c00008a1c0 r15 = 0x00007fff70188280 > > > > > > rip = 0x00000000052b8b80 > > > > > > Found by: given as instruction pointer in context > > > > > > 1 content_shell!~ScopedInErrorReport [asan_report.cc : 570 + 0x9] > > > > > > rbx = 0x00007fff701890c0 rbp = 0x000000000784fc1e > > > > > > rsp = 0x00007fff701887c0 r12 = 0x0000000007850ad4 > > > > > > r13 = 0x00007fff701890c0 r14 = 0x0000000007850ae1 > > > > > > r15 = 0x0000000007847ed0 rip = 0x0000000000537f38 > > > > > > Found by: call frame info > > > > > > 2 content_shell!__asan_report_error [asan_report.cc : 891 + 0x5] > > > > > > rbx = 0x00007fff701890c0 rbp = 0x000000000784fc1e > > > > > > rsp = 0x00007fff701887d0 r12 = 0x0000000007850ad4 > > > > > > r13 = 0x00007fff701890c0 r14 = 0x0000000007850ae1 > > > > > > r15 = 0x0000000007847ed0 rip = 0x0000000000537ab1 > > > > > > Found by: call frame info > > > > > > 3 content_shell!__asan_report_load8 [asan_rtl.cc : 369 + 0x1c] > > > > > > rbx = 0x00007fd7600d2510 rbp = 0x00007fff70189120 > > > > > > rsp = 0x00007fff70189110 r12 = 0x000060700009b630 > > > > > > r13 = 0x00007fff70189240 r14 = 0x00000ffaec01a4a2 > > > > > > r15 = 0x00007fd7600d2504 rip = 0x00000000005386b7 > > > > > > Found by: call frame info > > > > > > 4 content_shell!WebCore::LocalFrame::view() const [RefPtr.h : 56 + > > 0x5] > > > > > > rbx = 0x00007fd7600d2510 rbp = 0x00007fff70189130 > > > > > > rsp = 0x00007fff70189130 r12 = 0x000060700009b630 > > > > > > r13 = 0x00007fff70189240 r14 = 0x00000ffaec01a4a2 > > > > > > r15 = 0x00007fd7600d2504 rip = 0x0000000001ff22d5 > > > > > > Found by: call frame info > > > > > > 5 content_shell!WebCore::ScreenOrientationController::orientation() > > > const > > > > > > [ScreenOrientationController.cpp : 104 + 0x1d] > > > > > > rbx = 0x00007fd7600d2510 rbp = 0x00007fff70189150 > > > > > > rsp = 0x00007fff70189140 r12 = 0x000060700009b630 > > > > > > r13 = 0x00007fff70189240 r14 = 0x00000ffaec01a4a2 > > > > > > r15 = 0x00007fd7600d2504 rip = 0x0000000003b8c341 > > > > > > Found by: call frame info > > > > > > > > > > > > ^^^ The crash is happening when ScreenOrientationController accessed > > > > > > m_frame.view(), not when ScreenOrientationController used the view() > > value > > > > > > somewhere. > > > > > > > > > > Haraken, what is the rest of this stack trace? Where is this access to > > > > > ScreenOrientationController::orientation comming from? Is it coming from > > > > > pageVisibilityChanged? Since these objects don't die together we > probably > > > have > > > > > to explicitly propagate a notification with Oilpan. Maybe we can > propagate > > a > > > > > notification so that this orientation access never happens? > > > > > > > > Just right now the flakiness dashboard is not working and I cannot get a > > crash > > > > log... > > > > > > > > > > > > > > http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40... > > > > > > > > > https://storage.googleapis.com/chromium-layout-test-archives/WebKit_Win_Oilpa... > > > > Reproduction steps that work for me: > > > > gdb --args ./out/Debug/content_shell --single-process --no-sandbox > > --dump-render-tree - > > > > wait for it to print READY and then paste in: > > > > fast/dom/DeviceMotion/page-visibility.html > > fast/dom/DeviceMotion/page-visibility.html > > fast/dom/DeviceOrientation/page-visibility.html > > fast/events/page-visibility-iframe-propagation-test.html > > jquery/manipulation.html > > screen_orientation/page-visibility.html > > > > That leads to: > > > > WebCore::Widget::parent (this=0xcdcdcdcdcdcdcdcd) > > at ../../third_party/WebKit/Source/platform/Widget.h:96 > > 96 Widget* parent() const { return m_parent; } > > (gdb) where > > #0 WebCore::Widget::parent (this=0xcdcdcdcdcdcdcdcd) > > at ../../third_party/WebKit/Source/platform/Widget.h:96 > > #1 0x00007ffff6ada4ad in WebCore::Widget::root (this=0xcdcdcdcdcdcdcdcd) > > at ../../third_party/WebKit/Source/platform/Widget.cpp:60 > > #2 0x00007ffff6acb360 in WebCore::toHostWindow (widget=0xcdcdcdcdcdcdcdcd) > > at ../../third_party/WebKit/Source/platform/PlatformScreen.cpp:45 > > #3 0x00007ffff6acb5f5 in WebCore::screenOrientationType ( > > widget=0xcdcdcdcdcdcdcdcd) > > at ../../third_party/WebKit/Source/platform/PlatformScreen.cpp:101 > > #4 0x00007fffeb6f4b52 in WebCore::ScreenOrientationController::orientation ( > > this=0x7fffc6efb4a8) > > > > So it would seem that the widget has been deallocated at this point. I'll keep > > digging but thought I would post the reproduction steps. > > Right, so what is happening is that the frame is dead but that does not kill the > supplements because they are in the heap. Therefore, the > ScreenOrientationController supplement is still regsitered as a page lifecycle > observer with oilpan and therefore we get this notification with a dead frame > only in the oilpan build. With oilpan we need to explicitly unregister the > ScreenOrientationController as a page life-cycle observer when the LocalFrame is > going down. That looks very much like the problem. I'll upload a fix for this shortly.
Message was sent while issue was closed.
On 2014/06/12 08:54:46, zerny-chromium wrote: > On 2014/06/12 08:41:50, Mads Ager (chromium) wrote: > > On 2014/06/12 08:31:00, Mads Ager (chromium) wrote: > > > On 2014/06/12 07:16:15, sof wrote: > > > > On 2014/06/12 07:13:32, haraken wrote: > > > > > On 2014/06/12 06:28:39, Mads Ager (chromium) wrote: > > > > > > On 2014/06/11 16:12:44, haraken wrote: > > > > > > > On 2014/06/11 16:05:39, sof wrote: > > > > > > > > seeing some new flaky oilpan crashers for the past 12 hours, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40... > > > > > > > > > > > > > > > > any chance they're due to this? (Missing view() null check in > > > > > > > > ScreenOrientationController::orientation(), perhaps?) > > > > > > > > > > > > > > Thanks for looking into this. As far as I see the stack trace, it > > seems > > > > that > > > > > > > m_frame is gone. > > > > > > > > > > > > > > Thread 0 (crashed) > > > > > > > 0 > > > > > > > > > content_shell!google_breakpad::ExceptionHandler::SimulateSignalDelivery(int) > > > > > > > [exception_handler.cc : 440 + 0x0] > > > > > > > rax = 0x000000000abe2040 rdx = 0x0000000000000007 > > > > > > > rcx = 0x00000fffee031000 rbx = 0x00000fffee03104c > > > > > > > rsi = 0x0000000000000000 rdi = 0x00007fff70188320 > > > > > > > rbp = 0x00007fff701887b0 rsp = 0x00007fff70188260 > > > > > > > r8 = 0x0000000000000080 r9 = 0x0101010101010101 > > > > > > > r10 = 0x0000000000547057 r11 = 0x000000000abe6590 > > > > > > > r12 = 0x00007fff70188320 r13 = 0x00007fff70188260 > > > > > > > r14 = 0x000060c00008a1c0 r15 = 0x00007fff70188280 > > > > > > > rip = 0x00000000052b8b80 > > > > > > > Found by: given as instruction pointer in context > > > > > > > 1 content_shell!~ScopedInErrorReport [asan_report.cc : 570 + 0x9] > > > > > > > rbx = 0x00007fff701890c0 rbp = 0x000000000784fc1e > > > > > > > rsp = 0x00007fff701887c0 r12 = 0x0000000007850ad4 > > > > > > > r13 = 0x00007fff701890c0 r14 = 0x0000000007850ae1 > > > > > > > r15 = 0x0000000007847ed0 rip = 0x0000000000537f38 > > > > > > > Found by: call frame info > > > > > > > 2 content_shell!__asan_report_error [asan_report.cc : 891 + 0x5] > > > > > > > rbx = 0x00007fff701890c0 rbp = 0x000000000784fc1e > > > > > > > rsp = 0x00007fff701887d0 r12 = 0x0000000007850ad4 > > > > > > > r13 = 0x00007fff701890c0 r14 = 0x0000000007850ae1 > > > > > > > r15 = 0x0000000007847ed0 rip = 0x0000000000537ab1 > > > > > > > Found by: call frame info > > > > > > > 3 content_shell!__asan_report_load8 [asan_rtl.cc : 369 + 0x1c] > > > > > > > rbx = 0x00007fd7600d2510 rbp = 0x00007fff70189120 > > > > > > > rsp = 0x00007fff70189110 r12 = 0x000060700009b630 > > > > > > > r13 = 0x00007fff70189240 r14 = 0x00000ffaec01a4a2 > > > > > > > r15 = 0x00007fd7600d2504 rip = 0x00000000005386b7 > > > > > > > Found by: call frame info > > > > > > > 4 content_shell!WebCore::LocalFrame::view() const [RefPtr.h : 56 + > > > 0x5] > > > > > > > rbx = 0x00007fd7600d2510 rbp = 0x00007fff70189130 > > > > > > > rsp = 0x00007fff70189130 r12 = 0x000060700009b630 > > > > > > > r13 = 0x00007fff70189240 r14 = 0x00000ffaec01a4a2 > > > > > > > r15 = 0x00007fd7600d2504 rip = 0x0000000001ff22d5 > > > > > > > Found by: call frame info > > > > > > > 5 > content_shell!WebCore::ScreenOrientationController::orientation() > > > > const > > > > > > > [ScreenOrientationController.cpp : 104 + 0x1d] > > > > > > > rbx = 0x00007fd7600d2510 rbp = 0x00007fff70189150 > > > > > > > rsp = 0x00007fff70189140 r12 = 0x000060700009b630 > > > > > > > r13 = 0x00007fff70189240 r14 = 0x00000ffaec01a4a2 > > > > > > > r15 = 0x00007fd7600d2504 rip = 0x0000000003b8c341 > > > > > > > Found by: call frame info > > > > > > > > > > > > > > ^^^ The crash is happening when ScreenOrientationController accessed > > > > > > > m_frame.view(), not when ScreenOrientationController used the view() > > > value > > > > > > > somewhere. > > > > > > > > > > > > Haraken, what is the rest of this stack trace? Where is this access to > > > > > > ScreenOrientationController::orientation comming from? Is it coming > from > > > > > > pageVisibilityChanged? Since these objects don't die together we > > probably > > > > have > > > > > > to explicitly propagate a notification with Oilpan. Maybe we can > > propagate > > > a > > > > > > notification so that this orientation access never happens? > > > > > > > > > > Just right now the flakiness dashboard is not working and I cannot get a > > > crash > > > > > log... > > > > > > > > > > > > > > > > > > > > http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40... > > > > > > > > > > > > > > https://storage.googleapis.com/chromium-layout-test-archives/WebKit_Win_Oilpa... > > > > > > Reproduction steps that work for me: > > > > > > gdb --args ./out/Debug/content_shell --single-process --no-sandbox > > > --dump-render-tree - > > > > > > wait for it to print READY and then paste in: > > > > > > fast/dom/DeviceMotion/page-visibility.html > > > fast/dom/DeviceMotion/page-visibility.html > > > fast/dom/DeviceOrientation/page-visibility.html > > > fast/events/page-visibility-iframe-propagation-test.html > > > jquery/manipulation.html > > > screen_orientation/page-visibility.html > > > > > > That leads to: > > > > > > WebCore::Widget::parent (this=0xcdcdcdcdcdcdcdcd) > > > at ../../third_party/WebKit/Source/platform/Widget.h:96 > > > 96 Widget* parent() const { return m_parent; } > > > (gdb) where > > > #0 WebCore::Widget::parent (this=0xcdcdcdcdcdcdcdcd) > > > at ../../third_party/WebKit/Source/platform/Widget.h:96 > > > #1 0x00007ffff6ada4ad in WebCore::Widget::root (this=0xcdcdcdcdcdcdcdcd) > > > at ../../third_party/WebKit/Source/platform/Widget.cpp:60 > > > #2 0x00007ffff6acb360 in WebCore::toHostWindow (widget=0xcdcdcdcdcdcdcdcd) > > > at ../../third_party/WebKit/Source/platform/PlatformScreen.cpp:45 > > > #3 0x00007ffff6acb5f5 in WebCore::screenOrientationType ( > > > widget=0xcdcdcdcdcdcdcdcd) > > > at ../../third_party/WebKit/Source/platform/PlatformScreen.cpp:101 > > > #4 0x00007fffeb6f4b52 in WebCore::ScreenOrientationController::orientation > ( > > > this=0x7fffc6efb4a8) > > > > > > So it would seem that the widget has been deallocated at this point. I'll > keep > > > digging but thought I would post the reproduction steps. > > > > Right, so what is happening is that the frame is dead but that does not kill > the > > supplements because they are in the heap. Therefore, the > > ScreenOrientationController supplement is still regsitered as a page lifecycle > > observer with oilpan and therefore we get this notification with a dead frame > > only in the oilpan build. With oilpan we need to explicitly unregister the > > ScreenOrientationController as a page life-cycle observer when the LocalFrame > is > > going down. > > That looks very much like the problem. I'll upload a fix for this shortly. Fix CL at: https://codereview.chromium.org/335573004/ |
