Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(58)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 399303003: Eliminate use of sim unlock UI notifications (Closed)

Created:
6 years, 5 months ago by stevenjb
Modified:
6 years, 3 months ago
CC:
chromium-reviews, dbeam+watch-options_chromium.org, cbentzel+watch_chromium.org, nkostylev+watch_chromium.org, arv+watch_chromium.org, oshima+watch_chromium.org, stevenjb+watch_chromium.org, davemoore+watch_chromium.org
Project:
chromium
Visibility:
Public.

Description

Eliminate use of sim unlock UI notifications This includes some changes to the Fake Shill implementation to support enabling sim lock to test the UI. (Was also tested using pseudomodem). BUG=279351 For c/b/resources TBR=xiyuan@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=285666

Patch Set 1 #

Total comments: 7

Patch Set 2 : Rebase, feedback, fix test #

Total comments: 2

Patch Set 3 : Rebase, remove debug log #

Patch Set 4 : Rebase #

Patch Set 5 : Rebase #

Patch Set 6 : Rebase #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+157 lines, -175 lines) Patch
M chrome/browser/chrome_notification_types.h View 1 chunk +0 lines, -12 lines 0 comments Download
M chrome/browser/chromeos/sim_dialog_delegate.cc View 2 chunks +9 lines, -10 lines 0 comments Download
M chrome/browser/resources/chromeos/sim_unlock.js View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/resources/options/chromeos/internet_detail.js View 4 chunks +20 lines, -9 lines 0 comments Download
M chrome/browser/ui/webui/chromeos/sim_unlock_ui.cc View 12 chunks +0 lines, -69 lines 2 comments Download
M chrome/browser/ui/webui/options/chromeos/internet_options_handler.h View 4 chunks +3 lines, -11 lines 0 comments Download
M chrome/browser/ui/webui/options/chromeos/internet_options_handler.cc View 1 2 6 chunks +13 lines, -27 lines 0 comments Download
M chromeos/chromeos_switches.cc View 1 2 1 chunk +4 lines, -3 lines 0 comments Download
M chromeos/dbus/fake_shill_device_client.cc View 1 5 chunks +36 lines, -1 line 0 comments Download
M chromeos/dbus/fake_shill_manager_client.h View 1 chunk +3 lines, -0 lines 0 comments Download
M chromeos/dbus/fake_shill_manager_client.cc View 6 chunks +58 lines, -22 lines 0 comments Download
M chromeos/network/network_device_handler_unittest.cc View 1 4 chunks +11 lines, -10 lines 0 comments Download

Messages

