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

Issue 748164 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug

Blocking:
issue 728353



Sign in to add a comment

Disable chrome://settings-frame

Project Member Reported by steve...@chromium.org, Jul 24 2017

Issue description

We are aggressively removing old options UI code in 62.

chrome;//settings-frame is already largely broken in 61. We should disable the url.

 

Comment 1 by dbeam@chromium.org, Jul 24 2017

Labels: OS-Chrome
i think it's gone everywhere but CrOS, right?
Ah, you are correct.

Project Member

Comment 3 by bugdroid1@chromium.org, Jul 26 2017

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

commit eaa2161ed13ccee7456612363d5017222f7fdf25
Author: Steven Bennetts <stevenjb@chromium.org>
Date: Wed Jul 26 16:52:25 2017

Remove chrome://settings-frame on CrOS

This removes all remaining references to chrome://settings-frame
and removes the URL.

It does not remove the options code yet, see the list of issues
blocking  crbug.com/728353 

This also removes quick_unlock_configure_overlay.js  since it
included a reference to chrome://settings-frame and is only
used by the now fully deprecated options UI.

Bug:  748164 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I1b44f8a9c4b4eeefb972ae4b29eec95a8969220c
Reviewed-on: https://chromium-review.googlesource.com/583935
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489672}
[modify] https://crrev.com/eaa2161ed13ccee7456612363d5017222f7fdf25/chrome/browser/policy/policy_prefs_browsertest.cc
[modify] https://crrev.com/eaa2161ed13ccee7456612363d5017222f7fdf25/chrome/browser/ui/browser_navigator.cc
[modify] https://crrev.com/eaa2161ed13ccee7456612363d5017222f7fdf25/chrome/browser/ui/chrome_pages.cc
[modify] https://crrev.com/eaa2161ed13ccee7456612363d5017222f7fdf25/chrome/browser/ui/content_settings/content_setting_bubble_model_browsertest.cc
[modify] https://crrev.com/eaa2161ed13ccee7456612363d5017222f7fdf25/chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc
[modify] https://crrev.com/eaa2161ed13ccee7456612363d5017222f7fdf25/chrome/browser/ui/webui/options/options_ui.cc
[delete] https://crrev.com/247d8bf7a33ef9c33a4ee25089f6adc16ed84272/chrome/browser/ui/webui/sync_setup_browsertest.js
[modify] https://crrev.com/eaa2161ed13ccee7456612363d5017222f7fdf25/chrome/common/extensions/api/_api_features.json
[modify] https://crrev.com/eaa2161ed13ccee7456612363d5017222f7fdf25/chrome/common/url_constants.cc
[modify] https://crrev.com/eaa2161ed13ccee7456612363d5017222f7fdf25/chrome/common/url_constants.h
[modify] https://crrev.com/eaa2161ed13ccee7456612363d5017222f7fdf25/chrome/test/data/policy/policy_test_cases.json
[modify] https://crrev.com/eaa2161ed13ccee7456612363d5017222f7fdf25/chrome/test/data/webui/BUILD.gn
[modify] https://crrev.com/eaa2161ed13ccee7456612363d5017222f7fdf25/docs/chrome_settings.md
[modify] https://crrev.com/eaa2161ed13ccee7456612363d5017222f7fdf25/extensions/common/api/_api_features.json

Status: Fixed (was: Started)
Status: Assigned (was: Fixed)
ChromeOS: 9786.0.0, 62.0.3166.0. 

Content displayed for 1 second when entering chrome://settings-frame and then browser crash triggered afterwards. 

Always reproducible  

crash:
https://crash.corp.google.com/browse?stbtiq=976b7e3a04000000 

log: https://pantheon.corp.google.com/storage/browser/chromiumos-test-logs/bugfiles/cr/748164/ 

re-open. 
Status: Fixed (was: Assigned)
62.0.3166.0 was built @ 489161:
https://chromium.googlesource.com/chromium/src/+log/62.0.3165.0..62.0.3166.0?pretty=fuller&n=10000

The change in comment #3 landed @ 489672

Please re-verify with a more recent version.

Cc: mkarkada@chromium.org dhadd...@chromium.org
Status: Verified (was: Fixed)
chrome://settings-frame url is removed and no such browser crash seen on M62 (Chrome OS  9807.0.0, 62.0.3176.0 dev build).
Cc: dschuyler@chromium.org
Labels: -M-62 M-61 Merge-Request-61
chrome://settings-frame appears to be causing crashes on 61 also (issue 756140).

The easiest fix is just to merge this (or at least the parts that disable the URL).

Labels: Stability-Crash ReleaseBlock-Stable
Project Member

Comment 10 by sheriffbot@chromium.org, Aug 17 2017

