Remove text encoding UI

Remove text encoding UI

Text encoding UI is removed as a part of the project Eraser effort.
Manual encoding selection is not necessary any more as the new
encoding detector is turned on by default.

Request for manual encoding switching which may still exist
can be met by utilizing Chrome extension. See the bug for
detailed discussion.

Also removed the default encoding in Settings -> Customize fonts ->
Fonts and encoding.

Discussion at chrome-ui-review can be found here https://goo.gl/47cdcb

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

Committed: https://crrev.com/72125e2b16381f98e4bd44ae67240a736a648f5a
Cr-Commit-Position: refs/heads/master@{#415497}

[Jinsuk Kim, 2016-08-19 05:44:09]

Description was changed from

==========
Remove text encoding UI

Text encoding UI is removed as a part of the project Eraser effort.
Manual encoding selection is not necessary any more as the new
encoding detector is turned on by default.

Request for manual encoding switching can be met by utilizing
Chrome extension. See the bug for detailed discussion.

Also removed the default encoding in Settings -> Customize fonts ->
Fonts and encoding.

BUG=597488
==========

to

==========
Remove text encoding UI

Text encoding UI is removed as a part of the project Eraser effort.
Manual encoding selection is not necessary any more as the new
encoding detector is turned on by default.

Request for manual encoding switching can be met by utilizing
Chrome extension. See the bug for detailed discussion.

Also removed the default encoding in Settings -> Customize fonts ->
Fonts and encoding.

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

[Jinsuk Kim, 2016-08-19 05:45:50]

The CQ bit was checked by jinsukkim@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-19 05:46:05]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-19 08:39:49]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-19 08:39:50]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Jinsuk Kim, 2016-08-22 01:41:09]

Description was changed from

==========
Remove text encoding UI

Text encoding UI is removed as a part of the project Eraser effort.
Manual encoding selection is not necessary any more as the new
encoding detector is turned on by default.

Request for manual encoding switching can be met by utilizing
Chrome extension. See the bug for detailed discussion.

Also removed the default encoding in Settings -> Customize fonts ->
Fonts and encoding.

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

to

==========
Remove text encoding UI

Text encoding UI is removed as a part of the project Eraser effort.
Manual encoding selection is not necessary any more as the new
encoding detector is turned on by default.

Request for manual encoding switching which may still exist
can be met by utilizing Chrome extension. See the bug for 
detailed discussion.

Also removed the default encoding in Settings -> Customize fonts ->
Fonts and encoding.

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

[Jinsuk Kim, 2016-08-22 02:02:26]

Description was changed from

==========
Remove text encoding UI

Text encoding UI is removed as a part of the project Eraser effort.
Manual encoding selection is not necessary any more as the new
encoding detector is turned on by default.

Request for manual encoding switching which may still exist
can be met by utilizing Chrome extension. See the bug for 
detailed discussion.

Also removed the default encoding in Settings -> Customize fonts ->
Fonts and encoding.

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

to

==========
Remove text encoding UI

Text encoding UI is removed as a part of the project Eraser effort.
Manual encoding selection is not necessary any more as the new
encoding detector is turned on by default.

Request for manual encoding switching which may still exist
can be met by utilizing Chrome extension. See the bug for 
detailed discussion.

Also removed the default encoding in Settings -> Customize fonts ->
Fonts and encoding.

Discussion on chrome-ui-review@ can be found here
https://groups.google.com/a/google.com/forum/#!topic/chrome-ui-review/2FHdTZa...

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

[Jinsuk Kim, 2016-08-22 02:03:00]

Description was changed from

==========
Remove text encoding UI

Text encoding UI is removed as a part of the project Eraser effort.
Manual encoding selection is not necessary any more as the new
encoding detector is turned on by default.

Request for manual encoding switching which may still exist
can be met by utilizing Chrome extension. See the bug for 
detailed discussion.

Also removed the default encoding in Settings -> Customize fonts ->
Fonts and encoding.

Discussion on chrome-ui-review@ can be found here
https://groups.google.com/a/google.com/forum/#!topic/chrome-ui-review/2FHdTZa...

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

to