Total messages: 20 (1 generated)
stevenjb
https://codereview.chromium.org/399303003/diff/1/chrome/browser/chromeos/net/network_portal_detector_impl.cc File chrome/browser/chromeos/net/network_portal_detector_impl.cc (right): https://codereview.chromium.org/399303003/diff/1/chrome/browser/chromeos/net/network_portal_detector_impl.cc#newcode289 chrome/browser/chromeos/net/network_portal_detector_impl.cc:289: VLOG(2) << "CaptivePortalState not found for: " << guid; ...
6 years, 5 months ago (2014-07-18 00:29:40 UTC) #1
armansito
lgtm with one nit. https://codereview.chromium.org/399303003/diff/1/chromeos/dbus/fake_shill_device_client.cc File chromeos/dbus/fake_shill_device_client.cc (right): https://codereview.chromium.org/399303003/diff/1/chromeos/dbus/fake_shill_device_client.cc#newcode156 chromeos/dbus/fake_shill_device_client.cc:156: new base::FundamentalValue(require)); nit: SetBoolean?
6 years, 5 months ago (2014-07-18 03:34:58 UTC) #2
stevenjb
https://codereview.chromium.org/399303003/diff/1/chromeos/dbus/fake_shill_device_client.cc File chromeos/dbus/fake_shill_device_client.cc (right): https://codereview.chromium.org/399303003/diff/1/chromeos/dbus/fake_shill_device_client.cc#newcode156 chromeos/dbus/fake_shill_device_client.cc:156: new base::FundamentalValue(require)); On 2014/07/18 03:34:58, armansito wrote: > nit: ...
6 years, 5 months ago (2014-07-18 17:39:23 UTC) #3
stevenjb
The CQ bit was checked by stevenjb@chromium.org
6 years, 5 months ago (2014-07-18 17:40:13 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/stevenjb@chromium.org/399303003/20001
6 years, 5 months ago (2014-07-18 17:41:42 UTC) #5
stevenjb
The CQ bit was unchecked by stevenjb@chromium.org
6 years, 5 months ago (2014-07-18 18:02:23 UTC) #6
pneubeck (no reviews)
https://codereview.chromium.org/399303003/diff/20001/chrome/browser/ui/webui/options/chromeos/internet_options_handler.cc File chrome/browser/ui/webui/options/chromeos/internet_options_handler.cc (right): https://codereview.chromium.org/399303003/diff/20001/chrome/browser/ui/webui/options/chromeos/internet_options_handler.cc#newcode511 chrome/browser/ui/webui/options/chromeos/internet_options_handler.cc:511: LOG(ERROR) << "SIM_LOCK_ENABLED: " << device->sim_lock_enabled(); debugging left over?
6 years, 5 months ago (2014-07-21 08:39:22 UTC) #7
stevenjb
https://codereview.chromium.org/399303003/diff/20001/chrome/browser/ui/webui/options/chromeos/internet_options_handler.cc File chrome/browser/ui/webui/options/chromeos/internet_options_handler.cc (right): https://codereview.chromium.org/399303003/diff/20001/chrome/browser/ui/webui/options/chromeos/internet_options_handler.cc#newcode511 chrome/browser/ui/webui/options/chromeos/internet_options_handler.cc:511: LOG(ERROR) << "SIM_LOCK_ENABLED: " << device->sim_lock_enabled(); On 2014/07/21 08:39:22, ...
6 years, 5 months ago (2014-07-22 18:10:06 UTC) #8
stevenjb
The CQ bit was checked by stevenjb@chromium.org
6 years, 5 months ago (2014-07-24 20:24:38 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/stevenjb@chromium.org/399303003/40001
6 years, 5 months ago (2014-07-24 20:27:43 UTC) #10
stevenjb
The CQ bit was unchecked by stevenjb@chromium.org
6 years, 5 months ago (2014-07-24 21:34:11 UTC) #11
stevenjb
The CQ bit was checked by stevenjb@chromium.org
6 years, 5 months ago (2014-07-24 21:35:08 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/stevenjb@chromium.org/399303003/60001
6 years, 5 months ago (2014-07-24 21:37:49 UTC) #13
stevenjb
The CQ bit was unchecked by stevenjb@chromium.org
6 years, 5 months ago (2014-07-24 22:22:10 UTC) #14
stevenjb
The CQ bit was checked by stevenjb@chromium.org
6 years, 5 months ago (2014-07-25 17:22:52 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/stevenjb@chromium.org/399303003/100001
6 years, 5 months ago (2014-07-25 17:24:16 UTC) #16
commit-bot: I haz the power
Change committed as 285666
6 years, 5 months ago (2014-07-25 20:40:00 UTC) #17
Evan Stade
https://codereview.chromium.org/399303003/diff/100001/chrome/browser/ui/webui/chromeos/sim_unlock_ui.cc File chrome/browser/ui/webui/chromeos/sim_unlock_ui.cc (left): https://codereview.chromium.org/399303003/diff/100001/chrome/browser/ui/webui/chromeos/sim_unlock_ui.cc#oldcode298 chrome/browser/ui/webui/chromeos/sim_unlock_ui.cc:298: strings.SetString("cancel", l10n_util::GetStringUTF16(IDS_CANCEL)); I believe this was removed by mistake
6 years, 3 months ago (2014-09-22 22:12:16 UTC) #19
stevenjb
6 years, 3 months ago (2014-09-23 17:23:48 UTC) #20
Message was sent while issue was closed. 
https://codereview.chromium.org/399303003/diff/100001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/chromeos/sim_unlock_ui.cc (left):

https://codereview.chromium.org/399303003/diff/100001/chrome/browser/ui/webui...
chrome/browser/ui/webui/chromeos/sim_unlock_ui.cc:298:
strings.SetString("cancel", l10n_util::GetStringUTF16(IDS_CANCEL));
On 2014/09/22 22:12:16, Evan Stade wrote:
> I believe this was removed by mistake

Yes, that looks correct, we eliminated the "cancel" callback, but not the button
in the UI so we should restore this. I will put up a CL shortly.

Powered by Google App Engine
This is Rietveld 408576698