New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 785062 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Bug



Sign in to add a comment

[MD extensions] [error console] control pre-release error console

Project Member Reported by dschuyler@chromium.org, Nov 14 2017

Issue description

In chrome::extensions there are error buttons and an errors page that should be controlled by a --enable-error-console command line flag.

The current code appears to force the error console on in the dev channel and allow opt-in > dev channel.

IIUC this should instead be: 
- force disable on channels > dev
- allow opt-in on dev channel (disable by default)
 
Status: Started (was: Assigned)
CL at https://chromium-review.googlesource.com/c/chromium/src/+/770138
Summary: [MD extensions] [errors console] control pre-release error console (was: [MD extensions] control pre-release error console)
Summary: [MD extensions] [error console] control pre-release error console (was: [MD extensions] [errors console] control pre-release error console)
I think the current behavior is more-or-less correct - we want this on dev channel, since that's where it's been consistently.  It would also be nice to have this on > dev channel by opt-in.

I think the desired option is to disable it on dev channel?
#4 I'm flexible :)

Yeah the main goal would be to be able to explicitly disable it on any channel. Granted this would only be important on the dev channel currently, but if the default changes on other channels, I'd still want to be able to disable it on those channels. 

I think the ideal is what the chrome://flags normally do with choices of (Enable, Disable, Default). So that we can control it explicitly when desired. The Default would be enable on <= dev and disable on > dev.

WDYT?
Yep, that sounds reasonable to me.  All this should really need, I think, is changing ErrorConsole::IsEnabledForChromeExtensionsPage() to be:

bool ErrorConsole::IsEnabledForChromeExtensionsPage() const {
  if (!profile_->GetPrefs()->GetBoolean(prefs::kExtensionsUIDeveloperMode))
    return false;  // Only enabled in developer mode.

  // If the user specified a command line switch (either for enabling or disabling),
  // respect that.
  if (base::CommandLine::ForCurrentProcess()->HasSwitch(switches::kErrorConsole))
    return FeatureSwitch::error_console()->IsEnabled();

  // Enable by default on dev channel, disabled on other channels.
  return GetCurrentChannel() <= version_info::Channel::DEV;
}
Project Member

Comment 7 by bugdroid1@chromium.org, Nov 16 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ea8d15b6adaebdfd55db396b6623c8c27acf3f0b

commit ea8d15b6adaebdfd55db396b6623c8c27acf3f0b
Author: Dave Schuyler <dschuyler@chromium.org>
Date: Thu Nov 16 01:11:49 2017

[MD extensions] command line control of error-console

This CL makes the extensions error console page able to be enabled/disabled
using --enable-error-console and --disable-error-console command line switches.

Bug:  785062 
Change-Id: I4906be4effdc4b5c8a97451a1cb64688b8cbe699
Reviewed-on: https://chromium-review.googlesource.com/770138
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Commit-Queue: Dave Schuyler <dschuyler@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516933}
[modify] https://crrev.com/ea8d15b6adaebdfd55db396b6623c8c27acf3f0b/chrome/browser/extensions/error_console/error_console.cc
[modify] https://crrev.com/ea8d15b6adaebdfd55db396b6623c8c27acf3f0b/chrome/browser/extensions/error_console/error_console.h
[modify] https://crrev.com/ea8d15b6adaebdfd55db396b6623c8c27acf3f0b/chrome/browser/extensions/error_console/error_console_unittest.cc
[modify] https://crrev.com/ea8d15b6adaebdfd55db396b6623c8c27acf3f0b/extensions/common/feature_switch.cc
[modify] https://crrev.com/ea8d15b6adaebdfd55db396b6623c8c27acf3f0b/extensions/common/feature_switch.h

Status: Fixed (was: Started)

Sign in to add a comment