Issue 898453002: HID-detection screen moved to screenContext.

Issue 898453002: HID-detection screen moved to screenContext. (Closed)

Created:
5 years, 10 months ago by merkulova

Modified:
5 years, 10 months ago

Reviewers:
Denis Kuznetsov (DE-MUC)

CC:
chromium-reviews, nkostylev+watch_chromium.org, dzhioev+watch_chromium.org, arv+watch_chromium.org, oshima+watch_chromium.org, stevenjb+watch_chromium.org, davemoore+watch_chromium.org

Base URL:
https://chromium.googlesource.com/chromium/src.git@master

Target Ref:
refs/pending/heads/master

Project:
chromium

Visibility:
Public.

More Reviews

Description

HID-detection screen moved to screenContext. BUG=456779 Committed: https://crrev.com/cc4feaa0828ee6d58c6a3eecf67e9d6bf72588a7 Cr-Commit-Position: refs/heads/master@{#315788} Committed: https://crrev.com/7a5f7055c7a3e4b0e6a678187bb92541c116ab76 Cr-Commit-Position: refs/heads/master@{#315981} Committed: https://crrev.com/dcb7d0c41387023d1fdbed07222608c2831e4952 Cr-Commit-Position: refs/heads/master@{#316458}

Patch Set 2 : Comments addressed. #

Patch Set 3 : Context variable introduced. Logic for combo-deviced fixed. #

Patch Set 4 : Init for context var added. #

Total comments: 7

Patch Set 6 : Shortcuts used. #

Patch Set 7 : GetContextEditor used. #

Total comments: 7

Patch Set 9 : Broken self removed. Rebase. #

Patch Set 10 : Rebase #

Created: 5 years, 10 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+988 lines, -804 lines)			Patch
A	chrome/browser/chromeos/login/screens/hid_detection_model.h	View	1 2	1 chunk	+49 lines, -0 lines	0 comments	Download
A	chrome/browser/chromeos/login/screens/hid_detection_model.cc	View	1 2	1 chunk	+38 lines, -0 lines	0 comments	Download
M	chrome/browser/chromeos/login/screens/hid_detection_screen.h	View		1 chunk	+183 lines, -10 lines	0 comments	Download
M	chrome/browser/chromeos/login/screens/hid_detection_screen.cc	View	1 2 3 4 5 6	1 chunk	+515 lines, -17 lines	0 comments	Download
D	chrome/browser/chromeos/login/screens/hid_detection_screen_actor.h	View		1 chunk	+0 lines, -46 lines	0 comments	Download
A	chrome/browser/chromeos/login/screens/hid_detection_view.h	View		1 chunk	+36 lines, -0 lines	0 comments	Download
M	chrome/browser/chromeos/login/ui/oobe_display.h	View	1 2 3 4 5 6 7 8 9	2 chunks	+2 lines, -2 lines	0 comments	Download
M	chrome/browser/chromeos/login/wizard_controller.h	View	1 2 3 4 5 6 7 8 9	5 chunks	+8 lines, -4 lines	0 comments	Download
M	chrome/browser/chromeos/login/wizard_controller.cc	View	1 2 3 4 5 6 7 8 9	4 chunks	+8 lines, -5 lines	0 comments	Download
M	chrome/browser/resources/chromeos/login/oobe_screen_hid_detection.css	View		1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/resources/chromeos/login/oobe_screen_hid_detection.html	View		1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/resources/chromeos/login/oobe_screen_hid_detection.js	View	1 2 3 4 5 6 7 8	6 chunks	+103 lines, -47 lines	0 comments	Download
M	chrome/browser/ui/webui/chromeos/login/hid_detection_screen_handler.h	View	1 2 3 4 5	2 chunks	+10 lines, -168 lines	0 comments	Download
M	chrome/browser/ui/webui/chromeos/login/hid_detection_screen_handler.cc	View	1 2 3 4 5 6 7	6 chunks	+25 lines, -496 lines	0 comments	Download
M	chrome/browser/ui/webui/chromeos/login/oobe_ui.h	View	1 2 3 4 5 6 7 8 9	2 chunks	+2 lines, -2 lines	0 comments	Download
M	chrome/browser/ui/webui/chromeos/login/oobe_ui.cc	View	1 2 3 4 5 6 7 8 9	3 chunks	+4 lines, -4 lines	0 comments	Download
M	chrome/chrome_browser_chromeos.gypi	View	1 2 3 4 5 6 7 8 9	1 chunk	+3 lines, -1 line	0 comments	Download

Messages

Total messages: 32 (8 generated)

Expand Messages | Collapse Messages | Show Generated Messages | Hide Generated Messages

Denis Kuznetsov (DE-MUC)

