And when I say "blocks first run code cleanup" I mean enabling a tweak of import rules that makes first run twice as fast and recurring startups faster as well : https://crbug.com/555550#c114 so please consider this high priority.

hey gab@: the old (current) options UI seems to have used these prefs to persistently remember import choices (or at least clears them when an import is successful).  you may not have found this if you were only looking for the C++ kConstantName.

https://cs.chromium.org/chromium/src/chrome/browser/resources/options/import_data_overlay.js?type=cs&q=import_autofill_form_data&sq=package:chromium&l=186

when porting to Material Design, we re-used them (because that's what you do, when possible, in a port).

it probably doesn't matter much, but it's possible that if a user has chosen which things to import via the settings UI and not actually run the import, these prefs will be dropped if we use another set (without migration).

I'm sorry that somebody somewhere a long time ago re-used these prefs, but i can certainly understand how they could get confused.

if we change to a different pref path, we'll also need to update options.

Specifically, the old options set it here: https://cs.chromium.org/chromium/src/chrome/browser/resources/options/import_data_overlay.html?rcl=3de0c00180f67f978e98d3fe501d924a43440215&l=41

(and cleans it somewhere in import_data_overlay.js)

yep, the old options definitely uses these prefs

https://cs.chromium.org/chromium/src/chrome/browser/resources/options/import_data_overlay.html?rcl=3de0c00180f67f978e98d3fe501d924a43440215&l=41
https://cs.chromium.org/chromium/src/chrome/browser/resources/options/options.js?type=cs&q=options+%5C%5Bpref%5C%5D&sq=package:chromium&l=56
https://cs.chromium.org/chromium/src/chrome/browser/resources/options/pref_ui.js?type=cs&q=prefcheckbox&l=202

this is not a recent change

My main question is, how is that blocking the cleanup? Wouldn't any invocation of settings happen after first-run import, anyways?

(And the underlying question, of course, is what a minimal fix could look like :)

Thanks for the analysis, hadn't realized old code also had that dependency...

@groby: this is blocking the cleanup because the default is about to be flipped to not import by default -- which would mean the UI would have all checkboxes unticked by default per its dependency on the prefs... IMO it should just unconditionally have all checkboxes pre-ticked every time it comes up (without memory of its previous state through prefs -- this isn't setting like, it's merely a control for that UI).

So just to summarize: this is essentially a feature request for chrome://settings/importData (both MD and non-MD). The behavior should just be that all the boxes are always checked-by-default when you open the dialogue. We need someone to make the UI changes to not depend on the kImport* prefs, and instead just check the boxes by default.

We currently have turned off AutoImport on first run via Finch, but want to turn that on-by-default on trunk. Doing so would cause the checkboxes in chrome://settings/importData to be unchecked-by-default, due to their current dependence on kImport* prefs. So we are blocked on landing the change to make it the default behavior until this change is made.

Which team should own this feature request? The MD settings team, or the desktop team (cc'ing Rachel as well)?

I think it would be fine just to pre-select all checkboxes on chrome://settings/importData.  I doubt this will hamper anybody's workflow.  It would be a bigger deal on somewhere like chrome://settings/clearBrowserData (where folks rely on the same set of checkboxes being checked to instantly run a clear).

I just chatted with ainslie@ and he was OK with dropping the stickiness of these checkboxes (backed by these naughty prefs) because import is so uncommon.

Great, thanks Dan! So sounds like we're all on board from a desired-behavior standpoint.

I guess the question is: who should make the change? :) Is this something the MD settings team could take on?

I'd love it if the first run team could take this :)

We're pushing hard to finally make the cut to the new settings, and need all hands on deck for that. Do you have bandwidth available?

There actually isn't really a "first run team," per se. Gab worked on the disabling AutoImport stuff, but there's no team dedicated to first run, and Gab has said that he doesn't have bandwidth (or the relevant UI expertise) to make this change.

Rachel - could someone from the desktop team help out? See c#9 for a summary.

FYI, I started updating the MD Settings import dialog per comment #10, since I implemented it in the first place. 

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

commit 5a915bf6d281b1142802a87491a1d17e811f3a8b
Author: dpapad <dpapad@chromium.org>
Date: Mon Feb 27 21:05:23 2017

MD Settings: Stop using prefs to populate import data dialog.

BUG= 695611 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2717013002
Cr-Commit-Position: refs/heads/master@{#453332}

[modify] https://crrev.com/5a915bf6d281b1142802a87491a1d17e811f3a8b/chrome/browser/resources/settings/people_page/import_data_dialog.html
[modify] https://crrev.com/5a915bf6d281b1142802a87491a1d17e811f3a8b/chrome/browser/resources/settings/people_page/import_data_dialog.js
[modify] https://crrev.com/5a915bf6d281b1142802a87491a1d17e811f3a8b/chrome/test/data/webui/settings/import_data_dialog_test.js

Awesome! Thank you so much! Fixed?

I only removed from MD Settings. Someone else can take care of removing those prefs from the old Options, so re-opening.

Ah :(. Do we care about the old options?

gab@: it's the settings page that users see in current Chrome

additionally, we're now allowing users to import all data types and ignoring the policies potentially set by admins.

please verify with stakeholders that we want this behavior before I remove it from the old options page.

@ewald to discuss with enterprise team

if we do end up changing the semantics of the import prefs and policies (which seems quite possible), we should at least update the policy docs:

https://www.chromium.org/administrators/policy-list-3#ImportAutofillFormData

each one ends with "If enabled, this policy also affects the import dialog."

as of r453332, md-settings no longer respects these policies.

+blumberg (enterprise PM)

Changing policy semantics without long enough (6 to 12 months) warning is a no go from enterprise perspective. Nothing hurts Chrome in the enterprise sector more than pulling the rug under the admins legs no matter how small a rug it might seem.

In this particular case I must say the documentation is pretty vague about the disabled case but I verified that its current behavior not only prevents the action on the first run but prevents the user from ever using this feature. I think this behavior (properly) documented or not should be preserved if possible forever if not possible for a long enough deprecation period.

To be clear, removing this dependency is preventing us from landing a CL to turn off automatic import on first run by default for all users. This has *significant* performance wins across first run and subsequent startups.

It seems unfortunate to me that we would sacrifice those performance wins in order to preserve (what seems like unintended or at least undesirable behavior) for a related enterprise policy.

I'll set up some time for us to chat and see whether we can find a good path forward.

ewald@: if you're set on dropping some of this stuff from first run, why not JUST drop it from there but keep honoring them from the importData dialog in settings?  (and update their doc)

followup question: have you talked to the enterprise team about how admins feel about ... removing some of their abilities from first run?

My understanding is that admins can still enable autoimport during first run by turning on the pref. We're just changing the default value to be "off" now.

Re #25: I would be happy to decouple the two things, but that would require creating a new pref (either first first run or for manual import). That is one of the alternatives I think is worth exploring :)

Re. #26: ewald is correct, we're merely changing the default : doesn't affect any admin who specified it explicitly already.

One ugly thing we *could* do if decoupling the prefs is absolutely not possible:
 - Keep the prefs defaulting to "true".
 - On first run, query PrefService::Preference::IsDefaultValue() and if so use "false" instead of the PrefService's value...

There's no precedent I'm aware of for such an ugly hack but it would "work" (i.e. change the default on first run only...).

so i just spoke with gab@ offline

it seems like just changing the default (registration-time) value of the prefs here:
https://codereview.chromium.org/2705113005/diff/150001/chrome/browser/ui/browser_ui_prefs.cc

is a good way to not disable any master prefs / policy that admins have set, but also get a faster first run by default for MANY MANY users.

this is a super simple change and has very little risk nor downside, IMO.

it's also possible we could update how settings harnesses these pref values, but it has the potential to intentionally or unintentionally break some admins (if we ignore user prefs and only list to policy, we might break master prefs.  if we ignore both, we'll break both policy and master prefs).

the side effect on settings is simply that when showing the import data dialog (chrome://settings/importData), the checkboxes start unchecked by default.  this seems fine (if not even desired), because there's a fair chance a user accidentally presses Enter or accidentally taps OK before ready and accidentally imports too much.

sound good?  anybody have objections?

note: we'd also want to revert the CL that dpapad@ landed to continue respecting preferences and polices again in new, Material Design settings.

Agreed, given we've determined in issue 555550 that auto import makes recurring startups faster, it's arguably a good thing to default to checkboxes unticked in the manual import dialog (so user only imports what they truly intend). I can't imagine this introducing much friction per users typically doing this at most once.

If we *really* want to keep checkboxes ticked, the alternative we've discussed is to flip the registered default to false but only honor the pref in the UI, using "true" otherwise, if set through policy (ignores master prefs -- sounds fine given import master prefs are truly meant to drive auto import alone: I can't imagine an admin would care about the default ticked value of checkboxes in the manual import dialog but not outright prevent it through policy...)

Thanks everyone for driving quickly on this issue :)

Oops, we've determined that "*disabling* auto import makes recurring startups faster" :)

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

commit b1e4703e6fd4ba6bff38cc6d4d7a6c2945987141
Author: dbeam <dbeam@chromium.org>
Date: Wed Mar 01 03:23:15 2017

Revert of MD Settings: Stop using prefs to populate import data dialog. (patchset #2 id:20001 of https://codereview.chromium.org/2717013002/ )

Reason for revert:
We want to continue respecting policy and master prefs in the importData dialog.

Original issue's description:
> MD Settings: Stop using prefs to populate import data dialog.
>
> BUG= 695611 
> CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
>
> Review-Url: https://codereview.chromium.org/2717013002
> Cr-Commit-Position: refs/heads/master@{#453332}
> Committed: https://chromium.googlesource.com/chromium/src/+/5a915bf6d281b1142802a87491a1d17e811f3a8b

TBR=dpapad@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 695611 

Review-Url: https://codereview.chromium.org/2727513002
Cr-Commit-Position: refs/heads/master@{#453821}

[modify] https://crrev.com/b1e4703e6fd4ba6bff38cc6d4d7a6c2945987141/chrome/browser/resources/settings/people_page/import_data_dialog.html
[modify] https://crrev.com/b1e4703e6fd4ba6bff38cc6d4d7a6c2945987141/chrome/browser/resources/settings/people_page/import_data_dialog.js
[modify] https://crrev.com/b1e4703e6fd4ba6bff38cc6d4d7a6c2945987141/chrome/test/data/webui/settings/import_data_dialog_test.js

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

commit 0b1f71dfa6f7dc388e7b7bec334c030b9d203e6e
Author: dbeam <dbeam@chromium.org>
Date: Tue Mar 07 18:36:36 2017

Options/MD Settings: use new prefs to drive import data dialog

Previously, first run and the import data dialog used the same prefs.
This led to confusion and make it hard to change. This CL splits the
import data dialog (chrome://settings/importData) to use a different
set of prefs but continue to harness the same policies.

R=gab@chromium.org,dpapad@chromium.org
BUG= 695611 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2727813002
Cr-Commit-Position: refs/heads/master@{#455149}

[modify] https://crrev.com/0b1f71dfa6f7dc388e7b7bec334c030b9d203e6e/chrome/browser/extensions/api/settings_private/prefs_util.cc
[modify] https://crrev.com/0b1f71dfa6f7dc388e7b7bec334c030b9d203e6e/chrome/browser/policy/configuration_policy_handler_list_factory.cc
[modify] https://crrev.com/0b1f71dfa6f7dc388e7b7bec334c030b9d203e6e/chrome/browser/prefs/browser_prefs.cc
[modify] https://crrev.com/0b1f71dfa6f7dc388e7b7bec334c030b9d203e6e/chrome/browser/resources/options/import_data_overlay.html
[modify] https://crrev.com/0b1f71dfa6f7dc388e7b7bec334c030b9d203e6e/chrome/browser/resources/options/import_data_overlay.js
[modify] https://crrev.com/0b1f71dfa6f7dc388e7b7bec334c030b9d203e6e/chrome/browser/resources/settings/people_page/import_data_dialog.html
[modify] https://crrev.com/0b1f71dfa6f7dc388e7b7bec334c030b9d203e6e/chrome/browser/resources/settings/people_page/import_data_dialog.js
[modify] https://crrev.com/0b1f71dfa6f7dc388e7b7bec334c030b9d203e6e/chrome/browser/ui/webui/settings/md_settings_ui.cc
[modify] https://crrev.com/0b1f71dfa6f7dc388e7b7bec334c030b9d203e6e/chrome/browser/ui/webui/settings/md_settings_ui.h
[modify] https://crrev.com/0b1f71dfa6f7dc388e7b7bec334c030b9d203e6e/chrome/browser/ui/webui/settings/settings_import_data_handler.cc
[modify] https://crrev.com/0b1f71dfa6f7dc388e7b7bec334c030b9d203e6e/chrome/common/pref_names.cc
[modify] https://crrev.com/0b1f71dfa6f7dc388e7b7bec334c030b9d203e6e/chrome/common/pref_names.h
[modify] https://crrev.com/0b1f71dfa6f7dc388e7b7bec334c030b9d203e6e/chrome/test/data/policy/policy_test_cases.json
[modify] https://crrev.com/0b1f71dfa6f7dc388e7b7bec334c030b9d203e6e/chrome/test/data/webui/settings/import_data_dialog_test.js

the original description of this CL was a little misinformed, but I don't really care enough to fix it (you can if you want, gab@).  the title ended up being relevant enough.