==========
Remove text encoding UI

Text encoding UI is removed as a part of the project Eraser effort.
Manual encoding selection is not necessary any more as the new
encoding detector is turned on by default.

Request for manual encoding switching which may still exist
can be met by utilizing Chrome extension. See the bug for 
detailed discussion.

Also removed the default encoding in Settings -> Customize fonts ->
Fonts and encoding.

Discussion on chrome-ui-review@ can be found here https://goo.gl/47cdcb

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

[Jinsuk Kim, 2016-08-22 02:03:27]

Description was changed from

==========
Remove text encoding UI

Text encoding UI is removed as a part of the project Eraser effort.
Manual encoding selection is not necessary any more as the new
encoding detector is turned on by default.

Request for manual encoding switching which may still exist
can be met by utilizing Chrome extension. See the bug for 
detailed discussion.

Also removed the default encoding in Settings -> Customize fonts ->
Fonts and encoding.

Discussion on chrome-ui-review@ can be found here https://goo.gl/47cdcb

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

to

==========
Remove text encoding UI

Text encoding UI is removed as a part of the project Eraser effort.
Manual encoding selection is not necessary any more as the new
encoding detector is turned on by default.

Request for manual encoding switching which may still exist
can be met by utilizing Chrome extension. See the bug for 
detailed discussion.

Also removed the default encoding in Settings -> Customize fonts ->
Fonts and encoding.

Discussion at chrome-ui-review can be found here https://goo.gl/47cdcb

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

[Jinsuk Kim, 2016-08-22 02:12:12]

jinsukkim@chromium.org changed reviewers:
+ grt@chromium.org, msw@chromium.org, sievers@chromium.org,
stevenjb@chromium.org, thestig@chromium.org

[Jinsuk Kim, 2016-08-22 02:12:14]