https://chromiumcodereview.appspot.com/898453002/diff/1/chrome/browser/chromeos/login/screens/hid_detection_model.h File chrome/browser/chromeos/login/screens/hid_detection_model.h (right): https://chromiumcodereview.appspot.com/898453002/diff/1/chrome/browser/chromeos/login/screens/hid_detection_model.h#newcode20 chrome/browser/chromeos/login/screens/hid_detection_model.h:20: static const char kContextKeyEnteredPartPincode[]; 1) PartialPincode / PartiallyEntered 2) ...

5 years, 10 months ago (2015-02-03 13:31:42 UTC) #2

merkulova

https://codereview.chromium.org/898453002/diff/1/chrome/browser/chromeos/login/screens/hid_detection_model.h File chrome/browser/chromeos/login/screens/hid_detection_model.h (right): https://codereview.chromium.org/898453002/diff/1/chrome/browser/chromeos/login/screens/hid_detection_model.h#newcode20 chrome/browser/chromeos/login/screens/hid_detection_model.h:20: static const char kContextKeyEnteredPartPincode[]; On 2015/02/03 13:31:42, Denis Kuznetsov ...

5 years, 10 months ago (2015-02-03 14:41:01 UTC) #3

Denis Kuznetsov (DE-MUC)

https://codereview.chromium.org/898453002/diff/1/chrome/browser/chromeos/login/screens/hid_detection_screen.cc File chrome/browser/chromeos/login/screens/hid_detection_screen.cc (right): https://codereview.chromium.org/898453002/diff/1/chrome/browser/chromeos/login/screens/hid_detection_screen.cc#newcode264 chrome/browser/chromeos/login/screens/hid_detection_screen.cc:264: if (mouse_is_pairing_ || keyboard_is_pairing_) On 2015/02/03 14:41:01, merkulova wrote: ...

5 years, 10 months ago (2015-02-04 13:18:41 UTC) #4

merkulova

Denis, PTAL https://codereview.chromium.org/898453002/diff/1/chrome/browser/chromeos/login/screens/hid_detection_screen.cc File chrome/browser/chromeos/login/screens/hid_detection_screen.cc (right): https://codereview.chromium.org/898453002/diff/1/chrome/browser/chromeos/login/screens/hid_detection_screen.cc#newcode264 chrome/browser/chromeos/login/screens/hid_detection_screen.cc:264: if (mouse_is_pairing_ || keyboard_is_pairing_) On 2015/02/04 13:18:41, ...

5 years, 10 months ago (2015-02-05 12:07:25 UTC) #5

Denis Kuznetsov (DE-MUC)

https://chromiumcodereview.appspot.com/898453002/diff/60001/chrome/browser/chromeos/login/screens/hid_detection_screen.cc File chrome/browser/chromeos/login/screens/hid_detection_screen.cc (right): https://chromiumcodereview.appspot.com/898453002/diff/60001/chrome/browser/chromeos/login/screens/hid_detection_screen.cc#newcode200 chrome/browser/chromeos/login/screens/hid_detection_screen.cc:200: context_.SetInteger(kContextKeyNumKeysEnteredPinCode, entered); You can use a shortcut : context_.SetInteger(...).SetString(....) ...

5 years, 10 months ago (2015-02-05 13:33:19 UTC) #6

merkulova

https://codereview.chromium.org/898453002/diff/60001/chrome/browser/chromeos/login/screens/hid_detection_screen.cc File chrome/browser/chromeos/login/screens/hid_detection_screen.cc (right): https://codereview.chromium.org/898453002/diff/60001/chrome/browser/chromeos/login/screens/hid_detection_screen.cc#newcode200 chrome/browser/chromeos/login/screens/hid_detection_screen.cc:200: context_.SetInteger(kContextKeyNumKeysEnteredPinCode, entered); On 2015/02/05 13:33:19, Denis Kuznetsov wrote: > ...

5 years, 10 months ago (2015-02-09 08:54:37 UTC) #7

Denis Kuznetsov (DE-MUC)

5 years, 10 months ago (2015-02-09 12:03:06 UTC) #8

Denis Kuznetsov (DE-MUC)

Ah, I see, I thought you've asked me a question and would later send the ...

5 years, 10 months ago (2015-02-11 12:29:39 UTC) #10

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/898453002/100001

5 years, 10 months ago (2015-02-11 12:34:19 UTC) #12

commit-bot: I haz the power

Try jobs failed on following builders: ios_rel_device_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ng/builds/9696)

5 years, 10 months ago (2015-02-11 12:40:53 UTC) #14

Denis Kuznetsov (DE-MUC)

generally LGTM, but check the comments. https://chromiumcodereview.appspot.com/898453002/diff/120001/chrome/browser/chromeos/login/screens/hid_detection_screen.cc File chrome/browser/chromeos/login/screens/hid_detection_screen.cc (right): https://chromiumcodereview.appspot.com/898453002/diff/120001/chrome/browser/chromeos/login/screens/hid_detection_screen.cc#newcode85 chrome/browser/chromeos/login/screens/hid_detection_screen.cc:85: GetContextEditor().SetBoolean(kContextKeyNumKeysEnteredExpected, false); Here ...

