Broken ability to use "--expose-internals-for-testing" in an interactive content-shell session |
||||
Issue descriptionBased on https://crbug.com/587175#c10 it seems that 381b049a regressed the ability to use "--expose-internals-for-testing" in an interactive content-shell session.
,
Mar 14 2016
I've put together a fix for --expose-internals-for-testing at https://crrev.com/1799923002. I've gone through the other 4 switches and I am pretty sure that they were only active when used together with --run-layout-test. So - in theory they can stay this way / my previous CL didn't break anything here. OTOH, this inconsistent treatment the switches got is a bit surprising (i.e. wouldn't devs want to use --stable-release-mode and/or --enable-font-antialiasing when testing via interactive content_shell? is it ok if the answer is different than for --expose-internals-for-testing?).
,
Mar 15 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/756abda6b173d7b698bf43a0726e43eaab8c03dd commit 756abda6b173d7b698bf43a0726e43eaab8c03dd Author: lukasza <lukasza@chromium.org> Date: Tue Mar 15 21:58:50 2016 Make kExposeInternalsForTesting usable again for interactive content_shell. In https://crrev.com/1763113002 I've tried to separate layout-test-specific cmdline switches into a separate header file and handle some of them under LayoutTestContentBrowserClient::AppendExtraCommandLineSwitches rather than ShellContentBrowserClient::AppendExtraCommandLineSwitches. This was wrong - this meant that devs cannot pass these switches when debugging via content_shell running in interactive / non-layout-test mode. So - the current CL undoes part of https://crrev.com/1763113002 and restores processing of some of the switches back to ShellContentBrowserClient: - kExposeInternalsForTesting - this is useful for debugging layout tests (see https://crbug.com/594566 ). For now it is okay to leave propagation of the following switches in LayoutTestContentBrowserClient (because moving them in https://crrev.com/1763113002 was not a regression as explained below): - kStableReleaseMode - it only has effect if used together with --run-layout-test switch (i.e. see how ShellMainDelegate::BasicStartupComplete first checks switches::kRunLayoutTest before checking switches::kStableReleaseMode in a nested if). - kEnableLeakDetection only affects BlinkTestController and LayoutTestContentBrowserClient so it won't have any effect without --run-layout-test switch. - kEnableFontAntialiasing and kAlwaysUseComplexText only affects LayoutTestContentBrowserClient and LayoutTestRenderProcessObserver so it won't have any effect without --run-layout-test switch. BUG= 594566 Review URL: https://codereview.chromium.org/1799923002 Cr-Commit-Position: refs/heads/master@{#381330} [modify] https://crrev.com/756abda6b173d7b698bf43a0726e43eaab8c03dd/content/shell/browser/layout_test/layout_test_content_browser_client.cc [modify] https://crrev.com/756abda6b173d7b698bf43a0726e43eaab8c03dd/content/shell/browser/shell_content_browser_client.cc [modify] https://crrev.com/756abda6b173d7b698bf43a0726e43eaab8c03dd/content/shell/common/layout_test/layout_test_switches.cc [modify] https://crrev.com/756abda6b173d7b698bf43a0726e43eaab8c03dd/content/shell/common/layout_test/layout_test_switches.h [modify] https://crrev.com/756abda6b173d7b698bf43a0726e43eaab8c03dd/content/shell/common/shell_switches.cc [modify] https://crrev.com/756abda6b173d7b698bf43a0726e43eaab8c03dd/content/shell/common/shell_switches.h [modify] https://crrev.com/756abda6b173d7b698bf43a0726e43eaab8c03dd/content/shell/renderer/shell_render_view_observer.cc
,
Mar 15 2016
,
May 18 2016
|
||||
►
Sign in to add a comment |
||||
Comment 1 by lukasza@chromium.org
, Mar 14 2016