stevenjb@ please review changes in chrome/browser/resources
grt@ please review changes in chrome/app
msw@ please review changes in chrome/browser/ui
sievers@ please review changes in content
thestig@ please review changes in chrome/*.gypi,
chrome/browser/{app_controller.mm, browser_encoding_browsertest.cc,
character_encoding.*}

[grt (UTC plus 2), 2016-08-22 10:01:59]

chrome/app rubberstamp lgtm

[stevenjb, 2016-08-22 15:53:39]

lgtm

[msw, 2016-08-22 21:59:54]

It's great to see this cleanup! I have some comments/qs.

https://codereview.chromium.org/2254273003/diff/1/chrome/browser/character_en...
File chrome/browser/character_encoding.cc (left):

https://codereview.chromium.org/2254273003/diff/1/chrome/browser/character_en...
chrome/browser/character_encoding.cc:40: { IDC_ENCODING_UTF8, "UTF-8",
IDS_ENCODING_UNICODE },
Can we remove more here? Is GetCurrentDisplayEncodings still needed in
c/b/ui/webui/settings/font_handler.cc? Are the IDCs still needed at all?

https://codereview.chromium.org/2254273003/diff/1/chrome/browser/ui/browser.cc
File chrome/browser/ui/browser.cc (left):

https://codereview.chromium.org/2254273003/diff/1/chrome/browser/ui/browser.c...
chrome/browser/ui/browser.cc:833:
encoding_auto_detect_.SetValue(!encoding_auto_detect_.GetValue());
q: Can we remove Browser::encoding_auto_detect_?

https://codereview.chromium.org/2254273003/diff/1/chrome/browser/ui/browser.c...
chrome/browser/ui/browser.cc:846:
content::RecordAction(UserMetricsAction("OverrideEncoding"));
Please mark this action obsolete in tools/metrics/actions/actions.xml

https://codereview.chromium.org/2254273003/diff/1/chrome/browser/ui/browser.c...
chrome/browser/ui/browser.cc:855:
profile_->GetPrefs()->GetString(prefs::kRecentlySelectedEncoding),
If this isn't used, clear this pref for users and mark it deprecated (in
chrome/browser/prefs/browser_prefs.cc), then remove the constant after M55+.

https://codereview.chromium.org/2254273003/diff/1/chrome/browser/ui/browser_c...
File chrome/browser/ui/browser_command_controller.cc (left):

https://codereview.chromium.org/2254273003/diff/1/chrome/browser/ui/browser_c...
chrome/browser/ui/browser_command_controller.cc:477: // Page encoding commands
Can we remove these commands from chrome/app/chrome_command_ids.h?

https://codereview.chromium.org/2254273003/diff/1/chrome/browser/ui/toolbar/a...
File chrome/browser/ui/toolbar/app_menu_model.cc (left):

https://codereview.chromium.org/2254273003/diff/1/chrome/browser/ui/toolbar/a...
chrome/browser/ui/toolbar/app_menu_model.cc:158: current_tab->GetEncoding(),
command_id);
Can we remove WebContents::GetEncoding? (keeping a test accessor as needed?)

https://codereview.chromium.org/2254273003/diff/1/chrome/browser/ui/webui/opt...
File chrome/browser/ui/webui/options/font_settings_handler.cc (left):

https://codereview.chromium.org/2254273003/diff/1/chrome/browser/ui/webui/opt...
chrome/browser/ui/webui/options/font_settings_handler.cc:220:
pref_service->GetString(prefs::kStaticEncodings),
Ditto; can we clear/deprecate the pref and later remove kStaticEncodings?

[Jinsuk Kim, 2016-08-23 07:09:22]

https://codereview.chromium.org/2254273003/diff/1/chrome/browser/character_en...
File chrome/browser/character_encoding.cc (left):

https://codereview.chromium.org/2254273003/diff/1/chrome/browser/character_en...
chrome/browser/character_encoding.cc:40: { IDC_ENCODING_UTF8, "UTF-8",
IDS_ENCODING_UNICODE },
On 2016/08/22 21:59:53, msw wrote:
> Can we remove more here? Is GetCurrentDisplayEncodings still needed in
> c/b/ui/webui/settings/font_handler.cc? Are the IDCs still needed at all?

Good catch. Removed the method. The array is necessary for
|GetCanonicalEncodingNameByAliasName| which is still in use.

But this class can be slimmed down even further if the array gets simplified for
|GetCanonicalEncodingNameByAliasName| only or just leave it all to ICU. Then all
the IDC_* can go away too. Maybe we need a test in place first.

https://codereview.chromium.org/2254273003/diff/1/chrome/browser/ui/browser.cc
File chrome/browser/ui/browser.cc (left):

https://codereview.chromium.org/2254273003/diff/1/chrome/browser/ui/browser.c...
chrome/browser/ui/browser.cc:833:
encoding_auto_detect_.SetValue(!encoding_auto_detect_.GetValue());
On 2016/08/22 21:59:53, msw wrote:
> q: Can we remove Browser::encoding_auto_detect_?

Yes. Removed.

https://codereview.chromium.org/2254273003/diff/1/chrome/browser/ui/browser.c...
chrome/browser/ui/browser.cc:846:
content::RecordAction(UserMetricsAction("OverrideEncoding"));
On 2016/08/22 21:59:53, msw wrote:
> Please mark this action obsolete in tools/metrics/actions/actions.xml

Done.

https://codereview.chromium.org/2254273003/diff/1/chrome/browser/ui/browser.c...
chrome/browser/ui/browser.cc:855:
profile_->GetPrefs()->GetString(prefs::kRecentlySelectedEncoding),
On 2016/08/22 21:59:53, msw wrote:
> If this isn't used, clear this pref for users and mark it deprecated (in
> chrome/browser/prefs/browser_prefs.cc), then remove the constant after M55+.

Done.

https://codereview.chromium.org/2254273003/diff/1/chrome/browser/ui/browser_c...
File chrome/browser/ui/browser_command_controller.cc (left):

https://codereview.chromium.org/2254273003/diff/1/chrome/browser/ui/browser_c...
chrome/browser/ui/browser_command_controller.cc:477: // Page encoding commands
On 2016/08/22 21:59:54, msw wrote:
> Can we remove these commands from chrome/app/chrome_command_ids.h?

Please see my reply in character_encoding.cc

https://codereview.chromium.org/2254273003/diff/1/chrome/browser/ui/toolbar/a...
File chrome/browser/ui/toolbar/app_menu_model.cc (left):

https://codereview.chromium.org/2254273003/diff/1/chrome/browser/ui/toolbar/a...
chrome/browser/ui/toolbar/app_menu_model.cc:158: current_tab->GetEncoding(),
command_id);
On 2016/08/22 21:59:54, msw wrote:
> Can we remove WebContents::GetEncoding? (keeping a test accessor as needed?)

It's used here
https://cs.chromium.org/chromium/src/content/browser/web_contents/web_content...

But it's only for this java test
https://cs.chromium.org/chromium/src/chrome/android/javatests/src/org/chromiu...
Please see my TODO for replacing this test with
https://cs.chromium.org/chromium/src/chrome/browser/browser_encoding_browsert...
by enabling it. I'm planning to do this in a follow-up. This requires small
updates in non-UTF-8 test files that are not handled well by Rietveld. So I'd
rather handle it in separation.

https://codereview.chromium.org/2254273003/diff/1/chrome/browser/ui/webui/opt...
File chrome/browser/ui/webui/options/font_settings_handler.cc (left):

https://codereview.chromium.org/2254273003/diff/1/chrome/browser/ui/webui/opt...
chrome/browser/ui/webui/options/font_settings_handler.cc:220:
pref_service->GetString(prefs::kStaticEncodings),
On 2016/08/22 21:59:54, msw wrote:
> Ditto; can we clear/deprecate the pref and later remove kStaticEncodings?

Done.

[msw, 2016-08-23 18:06:27]

https://codereview.chromium.org/2254273003/diff/1/chrome/browser/character_en...
File chrome/browser/character_encoding.h (right):

https://codereview.chromium.org/2254273003/diff/1/chrome/browser/character_en...
chrome/browser/character_encoding.h:48: // FUNCTION IS NOT THREADSAFE. You must
run this function only in UI thread.
q: Is this no longer relevant?

https://codereview.chromium.org/2254273003/diff/20001/chrome/browser/characte...
File chrome/browser/character_encoding.h (right):

https://codereview.chromium.org/2254273003/diff/20001/chrome/browser/characte...
chrome/browser/character_encoding.h:15: static std::string
GetCanonicalEncodingNameByAliasName(
optional nit: this does not need to be a static class function.

https://codereview.chromium.org/2254273003/diff/20001/chrome/browser/chrome_c...
File chrome/browser/chrome_content_browser_client.cc (right):

https://codereview.chromium.org/2254273003/diff/20001/chrome/browser/chrome_c...
chrome/browser/chrome_content_browser_client.cc:2375:
web_prefs->uses_universal_detector = true;  // autodection is always on.
nit: 'detection'

https://codereview.chromium.org/2254273003/diff/20001/chrome/browser/ui/brows...
File chrome/browser/ui/browser.h (right):

https://codereview.chromium.org/2254273003/diff/20001/chrome/browser/ui/brows...
chrome/browser/ui/browser.h:38: #include "components/prefs/pref_member.h"
nit: maybe not needed?

https://codereview.chromium.org/2254273003/diff/20001/chrome/common/pref_name...
File chrome/common/pref_names.cc (right):

https://codereview.chromium.org/2254273003/diff/20001/chrome/common/pref_name...
chrome/common/pref_names.cc:222: // Deprecated 8/2016.
We should actually register and clear these two pref values from users' profile
preference stores in chrome/browser/prefs/browser_prefs.cc's
MigrateObsoleteProfilePrefs, and mark when they were deprecated there, so these
keys can be removed in about a year.

[Jinsuk Kim, 2016-08-23 22:46:14]

PTAL. Also added a unit test for GetCanonicalEncodingNameByAliasName.

https://codereview.chromium.org/2254273003/diff/1/chrome/browser/character_en...
File chrome/browser/character_encoding.h (right):

https://codereview.chromium.org/2254273003/diff/1/chrome/browser/character_en...
chrome/browser/character_encoding.h:48: // FUNCTION IS NOT THREADSAFE. You must
run this function only in UI thread.
On 2016/08/23 18:06:27, msw wrote:
> q: Is this no longer relevant?

That was necessary because |CanonicalEncodingMap| singleton was not threadsafe.
Now it's gone and it uses a const array only.

And I hope the linear search is okay since the array size is small.

https://codereview.chromium.org/2254273003/diff/20001/chrome/browser/characte...
File chrome/browser/character_encoding.h (right):

https://codereview.chromium.org/2254273003/diff/20001/chrome/browser/characte...
chrome/browser/character_encoding.h:15: static std::string
GetCanonicalEncodingNameByAliasName(
On 2016/08/23 18:06:27, msw wrote:
> optional nit: this does not need to be a static class function.

Did you mean to say this can be a simple helper function without an enclosing
class because now there's only one left?

https://codereview.chromium.org/2254273003/diff/20001/chrome/browser/chrome_c...
File chrome/browser/chrome_content_browser_client.cc (right):

https://codereview.chromium.org/2254273003/diff/20001/chrome/browser/chrome_c...
chrome/browser/chrome_content_browser_client.cc:2375:
web_prefs->uses_universal_detector = true;  // autodection is always on.
On 2016/08/23 18:06:27, msw wrote:
> nit: 'detection'

Done.

https://codereview.chromium.org/2254273003/diff/20001/chrome/browser/ui/brows...
File chrome/browser/ui/browser.h (right):

https://codereview.chromium.org/2254273003/diff/20001/chrome/browser/ui/brows...
chrome/browser/ui/browser.h:38: #include "components/prefs/pref_member.h"
On 2016/08/23 18:06:27, msw wrote:
> nit: maybe not needed?

Done.

https://codereview.chromium.org/2254273003/diff/20001/chrome/common/pref_name...
File chrome/common/pref_names.cc (right):

https://codereview.chromium.org/2254273003/diff/20001/chrome/common/pref_name...
chrome/common/pref_names.cc:222: // Deprecated 8/2016.
On 2016/08/23 18:06:27, msw wrote:
> We should actually register and clear these two pref values from users'
profile
> preference stores in chrome/browser/prefs/browser_prefs.cc's
> MigrateObsoleteProfilePrefs, and mark when they were deprecated there, so
these
> keys can be removed in about a year.

Thanks for clarifying - now I got it. Cleared the prefs in browser_prefs.cc. I
take it that these comments stay? Those defined in browser_prefs.cc are
commented with this but I don't see them in this file.

[msw, 2016-08-23 23:10:55]

lgtm with nits, a suggestion for followup, and one potentially important comment
(needing to register prefs before clearing them).

https://codereview.chromium.org/2254273003/diff/20001/chrome/browser/characte...
File chrome/browser/character_encoding.h (right):

https://codereview.chromium.org/2254273003/diff/20001/chrome/browser/characte...
chrome/browser/character_encoding.h:15: static std::string
GetCanonicalEncodingNameByAliasName(
On 2016/08/23 22:46:13, Jinsuk wrote:
> On 2016/08/23 18:06:27, msw wrote:
> > optional nit: this does not need to be a static class function.
> 
> Did you mean to say this can be a simple helper function without an enclosing
> class because now there's only one left?

Yes, your latest patch set has what I imagined, thanks!

https://codereview.chromium.org/2254273003/diff/40001/chrome/browser/characte...
File chrome/browser/character_encoding.h (right):

https://codereview.chromium.org/2254273003/diff/40001/chrome/browser/characte...
chrome/browser/character_encoding.h:10: #include "base/macros.h"
nit: not needed.

https://codereview.chromium.org/2254273003/diff/40001/chrome/browser/chrome_c...
File chrome/browser/chrome_content_browser_client.cc (right):

https://codereview.chromium.org/2254273003/diff/40001/chrome/browser/chrome_c...
chrome/browser/chrome_content_browser_client.cc:1304: std::string
ChromeContentBrowserClient::GetCanonicalEncodingNameByAliasName(
In a follow-up CL (shouldn't block this CL), it'd be nice to remove the
ContentBrowserClient::GetCanonicalEncodingNameByAliasName interface, and move
the helper function from chrome to content or some other lower layer that the 
two remaining callers (this file and
content/browser/web_contents/web_contents_impl.cc) can call directly.

https://codereview.chromium.org/2254273003/diff/40001/chrome/browser/prefs/br...
File chrome/browser/prefs/browser_prefs.cc (right):

https://codereview.chromium.org/2254273003/diff/40001/chrome/browser/prefs/br...
chrome/browser/prefs/browser_prefs.cc:743: // Added 8/2016
nit: trailing period.

https://codereview.chromium.org/2254273003/diff/40001/chrome/browser/prefs/br...
chrome/browser/prefs/browser_prefs.cc:744:
profile_prefs->ClearPref(prefs::kStaticEncodings);
I think you still need to actually register these prefs before being able to
clear them (but I may be wrong). You can register them in this file, which might
be a bit clearer than reinstating their registration in
chrome/browser/ui/prefs/prefs_tab_helper.cc

[Jinsuk Kim, 2016-08-24 08:23:42]

Thanks for reviewing. Please take another look at browser_prefs.cc to make sure
the change is what you expect.

https://codereview.chromium.org/2254273003/diff/40001/chrome/browser/characte...
File chrome/browser/character_encoding.h (right):

https://codereview.chromium.org/2254273003/diff/40001/chrome/browser/characte...
chrome/browser/character_encoding.h:10: #include "base/macros.h"
On 2016/08/23 23:10:55, msw wrote:
> nit: not needed.

Moved to .cc (for |arraysize|)

https://codereview.chromium.org/2254273003/diff/40001/chrome/browser/chrome_c...
File chrome/browser/chrome_content_browser_client.cc (right):

https://codereview.chromium.org/2254273003/diff/40001/chrome/browser/chrome_c...
chrome/browser/chrome_content_browser_client.cc:1304: std::string
ChromeContentBrowserClient::GetCanonicalEncodingNameByAliasName(
On 2016/08/23 23:10:55, msw wrote:
> In a follow-up CL (shouldn't block this CL), it'd be nice to remove the
> ContentBrowserClient::GetCanonicalEncodingNameByAliasName interface, and move
> the helper function from chrome to content or some other lower layer that the 
> two remaining callers (this file and
> content/browser/web_contents/web_contents_impl.cc) can call directly.

Left a TODO for a follow-up.

https://codereview.chromium.org/2254273003/diff/40001/chrome/browser/prefs/br...
File chrome/browser/prefs/browser_prefs.cc (right):

https://codereview.chromium.org/2254273003/diff/40001/chrome/browser/prefs/br...
chrome/browser/prefs/browser_prefs.cc:743: // Added 8/2016
On 2016/08/23 23:10:55, msw wrote:
> nit: trailing period.

Done.

https://codereview.chromium.org/2254273003/diff/40001/chrome/browser/prefs/br...
chrome/browser/prefs/browser_prefs.cc:744:
profile_prefs->ClearPref(prefs::kStaticEncodings);
On 2016/08/23 23:10:55, msw wrote:
> I think you still need to actually register these prefs before being able to
> clear them (but I may be wrong). You can register them in this file, which
might
> be a bit clearer than reinstating their registration in
> chrome/browser/ui/prefs/prefs_tab_helper.cc

Registered them in |RegisterProfilePrefs| in this class.

[msw, 2016-08-24 16:42:35]

lgtm

[Lei Zhang, 2016-08-24 18:30:29]

lgtm

[Jinsuk Kim, 2016-08-24 19:22:08]

jinsukkim@chromium.org changed reviewers:
+ asvitkine@google.com

[Jinsuk Kim, 2016-08-24 19:22:08]

asvitkine@ please review changes in tools/metrics/

[Jinsuk Kim, 2016-08-24 19:22:51]

jinsukkim@chromium.org changed reviewers:
+ jwd@chromium.org
- asvitkine@google.com

[Jinsuk Kim, 2016-08-24 19:22:52]

jwd@ please review changes in tools/metrics/.

[jwd, 2016-08-25 17:27:39]

LGTM with tiny nit.

https://codereview.chromium.org/2254273003/diff/60001/tools/metrics/actions/a...
File tools/metrics/actions/actions.xml (right):

https://codereview.chromium.org/2254273003/diff/60001/tools/metrics/actions/a...
tools/metrics/actions/actions.xml:12138: <obsolete>Obsolete as of
8/2016</obsolete>
tiny nit: 08/2016 please.

[Jinsuk Kim, 2016-08-26 22:53:57]

sievers@ would you review changes in content/? I'd like owner's LGTM. Thanks.

[no sievers, 2016-08-29 23:54:53]

https://codereview.chromium.org/2254273003/diff/60001/content/browser/web_con...
File content/browser/web_contents/web_contents_impl.cc (left):

https://codereview.chromium.org/2254273003/diff/60001/content/browser/web_con...
content/browser/web_contents/web_contents_impl.cc:2921: Send(new
ViewMsg_ResetPageEncodingToDefault(GetRoutingID()));
What about removal of the msgs and the renderer-side logic?

[Jinsuk Kim, 2016-08-30 01:03:41]

Patchset #5 (id:80001) has been deleted

[Jinsuk Kim, 2016-08-30 01:08:21]

jinsukkim@chromium.org changed reviewers:
+ rickyz@chromium.org

[Jinsuk Kim, 2016-08-30 01:08:23]

sievers@ PTAL

rickyz@ Would you review changes in content/common/view_messages.h?

https://codereview.chromium.org/2254273003/diff/60001/content/browser/web_con...
File content/browser/web_contents/web_contents_impl.cc (left):

https://codereview.chromium.org/2254273003/diff/60001/content/browser/web_con...
content/browser/web_contents/web_contents_impl.cc:2921: Send(new
ViewMsg_ResetPageEncodingToDefault(GetRoutingID()));
On 2016/08/29 23:54:53, sievers wrote:
> What about removal of the msgs and the renderer-side logic?

Done.

https://codereview.chromium.org/2254273003/diff/60001/tools/metrics/actions/a...
File tools/metrics/actions/actions.xml (right):

https://codereview.chromium.org/2254273003/diff/60001/tools/metrics/actions/a...
tools/metrics/actions/actions.xml:12138: <obsolete>Obsolete as of
8/2016</obsolete>
On 2016/08/25 17:27:39, jwd wrote:
> tiny nit: 08/2016 please.

Done.

[Jinsuk Kim, 2016-08-30 05:56:13]

The CQ bit was checked by jinsukkim@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-30 05:56:43]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-30 06:02:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-30 06:02:19]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  chromeos_x86-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  closure_compilation on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/closure_compila...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Jinsuk Kim, 2016-08-30 06:42:02]

The CQ bit was checked by jinsukkim@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-30 06:42:21]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-30 07:03:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-30 07:03:52]

Dry run: Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)

[Jinsuk Kim, 2016-08-30 10:02:17]

The CQ bit was checked by jinsukkim@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-30 10:02:37]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Jinsuk Kim, 2016-08-30 10:02:41]

Patchset #6 (id:120001) has been deleted

[Jinsuk Kim, 2016-08-30 10:02:56]

Patchset #6 (id:140001) has been deleted

[commit-bot: I haz the power, 2016-08-30 10:12:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-30 10:12:14]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[no sievers, 2016-08-30 20:39:18]

content lgtm but it looks like the encoding override code in blink is
potentially unused now too?

[rickyz (no longer on Chrome), 2016-08-30 22:16:33]

On 2016/08/30 at 20:39:18, sievers wrote:
> content lgtm but it looks like the encoding override code in blink is
potentially unused now too?

content/common/view_messages.h lgtm, always happy to remove IPCs :-)

[Jinsuk Kim, 2016-08-30 22:26:51]

On 2016/08/30 20:39:18, sievers wrote:
> content lgtm but it looks like the encoding override code in blink is
> potentially unused now too?

Let me handle it in a follow up CL.

[Jinsuk Kim, 2016-08-30 22:26:59]

The CQ bit was checked by jinsukkim@chromium.org

[Jinsuk Kim, 2016-08-30 22:26:59]

The patchset sent to the CQ was uploaded after l-g-t-m from
stevenjb@chromium.org, grt@chromium.org, thestig@chromium.org, msw@chromium.org,
jwd@chromium.org
Link to the patchset:
https://chromiumcodereview.appspot.com/2254273003/#ps160001 (title: "rebased")

[commit-bot: I haz the power, 2016-08-30 22:27:46]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-30 23:52:22]

Description was changed from

==========
Remove text encoding UI

Text encoding UI is removed as a part of the project Eraser effort.
Manual encoding selection is not necessary any more as the new
encoding detector is turned on by default.

Request for manual encoding switching which may still exist
can be met by utilizing Chrome extension. See the bug for 
detailed discussion.

Also removed the default encoding in Settings -> Customize fonts ->
Fonts and encoding.

Discussion at chrome-ui-review can be found here https://goo.gl/47cdcb

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

to

==========
Remove text encoding UI

Text encoding UI is removed as a part of the project Eraser effort.
Manual encoding selection is not necessary any more as the new
encoding detector is turned on by default.

Request for manual encoding switching which may still exist
can be met by utilizing Chrome extension. See the bug for 
detailed discussion.

Also removed the default encoding in Settings -> Customize fonts ->
Fonts and encoding.

Discussion at chrome-ui-review can be found here https://goo.gl/47cdcb

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

[commit-bot: I haz the power, 2016-08-30 23:52:24]

Committed patchset #6 (id:160001)

[commit-bot: I haz the power, 2016-08-30 23:53:39]

Description was changed from

==========
Remove text encoding UI

Text encoding UI is removed as a part of the project Eraser effort.
Manual encoding selection is not necessary any more as the new
encoding detector is turned on by default.

Request for manual encoding switching which may still exist
can be met by utilizing Chrome extension. See the bug for 
detailed discussion.

Also removed the default encoding in Settings -> Customize fonts ->
Fonts and encoding.

Discussion at chrome-ui-review can be found here https://goo.gl/47cdcb

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

to

==========
Remove text encoding UI

Text encoding UI is removed as a part of the project Eraser effort.
Manual encoding selection is not necessary any more as the new
encoding detector is turned on by default.

Request for manual encoding switching which may still exist
can be met by utilizing Chrome extension. See the bug for
detailed discussion.

Also removed the default encoding in Settings -> Customize fonts ->
Fonts and encoding.

Discussion at chrome-ui-review can be found here https://goo.gl/47cdcb

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

Committed: https://crrev.com/72125e2b16381f98e4bd44ae67240a736a648f5a
Cr-Commit-Position: refs/heads/master@{#415497}
==========

[commit-bot: I haz the power, 2016-08-30 23:53:41]

Patchset 6 (id:??) landed as
https://crrev.com/72125e2b16381f98e4bd44ae67240a736a648f5a
Cr-Commit-Position: refs/heads/master@{#415497}

[snowflake2396, 2017-03-11 08:37:44]

snowflake2396@gmail.com changed reviewers:
+ snowflake2396@gmail.com

[snowflake2396, 2017-03-11 08:37:46]

https://tinyspacesliving.com/best-sleeper-sofas-buying-guide/

[snowflake2396, 2017-03-11 10:37:48]

On 2017/03/11 10:36:36, snowflake2396 wrote:

[ikimotho, 2017-06-23 10:52:22]

On 2017/03/11 10:37:48, snowflake2396 wrote:
> On 2017/03/11 10:36:36, snowflake2396 wrote:

https://bestbuyhq.com/best-american-pressure-canner/
https://bestbuyhq.com/

[ikimotho, 2017-06-26 16:01:05]

On 2017/06/23 10:52:22, ikimotho wrote:
> On 2017/03/11 10:37:48, snowflake2396 wrote:
> > On 2017/03/11 10:36:36, snowflake2396 wrote:
> 
> https://bestbuyhq.com/best-american-pressure-canner/
> https://bestbuyhq.com/

[ikimotho, 2017-06-26 16:01:38]

On 2017/03/11 08:37:46, snowflake2396 wrote:
> https://tinyspacesliving.com/best-sleeper-sofas-buying-guide/

http://floralbeautyspot.com/