Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(298)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 2600143002: Revert of IntersectionObserverCallbackImpl should use Member<ExecutionContext> (Closed)

Created:
3 years, 12 months ago by Avi (use Gerrit)
Modified:
3 years, 11 months ago
Reviewers:
haraken, sof
CC:
chromium-reviews, sof, eae+blinkwatch, blink-reviews-dom_chromium.org, dglazkov+blink, blink-reviews, rwlbuis
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of IntersectionObserverCallbackImpl should use Member<ExecutionContext> (patchset #1 id:1 of https://codereview.chromium.org/2601733002/ ) Reason for revert: Crashes telemetry bots. https://build.chromium.org/p/chromium.mac/builders/Mac10.10%20Tests/builds/11254 https://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%281%29/builds/61679 005be100 67b85543 00000012 212a2238 00000000 chrome_child!blink::IntersectionObserver::computeIntersectionObservations+0x2e 005be140 67c44f75 212a2238 005be184 67c43f05 chrome_child!blink::IntersectionObserverController::computeTrackedIntersectionObservations+0xa2 005be14c 67c43f05 00000012 6939fcdc 2b941820 chrome_child!blink::FrameView::updateViewportIntersectionsForSubtree+0x3b 005be184 67c439fd 00000012 67e3e961 384b0000 chrome_child!blink::FrameView::updateLifecyclePhasesInternal+0x22f 005be18c 67e3e961 384b0000 00000000 005be1a8 chrome_child!blink::FrameView::updateAllLifecyclePhases+0x18 005be19c 679b9ce1 318e1a88 005be1e0 679a77e5 chrome_child!blink::PageAnimator::updateAllLifecyclePhases+0x1c 005be1a8 679a77e5 318e1820 318e1a88 0139c080 chrome_child!blink::PageWidgetDelegate::updateAllLifecyclePhases+0x11 005be1e0 684c45d9 049c90fc 04952dd0 00000000 chrome_child!blink::WebViewImpl::updateAllLifecyclePhases+0xa2 005be354 684cd388 04952dd0 00000000 0165ca70 chrome_child!cc::ProxyMain::BeginMainFrame+0x409 005be4d0 684ceba6 049c90f8 684c41d0 049c90e8 chrome_child!base::internal::Invoker<base::internal::BindState<void (__thiscall cc::ProxyMain::*)(std::unique_ptr<cc::BeginMainFrameAndCommitState,std::default_delete<cc::BeginMainFrameAndCommitState> >),base::WeakPtr<cc::ProxyMain>,base::internal::PassedWrapper<std::unique_ptr<cc::BeginMainFrameAndCommitState,std::default_delete<cc::BeginMainFrameAndCommitState> > > >,void __cdecl(void)>::RunImpl<void (__thiscall cc::ProxyMain::*const &)(std::unique_ptr<cc::BeginMainFrameAndCommitState,std::default_delete<cc::BeginMainFrameAndCommitState> >),std::tuple<base::WeakPtr<cc::ProxyMain>,base::internal::PassedWrapper<std::unique_ptr<cc::BeginMainFrameAndCommitState,std::default_delete<cc::BeginMainFrameAndCommitState> > > > const &,0,1>+0x98 005be4e4 671ff214 049c90e8 0165dcd0 00000000 chrome_child!base::internal::Invoker<base::internal::BindState<void (__thiscall cc::ProxyMain::*)(std::unique_ptr<cc::BeginMainFrameAndCommitState,std::default_delete<cc::BeginMainFrameAndCommitState> >),base::WeakPtr<cc::ProxyMain>,base::internal::PassedWrapper<std::unique_ptr<cc::BeginMainFrameAndCommitState,std::default_delete<cc::BeginMainFrameAndCommitState> > > >,void __cdecl(void)>::Run+0x16 005be5a4 67962b56 68e02cb4 005be8e0 005be9dc chrome_child!base::debug::TaskAnnotator::RunTask+0x164 005be9cc 679622cc 0165be08 005bea0c 0165c958 chrome_child!blink::scheduler::TaskQueueManager::ProcessTaskFromWorkQueue+0x3dc 005bea38 6796176f 00000000 00000000 00000003 chrome_child!blink::scheduler::TaskQueueManager::DoWork+0x119 005bea4c 679617b5 679621b3 00000000 0165bc38 chrome_child!base::internal::FunctorTraits<void (__thiscall blink::scheduler::TaskQueueManager::*)(base::TimeTicks,bool),void>::Invoke<base::WeakPtr<blink::scheduler::TaskQueueManager> const &,base::TimeTicks const &,bool const &>+0x22 005bea68 679617d1 0165bc20 0165bc38 0165bc30 chrome_child!base::internal::InvokeHelper<1,void>::MakeItSo<void (__thiscall blink::scheduler::TaskQueueManager::*const &)(base::TimeTicks,bool),base::WeakPtr<blink::scheduler::TaskQueueManager> const &,base::TimeTicks const &,bool const &>+0x25 005bea80 67962e9d 0165bc20 0165bc28 0165bc10 chrome_child!base::internal::Invoker<base::internal::BindState<void (__thiscall blink::scheduler::TaskQueueManager::*)(base::TimeTicks,bool),base::WeakPtr<blink::scheduler::TaskQueueManager>,base::TimeTicks,bool>,void __cdecl(void)>::RunImpl<void (__thiscall blink::scheduler::TaskQueueManager::*const &)(base::TimeTicks,bool),std::tuple<base::WeakPtr<blink::scheduler::TaskQueueManager>,base::TimeTicks,bool> const &,0,1,2>+0x17 005bea94 671ff214 0165bc10 00000000 00000000 chrome_child!base::internal::Invoker<base::internal::BindState<void (__thiscall blink::scheduler::TaskQueueManager::*)(base::TimeTicks,bool),base::WeakPtr<blink::scheduler::TaskQueueManager>,base::TimeTicks,bool>,void __cdecl(void)>::Run+0x16 005beb54 671cb5ad 68bfd37c 005bf830 0165c958 chrome_child!base::debug::TaskAnnotator::RunTask+0x164 005bf7b8 671caa75 005bf830 0164adb8 0165c958 chrome_child!base::MessageLoop::RunTask+0x4cd 005bf874 67200714 00000000 0165c958 00000000 chrome_child!base::MessageLoop::DoWork+0x255 005bf88c 671cb0ce 0165c958 005bf9dc 68b17d6c chrome_child!base::MessagePumpDefault::Run+0x74 005bf954 671d95af 68f11b10 00000000 687c6210 chrome_child!base::MessageLoop::RunHandler+0x5e 005bf9c8 6816baa8 01631980 00000003 01632a28 chrome_child!base::RunLoop::Run+0x6f 005bfaa4 67186ea4 005bfadc 0163d818 00000000 chrome_child!content::RendererMain+0x1e6 005bfab8 67186e12 005bfaf4 005bfadc 005bfbe4 chrome_child!content::RunNamedProcessTypeMain+0x61 005bfb10 671864d9 0100b768 6d004270 005bfc18 chrome_child!content::ContentMainRunnerImpl::Run+0x97 005bfb20 668fd5a7 005bfc00 668fd4f0 0100b770 chrome_child!content::ContentMain+0x23 Original issue's description: > IntersectionObserverCallbackImpl should use Member<ExecutionContext> > > Currently IntersectionObserverCallbackImpl uses WeakMember<ExecutionContext> > but I don't see any reason it has to be a weak member. > Also some call site assumes that IntersectionObserverCallbackImpl::getExecutionContext() > doesn't return null. > > So it looks better to let IntersectionObserverCallbackImpl own Member<ExecutionContext>. > > BUG=610176 > > Committed: https://crrev.com/61862d1ea4b844b5b5051ea34945284b4a5dac71 > Cr-Commit-Position: refs/heads/master@{#440714} TBR=sigbjornf@opera.com,haraken@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=610176 Committed: https://crrev.com/90fda9f137b984c32b16bccdb664bc9742865331 Cr-Commit-Position: refs/heads/master@{#440721}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+6 lines, -3 lines) Patch
M third_party/WebKit/Source/core/dom/IntersectionObserver.cpp View 3 chunks +6 lines, -3 lines 0 comments Download

Messages

Total messages: 9 (4 generated)
Avi (use Gerrit)
Created Revert of IntersectionObserverCallbackImpl should use Member<ExecutionContext>
3 years, 12 months ago (2016-12-26 20:17:06 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2600143002/1
3 years, 12 months ago (2016-12-26 20:17:21 UTC) #3
commit-bot: I haz the power
Committed patchset #1 (id:1)
3 years, 12 months ago (2016-12-26 20:18:23 UTC) #6
haraken
Thanks, LGTM to revert
3 years, 12 months ago (2016-12-27 00:33:00 UTC) #7
commit-bot: I haz the power
3 years, 11 months ago (2017-01-02 15:45:12 UTC) #9
Message was sent while issue was closed. 
Patchset 1 (id:??) landed as
https://crrev.com/90fda9f137b984c32b16bccdb664bc9742865331
Cr-Commit-Position: refs/heads/master@{#440721}

Powered by Google App Engine
This is Rietveld 408576698