Add a runtime flag for interactive autocomplete

Add a runtime flag for interactive autocomplete (and trigger the necessary
features in WebKit to support this).

BUG=157661
TBR=thakis@chromium.org

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=166809

[Dan Beam, 2012-11-07 03:09:48]

jam@: chrome/renderer, chrome/browser/chrome_content_browser_client.cc, OWNERS
estade@: everything else

[Albert Bodenhamer, 2012-11-07 17:51:15]

We have several different flags now for different aspects of things we're
adding.  Does it make sense to combine some of them?


On Tue, Nov 6, 2012 at 7:09 PM, <dbeam@chromium.org> wrote:

> Reviewers: Evan Stade, John Abd-El-Malek,
>
> Message:
> jam@: chrome/renderer, chrome/browser/chrome_content_**browser_client.cc,
> OWNERS
> estade@: everything else
>
> Description:
> Add a feature flag for interactive autocomplete (and trigger the necessary
> features in WebKit to support this).
>
> BUG=157661
>
>
> Please review this at
https://chromiumcodereview.**appspot.com/11366127/<https://chromiumcodereview...
>
> SVN Base:
svn://svn.chromium.org/chrome/**trunk/src<http://svn.chromium.org/chrome/trunk/src>
>
> Affected files:
>   M chrome/app/generated_**resources.grd
>   M chrome/browser/about_flags.cc
>   M chrome/browser/chrome_content_**browser_client.cc
>   M chrome/common/chrome_switches.**h
>   M chrome/common/chrome_switches.**cc
>   M chrome/renderer/chrome_render_**process_observer.cc
>
>
> Index: chrome/app/generated_**resources.grd
> diff --git a/chrome/app/generated_**resources.grd b/chrome/app/generated_*
> *resources.grd
> index c294918622654a2983cd028e3883ea**2d56aec84c..**
> d8d8398104cfa0b64b1f1fd7855403**c5b7df4f82 100644
> --- a/chrome/app/generated_**resources.grd
> +++ b/chrome/app/generated_**resources.grd
> @@ -6834,6 +6834,13 @@ Keep your key file in a safe place. You will need
> it to create new versions of y
>          Enable passive gathering of performance-related metrics and
> events and provide the option to view this data in a graphical fashion.
>        </message>
>
> +      <message name="IDS_FLAGS_ENABLE_**INTERACTIVE_AUTOCOMPLETE_NAME"
> desc="Name of the flag that enables a tab modal interactive autocomplete
> UI.">
> +        Enable interactive autocomplete
> +      </message>
> +      <message
name="IDS_FLAGS_ENABLE_**INTERACTIVE_AUTOCOMPLETE_**DESCRIPTION"
> desc="Description of the flag to enable a tab-modal interactive
> autocomplete UI.">
> +        Enable a tab-modal interactive autocomplete UI that can be
> invoked by calling HTMLFormElement#**requestAutocomplete.
> +      </message>
> +
>        <!-- Crashes -->
>        <message name="IDS_CRASHES_TITLE" desc="Title for the
> chrome://crashes page.">
>          Crashes
> Index: chrome/browser/about_flags.cc
> diff --git a/chrome/browser/about_flags.**cc b/chrome/browser/about_flags.
> **cc
> index 54069736dbfcd5a155b22c173ae78c**2166ec61e5..**
> d6a73f8eac4c7386d46b74b5f98eec**b9fae4568d 100644
> --- a/chrome/browser/about_flags.**cc
> +++ b/chrome/browser/about_flags.**cc
> @@ -1078,6 +1078,13 @@ const Experiment kExperiments[] = {
>      kOsAll,
>      SINGLE_VALUE_TYPE(switches::**kWebIntentsInvocationEnabled)
>    },
> +  {
> +    "enable-interactive-**autocomplete",
> +    IDS_FLAGS_ENABLE_INTERACTIVE_**AUTOCOMPLETE_NAME,
> +    IDS_FLAGS_ENABLE_INTERACTIVE_**AUTOCOMPLETE_DESCRIPTION,
> +    kOsWin | kOsCrOS,
> +    SINGLE_VALUE_TYPE(switches::**kEnableInteractiveAutocomplete**)
> +  },
>  };
>
>  const Experiment* experiments = kExperiments;
> Index: chrome/browser/chrome_content_**browser_client.cc
> diff --git a/chrome/browser/chrome_**content_browser_client.cc
> b/chrome/browser/chrome_**content_browser_client.cc
> index 209762f28622bf5ff3226c49e1a9d4**e5f20c1658..**
> 087dd55e0227f6f811ae2092c4c989**35dc028d94 100644
> --- a/chrome/browser/chrome_**content_browser_client.cc
> +++ b/chrome/browser/chrome_**content_browser_client.cc
> @@ -923,6 +923,7 @@ void ChromeContentBrowserClient::**
> AppendExtraCommandLineSwitches**(
>        switches::**kEnableExperimentalExtensionAp**is,
>        switches::**kEnableFramelessConstrainedDia**logs,
>        switches::kEnableIPCFuzzing,
> +      switches::**kEnableInteractiveAutocomplete**,
>        switches::kEnableNaCl,
>        switches::kEnableNaClIPCProxy,
>        switches::**kEnablePasswordGeneration,
> Index: chrome/common/chrome_switches.**cc
> diff --git a/chrome/common/chrome_**switches.cc b/chrome/common/chrome_**
> switches.cc
> index 50e69e25b8f2b100b345f5e492de0b**7e14646050..**
> 0223fff858d797becf9eb9a48584b0**600c413d95 100644
> --- a/chrome/common/chrome_**switches.cc
> +++ b/chrome/common/chrome_**switches.cc
> @@ -488,6 +488,11 @@ const char kEnableDevToolsExperiments[]     =
> "enable-devtools-experiments";
>  // Enables Drive v2 API instead of Google Documents List API.
>  const char kEnableDriveV2Api[]              = "enable-drive-v2-api";
>
> +// Enables a tab-modal interactive autocomplete UI and a way to invoke
> this UI
> +// from WebKit by enabling HTMLFormElement#**requestAutocomplete (and
> associated
> +// autocomplete* events and logic).
> +const char kEnableInteractiveAutocomplete**[] = "enable-interactive-**
> autocomplete";
> +
>  // Enables extensions to be easily installed from sites other than the web
>  // store. Without this flag, they can still be installed, but must be
> manually
>  // dragged onto chrome://extensions/.
> Index: chrome/common/chrome_switches.**h
> diff --git a/chrome/common/chrome_**switches.h b/chrome/common/chrome_**
> switches.h
> index 75487aeb9bff1c2140b49e0963ea68**4f9267f3e8..**
> e856a742793298457fdb7d08cdb13e**bd0f13cf17 100644
> --- a/chrome/common/chrome_**switches.h
> +++ b/chrome/common/chrome_**switches.h
> @@ -141,6 +141,7 @@ extern const char kEnableContacts[];
>  extern const char kEnableCrxlessWebApps[];
>  extern const char kEnableDevToolsExperiments[];
>  extern const char kEnableDriveV2Api[];
> +extern const char kEnableInteractiveAutocomplete**[];
>  extern const char kEasyOffStoreExtensionInstall[**];
>  extern const char kEnableExperimentalExtensionAp**is[];
>  extern const char kEnableExtensionActivityLoggin**g[];
> Index: chrome/renderer/chrome_render_**process_observer.cc
> diff --git a/chrome/renderer/chrome_**render_process_observer.cc
> b/chrome/renderer/chrome_**render_process_observer.cc
> index 3e03c3f8217edfb507f87fb789a766**9ac107beb3..**
> 50459cb5eaa0fc666fb51f419b6cc4**f4a0c2d2fe 100644
> --- a/chrome/renderer/chrome_**render_process_observer.cc
> +++ b/chrome/renderer/chrome_**render_process_observer.cc
> @@ -166,6 +166,11 @@ ChromeRenderProcessObserver::**
> ChromeRenderProcessObserver(
>      base::StatisticsRecorder::set_**dump_on_exit(true);
>    }
>
> +#if defined(TOOLKIT_VIEWS)
> +  WebRuntimeFeatures::**enableRequestAutocomplete(
> +      command_line.HasSwitch(**switches::**kEnableInteractiveAutocomplete
> **));
> +#endif
> +
>    RenderThread* thread = RenderThread::Get();
>    resource_delegate_.reset(new RendererResourceDelegate());
>    thread->**SetResourceDispatcherDelegate(**resource_delegate_.get());
>
>
>


-- 
Albert Bodenhamer | Software Engineer | abodenha@chromium.<abodenha@google.com>
org

[Dan Beam, 2012-11-07 18:17:17]

On 2012/11/07 17:51:15, Albert Bodenhamer wrote:
> We have several different flags now for different aspects of things we're
> adding.  Does it make sense to combine some of them?

Which flags would you want to combine?

> 
> 
> On Tue, Nov 6, 2012 at 7:09 PM, <mailto:dbeam@chromium.org> wrote:
> 
> > Reviewers: Evan Stade, John Abd-El-Malek,
> >
> > Message:
> > jam@: chrome/renderer, chrome/browser/chrome_content_**browser_client.cc,
> > OWNERS
> > estade@: everything else
> >
> > Description:
> > Add a feature flag for interactive autocomplete (and trigger the necessary
> > features in WebKit to support this).
> >
> > BUG=157661
> >
> >
> > Please review this at
>
https://chromiumcodereview.**appspot.com/11366127/%3Chttps://chromiumcoderevi...>
> >
> > SVN Base:
>
svn://svn.chromium.org/chrome/**trunk/src<http://svn.chromium.org/chrome/trunk/src>
> >
> > Affected files:
> >   M chrome/app/generated_**resources.grd
> >   M chrome/browser/about_flags.cc
> >   M chrome/browser/chrome_content_**browser_client.cc
> >   M chrome/common/chrome_switches.**h
> >   M chrome/common/chrome_switches.**cc
> >   M chrome/renderer/chrome_render_**process_observer.cc
> >
> >
> > Index: chrome/app/generated_**resources.grd
> > diff --git a/chrome/app/generated_**resources.grd b/chrome/app/generated_*
> > *resources.grd
> > index c294918622654a2983cd028e3883ea**2d56aec84c..**
> > d8d8398104cfa0b64b1f1fd7855403**c5b7df4f82 100644
> > --- a/chrome/app/generated_**resources.grd
> > +++ b/chrome/app/generated_**resources.grd
> > @@ -6834,6 +6834,13 @@ Keep your key file in a safe place. You will need
> > it to create new versions of y
> >          Enable passive gathering of performance-related metrics and
> > events and provide the option to view this data in a graphical fashion.
> >        </message>
> >
> > +      <message name="IDS_FLAGS_ENABLE_**INTERACTIVE_AUTOCOMPLETE_NAME"
> > desc="Name of the flag that enables a tab modal interactive autocomplete
> > UI.">
> > +        Enable interactive autocomplete
> > +      </message>
> > +      <message
> name="IDS_FLAGS_ENABLE_**INTERACTIVE_AUTOCOMPLETE_**DESCRIPTION"
> > desc="Description of the flag to enable a tab-modal interactive
> > autocomplete UI.">
> > +        Enable a tab-modal interactive autocomplete UI that can be
> > invoked by calling HTMLFormElement#**requestAutocomplete.
> > +      </message>
> > +
> >        <!-- Crashes -->
> >        <message name="IDS_CRASHES_TITLE" desc="Title for the
> > chrome://crashes page.">
> >          Crashes
> > Index: chrome/browser/about_flags.cc
> > diff --git a/chrome/browser/about_flags.**cc b/chrome/browser/about_flags.
> > **cc
> > index 54069736dbfcd5a155b22c173ae78c**2166ec61e5..**
> > d6a73f8eac4c7386d46b74b5f98eec**b9fae4568d 100644
> > --- a/chrome/browser/about_flags.**cc
> > +++ b/chrome/browser/about_flags.**cc
> > @@ -1078,6 +1078,13 @@ const Experiment kExperiments[] = {
> >      kOsAll,
> >      SINGLE_VALUE_TYPE(switches::**kWebIntentsInvocationEnabled)
> >    },
> > +  {
> > +    "enable-interactive-**autocomplete",
> > +    IDS_FLAGS_ENABLE_INTERACTIVE_**AUTOCOMPLETE_NAME,
> > +    IDS_FLAGS_ENABLE_INTERACTIVE_**AUTOCOMPLETE_DESCRIPTION,
> > +    kOsWin | kOsCrOS,
> > +    SINGLE_VALUE_TYPE(switches::**kEnableInteractiveAutocomplete**)
> > +  },
> >  };
> >
> >  const Experiment* experiments = kExperiments;
> > Index: chrome/browser/chrome_content_**browser_client.cc
> > diff --git a/chrome/browser/chrome_**content_browser_client.cc
> > b/chrome/browser/chrome_**content_browser_client.cc
> > index 209762f28622bf5ff3226c49e1a9d4**e5f20c1658..**
> > 087dd55e0227f6f811ae2092c4c989**35dc028d94 100644
> > --- a/chrome/browser/chrome_**content_browser_client.cc
> > +++ b/chrome/browser/chrome_**content_browser_client.cc
> > @@ -923,6 +923,7 @@ void ChromeContentBrowserClient::**
> > AppendExtraCommandLineSwitches**(
> >        switches::**kEnableExperimentalExtensionAp**is,
> >        switches::**kEnableFramelessConstrainedDia**logs,
> >        switches::kEnableIPCFuzzing,
> > +      switches::**kEnableInteractiveAutocomplete**,
> >        switches::kEnableNaCl,
> >        switches::kEnableNaClIPCProxy,
> >        switches::**kEnablePasswordGeneration,
> > Index: chrome/common/chrome_switches.**cc
> > diff --git a/chrome/common/chrome_**switches.cc b/chrome/common/chrome_**
> > switches.cc
> > index 50e69e25b8f2b100b345f5e492de0b**7e14646050..**
> > 0223fff858d797becf9eb9a48584b0**600c413d95 100644
> > --- a/chrome/common/chrome_**switches.cc
> > +++ b/chrome/common/chrome_**switches.cc
> > @@ -488,6 +488,11 @@ const char kEnableDevToolsExperiments[]     =
> > "enable-devtools-experiments";
> >  // Enables Drive v2 API instead of Google Documents List API.
> >  const char kEnableDriveV2Api[]              = "enable-drive-v2-api";
> >
> > +// Enables a tab-modal interactive autocomplete UI and a way to invoke
> > this UI
> > +// from WebKit by enabling HTMLFormElement#**requestAutocomplete (and
> > associated
> > +// autocomplete* events and logic).
> > +const char kEnableInteractiveAutocomplete**[] = "enable-interactive-**
> > autocomplete";
> > +
> >  // Enables extensions to be easily installed from sites other than the web
> >  // store. Without this flag, they can still be installed, but must be
> > manually
> >  // dragged onto chrome://extensions/.
> > Index: chrome/common/chrome_switches.**h
> > diff --git a/chrome/common/chrome_**switches.h b/chrome/common/chrome_**
> > switches.h
> > index 75487aeb9bff1c2140b49e0963ea68**4f9267f3e8..**
> > e856a742793298457fdb7d08cdb13e**bd0f13cf17 100644
> > --- a/chrome/common/chrome_**switches.h
> > +++ b/chrome/common/chrome_**switches.h
> > @@ -141,6 +141,7 @@ extern const char kEnableContacts[];
> >  extern const char kEnableCrxlessWebApps[];
> >  extern const char kEnableDevToolsExperiments[];
> >  extern const char kEnableDriveV2Api[];
> > +extern const char kEnableInteractiveAutocomplete**[];
> >  extern const char kEasyOffStoreExtensionInstall[**];
> >  extern const char kEnableExperimentalExtensionAp**is[];
> >  extern const char kEnableExtensionActivityLoggin**g[];
> > Index: chrome/renderer/chrome_render_**process_observer.cc
> > diff --git a/chrome/renderer/chrome_**render_process_observer.cc
> > b/chrome/renderer/chrome_**render_process_observer.cc
> > index 3e03c3f8217edfb507f87fb789a766**9ac107beb3..**
> > 50459cb5eaa0fc666fb51f419b6cc4**f4a0c2d2fe 100644
> > --- a/chrome/renderer/chrome_**render_process_observer.cc
> > +++ b/chrome/renderer/chrome_**render_process_observer.cc
> > @@ -166,6 +166,11 @@ ChromeRenderProcessObserver::**
> > ChromeRenderProcessObserver(
> >      base::StatisticsRecorder::set_**dump_on_exit(true);
> >    }
> >
> > +#if defined(TOOLKIT_VIEWS)
> > +  WebRuntimeFeatures::**enableRequestAutocomplete(
> > +      command_line.HasSwitch(**switches::**kEnableInteractiveAutocomplete
> > **));
> > +#endif
> > +
> >    RenderThread* thread = RenderThread::Get();
> >    resource_delegate_.reset(new RendererResourceDelegate());
> >    thread->**SetResourceDispatcherDelegate(**resource_delegate_.get());
> >
> >
> >
> 
> 
> -- 
> Albert Bodenhamer | Software Engineer |
mailto:abodenha@chromium.<abodenha@google.com>
> org

[Dan Beam, 2012-11-07 23:52:42]

estade@/jam@: friendly ping

[Evan Stade, 2012-11-08 00:02:03]

lgtm, except remove references to tab modality (it's an odd detail to include)

[Dan Beam, 2012-11-08 00:11:27]

On 2012/11/08 00:02:03, Evan Stade wrote:
> lgtm, except remove references to tab modality (it's an odd detail to include)

Done.

[Ilya Sherman, 2012-11-08 05:44:35]

-jam, +Nico for chrome/ OWNERS review

(lgtm)

https://chromiumcodereview.appspot.com/11366127/diff/10002/chrome/app/generat...
File chrome/app/generated_resources.grd (right):

https://chromiumcodereview.appspot.com/11366127/diff/10002/chrome/app/generat...
chrome/app/generated_resources.grd:6842: +      </message>
nit: I'd recommend adding these near the other Autofill flag descriptions.

[Nico, 2012-11-08 16:50:00]

This sounds similar to http://codereview.chromium.org/11264053/ (which says
autocomplete, but so does the bug linked to from this issue here). Are there two
independent efforts to change autocomplete underway?

[Albert Bodenhamer, 2012-11-08 17:36:04]

Let's discuss in private email.


On Thu, Nov 8, 2012 at 8:50 AM, <thakis@chromium.org> wrote:

> This sounds similar to
http://codereview.chromium.**org/11264053/<http://codereview.chromium.org/112...
says
> autocomplete, but so does the bug linked to from this issue here). Are
> there two
> independent efforts to change autocomplete underway?
>
>
https://chromiumcodereview.**appspot.com/11366127/<https://chromiumcodereview...
>



-- 
Albert Bodenhamer | Software Engineer | abodenha@chromium.<abodenha@google.com>
org

[Albert Bodenhamer, 2012-11-08 20:28:44]

lgtm

The flags I was associating with this are actually unrelated.

[Dan Beam, 2012-11-08 20:32:22]

+thestig@ for chrome/OWNERS

[Lei Zhang, 2012-11-08 20:33:49]

lgtm

[Dan Beam, 2012-11-08 21:19:01]

On 2012/11/08 16:50:00, Nico wrote:
> This sounds similar to http://codereview.chromium.org/11264053/ (which says
> autocomplete, but so does the bug linked to from this issue here). Are there
two
> independent efforts to change autocomplete underway?

