Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2734713002:
WebViewTestProxyBase: clear out main test delegate upon destruction. (Closed)
DescriptionWebViewTestProxyBase: clear out main test delegate upon destruction.
The first BlinkTestRunner created is set as the main delegate of test
interfaces along with being set as the delegate of the web view test
proxy object.
When that view test proxy goes away, unregister the main delegate at
the same time as it can no longer be safely accessed.
R=tkent
BUG=698166
Review-Url: https://codereview.chromium.org/2734713002
Cr-Commit-Position: refs/heads/master@{#454834}
Committed: https://chromium.googlesource.com/chromium/src/+/dd42ff761a2b4725279bb659de9e5205567875a4
Patch Set 1 #Patch Set 2 : trigger layout tests.. #Patch Set 3 : undo ps#2 #
Total comments: 1
Messages
Total messages: 24 (18 generated)
The CQ bit was checked by sigbjornf@opera.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by sigbjornf@opera.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by sigbjornf@opera.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
sigbjornf@opera.com changed reviewers: + dmazzoni@chromium.org, tkent@chromium.org
please take a look.
lgtm
Description was changed from ========== WebViewTestProxyBase: clear out main test delegate upon destruction. The first BlinkTestRunner created is set as the main delegate of test interfaces along with being set as the delegate of the web view test proxy object. When that view test proxy goes away, unregister the main delegate at the same time as it can no longer be safely accessed. R= BUG=698166 ========== to ========== WebViewTestProxyBase: clear out main test delegate upon destruction. The first BlinkTestRunner created is set as the main delegate of test interfaces along with being set as the delegate of the web view test proxy object. When that view test proxy goes away, unregister the main delegate at the same time as it can no longer be safely accessed. R=tkent BUG=698166 ==========
The CQ bit was checked by sigbjornf@opera.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1488781855178080,
"parent_rev": "819689c1610b05a75dd3c20e3d9dccfa2781309e", "commit_rev":
"dd42ff761a2b4725279bb659de9e5205567875a4"}
Message was sent while issue was closed.
Description was changed from ========== WebViewTestProxyBase: clear out main test delegate upon destruction. The first BlinkTestRunner created is set as the main delegate of test interfaces along with being set as the delegate of the web view test proxy object. When that view test proxy goes away, unregister the main delegate at the same time as it can no longer be safely accessed. R=tkent BUG=698166 ========== to ========== WebViewTestProxyBase: clear out main test delegate upon destruction. The first BlinkTestRunner created is set as the main delegate of test interfaces along with being set as the delegate of the web view test proxy object. When that view test proxy goes away, unregister the main delegate at the same time as it can no longer be safely accessed. R=tkent BUG=698166 Review-Url: https://codereview.chromium.org/2734713002 Cr-Commit-Position: refs/heads/master@{#454834} Committed: https://chromium.googlesource.com/chromium/src/+/dd42ff761a2b4725279bb659de9e... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/dd42ff761a2b4725279bb659de9e...
Message was sent while issue was closed.
tasak@google.com changed reviewers: + tasak@google.com
Message was sent while issue was closed.
https://codereview.chromium.org/2734713002/diff/40001/content/shell/test_runn... File content/shell/test_runner/web_view_test_proxy.cc (right): https://codereview.chromium.org/2734713002/diff/40001/content/shell/test_runn... content/shell/test_runner/web_view_test_proxy.cc:36: test_interfaces_->SetDelegate(nullptr); I think, this breaks "--single-process". Received signal 6 #0 0x7f2824a7939c base::debug::StackTrace::StackTrace() #1 0x7f2824a78f01 base::debug::(anonymous namespace)::StackDumpSignalHandler() #2 0x7f28247aa330 <unknown> #3 0x7f281cdb4c37 gsignal #4 0x7f281cdb8028 abort #5 0x7f2824a76c75 base::debug::BreakDebugger() #6 0x7f2824a966c6 logging::LogMessage::~LogMessage() #7 0x7f28202c9546 test_runner::GamepadController::Create() #8 0x7f28202dd39c test_runner::TestInterfaces::SetDelegate() #9 0x7f282030fe60 test_runner::WebViewTestProxyBase::~WebViewTestProxyBase() #10 0x000000565b55 _ZN11test_runner16WebViewTestProxyIN7content14RenderViewImplEJPNS1_22CompositorDependenciesERKNS1_5mojom16CreateViewParamsEEED0Ev #11 0x7f2823d2843d _ZN4base8internal9BindStateIMN7content12RenderWidgetEFvvEJ13scoped_refptrIS3_EEE7DestroyEPKNS0_13BindStateBaseE #12 0x7f2824a79eff base::debug::TaskAnnotator::RunTask() SetDelegate(nullptr) invokes test_runner::GamepadController::Create(nullptr).
Message was sent while issue was closed.
On 2017/03/07 03:06:02, tasak wrote: > https://codereview.chromium.org/2734713002/diff/40001/content/shell/test_runn... > File content/shell/test_runner/web_view_test_proxy.cc (right): > > https://codereview.chromium.org/2734713002/diff/40001/content/shell/test_runn... > content/shell/test_runner/web_view_test_proxy.cc:36: > test_interfaces_->SetDelegate(nullptr); > I think, this breaks "--single-process". > > Received signal 6 > #0 0x7f2824a7939c base::debug::StackTrace::StackTrace() > #1 0x7f2824a78f01 base::debug::(anonymous namespace)::StackDumpSignalHandler() > #2 0x7f28247aa330 <unknown> > #3 0x7f281cdb4c37 gsignal > #4 0x7f281cdb8028 abort > #5 0x7f2824a76c75 base::debug::BreakDebugger() > #6 0x7f2824a966c6 logging::LogMessage::~LogMessage() > #7 0x7f28202c9546 test_runner::GamepadController::Create() > #8 0x7f28202dd39c test_runner::TestInterfaces::SetDelegate() > #9 0x7f282030fe60 test_runner::WebViewTestProxyBase::~WebViewTestProxyBase() > #10 0x000000565b55 > _ZN11test_runner16WebViewTestProxyIN7content14RenderViewImplEJPNS1_22CompositorDependenciesERKNS1_5mojom16CreateViewParamsEEED0Ev > #11 0x7f2823d2843d > _ZN4base8internal9BindStateIMN7content12RenderWidgetEFvvEJ13scoped_refptrIS3_EEE7DestroyEPKNS0_13BindStateBaseE > #12 0x7f2824a79eff base::debug::TaskAnnotator::RunTask() > > > SetDelegate(nullptr) invokes test_runner::GamepadController::Create(nullptr). Thanks, it indeed does break - https://codereview.chromium.org/2738513004/ addressed GamepadController resetting. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