5 years, 10 months ago (2015-02-11 16:09:24 UTC) #17

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/898453002/120001

5 years, 10 months ago (2015-02-11 16:38:19 UTC) #19

commit-bot: I haz the power

Patchset 7 (id:??) landed as https://crrev.com/cc4feaa0828ee6d58c6a3eecf67e9d6bf72588a7 Cr-Commit-Position: refs/heads/master@{#315788}

5 years, 10 months ago (2015-02-11 17:47:15 UTC) #21

dewittj

A revert of this CL (patchset #7 id:120001) has been created in https://codereview.chromium.org/915133002/ by dewittj@chromium.org. ...

5 years, 10 months ago (2015-02-11 19:10:45 UTC) #22

Message was sent while issue was closed.

A revert of this CL (patchset #7 id:120001) has been created in
https://codereview.chromium.org/915133002/ by dewittj@chromium.org.

The reason for reverting is: I believe this is causing interactive_ui_tests
failures:

https://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%...

LoginUITest.OobeCatchException
LoginUITest.OobeNoExceptions


LoginUITest.OobeCatchException (run #4):
[ RUN      ] LoginUITest.OobeCatchException
[1:1:0211/103529:ERROR:memory_mapped_file.cc(42)] Couldn't open
/tmp/run_tha_testo_ZzaC/out/Debug/chrome_200_percent.pak
[1:1:0211/103529:ERROR:data_pack.cc(78)] Failed to mmap datapack
[31306:31306:0211/103529:INFO:audio_manager_pulse.cc(258)] Failed to connect to
the context.  Error: Connection refused
[31306:31306:0211/103529:ERROR:memory_mapped_file.cc(42)] Couldn't open
/tmp/run_tha_testo_ZzaC/out/Debug/chrome_200_percent.pak
[31306:31306:0211/103529:ERROR:data_pack.cc(78)] Failed to mmap datapack
Xlib:  extension "RANDR" missing on display ":9".
[31306:31306:0211/103529:WARNING:child_account_service.cc(322)] User instance
wasn't found while setting child account flag.
[31306:31306:0211/103529:WARNING:data_reduction_proxy_settings.cc(366)] SPDY
proxy OFF at startup
[31306:31306:0211/103529:ERROR:hid_detection_screen_handler.cc(35)]
HIDDetectionScreenHandler::HIDDetectionScreenHandler
[31306:31306:0211/103529:ERROR:hid_detection_screen_handler.cc(79)]
HIDDetectionScreenHandler::CheckIsScreenRequired 0x325bf0183920
[31306:31324:0211/103529:WARNING:freezer_cgroup_process_manager.cc(58)] Cgroup
freezer does not exist or is not writable. Unable to freeze renderer processes.
../../chrome/browser/chromeos/login/test/js_checker.cc:62: Failure
Value of: GetBool(expression)
  Actual: false
Expected: true
cr.ErrorStore.getInstance().length == 0
../../chrome/browser/chromeos/login/test/js_checker.cc:62: Failure
Value of: GetBool(expression)
  Actual: false
Expected: true
cr.ErrorStore.getInstance().length == 1
../../chrome/browser/chromeos/login/test/js_checker.cc:62: Failure
Value of: GetBool(expression)
  Actual: false
Expected: true
cr.ErrorStore.getInstance().length == 2
[7:8:0211/103533:WARNING:channel.cc(547)] Failed to send message to ack remove
remote endpoint (local ID 1, remote ID 1)
[7:8:0211/103533:WARNING:channel.cc(547)] Failed to send message to ack remove
remote endpoint (local ID 2147483648, remote ID 2)
[  FAILED  ] LoginUITest.OobeCatchException, where TypeParam =  and GetParam() =
 (4704 ms).

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/898453002/140001

5 years, 10 months ago (2015-02-12 14:53:13 UTC) #25

commit-bot: I haz the power

Patchset 8 (id:??) landed as https://crrev.com/7a5f7055c7a3e4b0e6a678187bb92541c116ab76 Cr-Commit-Position: refs/heads/master@{#315981}

5 years, 10 months ago (2015-02-12 15:38:24 UTC) #27

jonross

A revert of this CL (patchset #8 id:140001) has been created in https://codereview.chromium.org/921583005/ by jonross@chromium.org. ...

5 years, 10 months ago (2015-02-12 19:12:41 UTC) #28

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/898453002/180001

5 years, 10 months ago (2015-02-16 11:42:28 UTC) #30

commit-bot: I haz the power

5 years, 10 months ago (2015-02-16 12:32:17 UTC) #32

Message was sent while issue was closed.

Patchset 10 (id:??) landed as
https://crrev.com/dcb7d0c41387023d1fdbed07222608c2831e4952
Cr-Commit-Position: refs/heads/master@{#316458}

Expand Messages | Collapse Messages | Show Generated Messages | Hide Generated Messages