That review is for auto*fill* rather than auto*complete*.  They are similar
names and they have similar functions, but auto*complete* is the naming that the
HTML spec chose to use and the name of HTML attributes on forms and other
elements (input, maybe select?) that this CL affects the behavior of. 
Additionally, the new autofill UI isn't really related to the interactive
autocomplete UI (they're mutually exclusive, i.e. they should likely never both
be showing).

[Dan Beam, 2012-11-08 21:40:30]

https://chromiumcodereview.appspot.com/11366127/diff/10002/chrome/app/generat...
File chrome/app/generated_resources.grd (right):

https://chromiumcodereview.appspot.com/11366127/diff/10002/chrome/app/generat...
chrome/app/generated_resources.grd:6842: +      </message>
On 2012/11/08 05:44:35, Ilya Sherman wrote:
> nit: I'd recommend adding these near the other Autofill flag descriptions.

Done.  However, I think there will continue to be confusion regarding the
differences while we keep using these terms (autofill vs. autocomplete) almost
interchangeably :(.

[commit-bot: I haz the power, 2012-11-08 21:45:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dbeam@chromium.org/11366127/9002

[commit-bot: I haz the power, 2012-11-08 21:46:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dbeam@chromium.org/11366127/9002

[commit-bot: I haz the power, 2012-11-08 23:51:53]

Change committed as 166809