Labels: -Merge-Request-61 Merge-Review-61 Hotlist-Merge-Review
This bug requires manual review: M61 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), ketakid@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Issue 756140 has been merged into this issue.
Labels: -Merge-Review-61 Merge-Approved-61
Approving merge to M61 Chrome OS.
Project Member

Comment 13 by bugdroid1@chromium.org, Aug 17 2017

Labels: -merge-approved-61 merge-merged-3163
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e4603e4609cf5c2f49732a71d2de81e56730d708

commit e4603e4609cf5c2f49732a71d2de81e56730d708
Author: Steven Bennetts <stevenjb@chromium.org>
Date: Thu Aug 17 17:52:04 2017

Remove chrome://settings-frame on CrOS

This removes all remaining references to chrome://settings-frame
and removes the URL.

It does not remove the options code yet, see the list of issues
blocking  crbug.com/728353 

This also removes quick_unlock_configure_overlay.js  since it
included a reference to chrome://settings-frame and is only
used by the now fully deprecated options UI.

TBR=stevenjb@chromium.org

(cherry picked from commit eaa2161ed13ccee7456612363d5017222f7fdf25)

Bug:  748164 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I1b44f8a9c4b4eeefb972ae4b29eec95a8969220c
Reviewed-on: https://chromium-review.googlesource.com/583935
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#489672}
Reviewed-on: https://chromium-review.googlesource.com/619230
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#630}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/e4603e4609cf5c2f49732a71d2de81e56730d708/chrome/browser/policy/policy_prefs_browsertest.cc
[modify] https://crrev.com/e4603e4609cf5c2f49732a71d2de81e56730d708/chrome/browser/ui/browser_navigator.cc
[modify] https://crrev.com/e4603e4609cf5c2f49732a71d2de81e56730d708/chrome/browser/ui/chrome_pages.cc
[modify] https://crrev.com/e4603e4609cf5c2f49732a71d2de81e56730d708/chrome/browser/ui/content_settings/content_setting_bubble_model_browsertest.cc
[modify] https://crrev.com/e4603e4609cf5c2f49732a71d2de81e56730d708/chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc
[modify] https://crrev.com/e4603e4609cf5c2f49732a71d2de81e56730d708/chrome/browser/ui/webui/options/options_ui.cc
[delete] https://crrev.com/c0b1d49f210a826355499a9dc84ce78d06b2da0b/chrome/browser/ui/webui/sync_setup_browsertest.js
[modify] https://crrev.com/e4603e4609cf5c2f49732a71d2de81e56730d708/chrome/common/extensions/api/_api_features.json
[modify] https://crrev.com/e4603e4609cf5c2f49732a71d2de81e56730d708/chrome/common/url_constants.cc
[modify] https://crrev.com/e4603e4609cf5c2f49732a71d2de81e56730d708/chrome/common/url_constants.h
[modify] https://crrev.com/e4603e4609cf5c2f49732a71d2de81e56730d708/chrome/test/data/policy/policy_test_cases.json
[modify] https://crrev.com/e4603e4609cf5c2f49732a71d2de81e56730d708/chrome/test/data/webui/BUILD.gn
[modify] https://crrev.com/e4603e4609cf5c2f49732a71d2de81e56730d708/docs/chrome_settings.md
[modify] https://crrev.com/e4603e4609cf5c2f49732a71d2de81e56730d708/extensions/common/api/_api_features.json

Note: in order for the branch to compile, we also need to merge:

https://codereview.chromium.org/2933653002

commit aeb9d32b6c3329f1ee586f48294e625a19a3c31c

Project Member

Comment 15 by bugdroid1@chromium.org, Aug 17 2017

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

commit bcd8b8519555f32b1e2491faa2c7b3309e82472a
Author: Steven Bennetts <stevenjb@chromium.org>
Date: Thu Aug 17 20:48:05 2017

Remove bidi checker tests and some of the backing code

R=agrieve@chromium.org
TBR=dbeam@chromium.org
BUG= 748164 

(cherry picked from commit aeb9d32b6c3329f1ee586f48294e625a19a3c31c)

Review-Url: https://codereview.chromium.org/2933653002
Cr-Original-Commit-Position: refs/heads/master@{#489361}
Change-Id: I2e87e14ae351554c6ee16b62fe6d184ee214e3f1
Reviewed-on: https://chromium-review.googlesource.com/619664
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#643}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[delete] https://crrev.com/84dbe01cba1270eff8ca42fdba1036e90f28d9ac/chrome/browser/ui/webui/bidi_checker_web_ui_test.cc
[delete] https://crrev.com/84dbe01cba1270eff8ca42fdba1036e90f28d9ac/chrome/browser/ui/webui/bidi_checker_web_ui_test.h
[modify] https://crrev.com/bcd8b8519555f32b1e2491faa2c7b3309e82472a/chrome/test/BUILD.gn

Sign in to add a comment