Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(434)

Issue 2839993002: [Android] Adding Smart GO/NEXT feature in Chrome (Closed)

Created:
12 months ago by AKVT
Modified:
10 months, 2 weeks ago
CC:
aelias_OOO_until_Jul13, agrieve+watch_chromium.org, blink-reviews, blink-reviews-api_chromium.org, chromium-reviews, creis+watch_chromium.org, darin-cc_chromium.org, dglazkov+blink, jam, kinuko+watch, mlamouri+watch-content_chromium.org, nasko+codewatch_chromium.org, Ted C
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

[Android] Adding Smart GO/NEXT feature in Chrome Smart Go/Next brings better user experience to the user during form submitting applications. For navigating between form elements, user can use NEXT/PREVIOUS button from IME without touching on individual fields. This will avoid unnecessary form submissions before filling or visiting all fields in the form. Additionally it will save user time and avoid redundant network requests before actually filling/attending entire fields in the form Design Document: https://docs.google.com/document/d/1h0diigZ8LUi7A3UKJ_zwNUbvNQoe-Nwr55_p6ivSPNg/edit?usp=sharing BUG=410785, 648986 Review-Url: https://codereview.chromium.org/2839993002 Cr-Commit-Position: refs/heads/master@{#479126} Committed: https://chromium.googlesource.com/chromium/src/+/8d80ad1410dcb23f55c323a97e57eed750a611dc

Patch Set 1 #

Total comments: 11

Patch Set 2 : Moved the triggering logic from WebView to WebFrame #

Patch Set 3 : Fixed WebViewTest build issues after restructure. #

Total comments: 6

Patch Set 4 : Rebased the patch along with review comment fixes. #

Total comments: 32

Patch Set 5 : Moved the Focus movement logic from IMC to FocusController and addressed other review comments. #

Total comments: 4

Patch Set 6 : Brought up blink enum to Java layer instead of boolean #

Total comments: 8

Patch Set 7 : Fixed FocusController and ImeAdapter related review comments. #

Total comments: 2

Patch Set 8 : Added few test related to Smart Go/next logic #

Total comments: 5

Patch Set 9 : testAdvanceFocusNextAndPrevious() is added for test coverage. #

Patch Set 10 : Reverted unnecessary touch in ImeTest. #

Total comments: 4

Patch Set 11 : Fixed ImeTest review comments. #

Total comments: 6

Patch Set 12 : Added inputType check in ImeTest #

Total comments: 30

Patch Set 13 : Fixed the ImeTest issues and added more coverage in WebViewTest #

Total comments: 9

Patch Set 14 : Fixed blink side review comments. #

Patch Set 15 : Rebased the patch for running android_fyi_perf bot. #

Patch Set 16 : Fixed the build break by replacing WebViewImpl object with WebViewBase #

Patch Set 17 : Rebased the patch and fixed ImeTest#testShowAndHideSoftInput #

Patch Set 18 : Rebased the patch for checking falky failures #

Patch Set 19 : Fixed testSelectActionBarClearedOnTappingOutsideInput failure #

Patch Set 20 : Rebased the patch and Fixed Webkit Layout test failure. #

Patch Set 21 : Rearranged the test HTML for fixing IME falky test. #

Patch Set 22 : Fixed BookmarksTest crash. #

Patch Set 23 : Fixed ImeTest#testSelectActionBarClearedOnTappingOutsideInput flaky failure #

Total comments: 5

Patch Set 24 : Preserved the DCHECK and fixed browser_tests failure #

Total comments: 3

Patch Set 25 : Rebased the patch #

Patch Set 26 : Rebased the patch from TOT #

Unified diffs Side-by-side diffs Delta from patch set Stats (+784 lines, -30 lines) Patch
M content/browser/android/ime_adapter_android.h View 1 2 3 4 5 1 chunk +4 lines, -0 lines 0 comments Download
M content/browser/android/ime_adapter_android.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 2 chunks +12 lines, -0 lines 0 comments Download
M content/common/frame_messages.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 1 chunk +6 lines, -0 lines 0 comments Download
M content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 3 chunks +24 lines, -8 lines 0 comments Download
M content/public/android/java/src/org/chromium/content/browser/input/ImeUtils.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 3 chunks +15 lines, -14 lines 0 comments Download
M content/public/android/javatests/src/org/chromium/content/browser/input/ImeActivityTestRule.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 3 chunks +7 lines, -3 lines 0 comments Download
M content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 2 chunks +64 lines, -0 lines 0 comments Download
M content/public/test/android/javatests/src/org/chromium/content/browser/test/util/TestInputMethodManagerWrapper.java View 1 2 3 4 5 6 7 8 9 10 11 12 5 chunks +8 lines, -0 lines 0 comments Download
M content/renderer/render_frame_impl.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 1 chunk +1 line, -0 lines 0 comments Download
M content/renderer/render_frame_impl.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 3 chunks +8 lines, -0 lines 0 comments Download
M content/test/data/android/input/input_forms.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 2 chunks +9 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/core/editing/InputMethodController.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 2 chunks +10 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/page/FocusController.h View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/core/page/FocusController.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 3 chunks +45 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/web/WebLocalFrameImpl.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/web/WebLocalFrameImpl.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 1 chunk +16 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/web/tests/WebViewTest.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 2 chunks +483 lines, -0 lines 0 comments Download
A third_party/WebKit/Source/web/tests/data/advance_focus_in_form_with_disabled_and_readonly_elements.html View 1 chunk +11 lines, -0 lines 0 comments Download
A third_party/WebKit/Source/web/tests/data/advance_focus_in_form_with_key_event_listeners.html View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +31 lines, -0 lines 0 comments Download
A third_party/WebKit/Source/web/tests/data/advance_focus_in_form_with_tabindex_elements.html View 1 chunk +12 lines, -0 lines 0 comments Download
M third_party/WebKit/public/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/public/platform/WebFocusType.h View 1 2 3 4 5 1 chunk +3 lines, -0 lines 0 comments Download
M third_party/WebKit/public/platform/WebTextInputType.h View 1 2 3 4 1 chunk +3 lines, -1 line 0 comments Download
M third_party/WebKit/public/web/WebLocalFrame.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 2 chunks +8 lines, -0 lines 0 comments Download

Messages

Total messages: 137 (38 generated)
AKVT
PTAL! In the meantime, could you help me to find the steps for running WebViewTest. ...
12 months ago (2017-04-25 14:38:50 UTC) #8
nasko
On 2017/04/25 14:38:50, AKVT wrote: > PTAL! > > In the meantime, could you help ...
12 months ago (2017-04-25 21:31:13 UTC) #9
nasko
Adding ekaramad@, who has done a lot of work recently on IME in the context ...
12 months ago (2017-04-25 21:32:46 UTC) #11
EhsanK
I am not a good reviewer for core blink changes (InputMethodController). But I left some ...
12 months ago (2017-04-25 22:43:08 UTC) #12
AKVT
On 2017/04/25 21:31:13, nasko wrote: > On 2017/04/25 14:38:50, AKVT wrote: > > PTAL! > ...
12 months ago (2017-04-26 00:29:01 UTC) #13
AKVT
PTAL! https://codereview.chromium.org/2839993002/diff/1/content/browser/android/ime_adapter_android.cc File content/browser/android/ime_adapter_android.cc (right): https://codereview.chromium.org/2839993002/diff/1/content/browser/android/ime_adapter_android.cc#newcode314 content/browser/android/ime_adapter_android.cc:314: rfh->Send(new FrameMsg_AdvanceFocusInForm(rfh->GetRoutingID(), forward)); On 2017/04/25 22:43:08, EhsanK wrote: ...
12 months ago (2017-04-26 10:33:04 UTC) #14
EhsanK
Thanks for addressing the issues! LGTM with suggestions. https://codereview.chromium.org/2839993002/diff/1/content/browser/android/ime_adapter_android.cc File content/browser/android/ime_adapter_android.cc (right): https://codereview.chromium.org/2839993002/diff/1/content/browser/android/ime_adapter_android.cc#newcode314 content/browser/android/ime_adapter_android.cc:314: rfh->Send(new ...
12 months ago (2017-04-26 21:31:59 UTC) #15
kochi
Did you rebase after the Blink big renaming? (which happened around early this month)
12 months ago (2017-04-27 02:03:17 UTC) #16
AKVT
On 2017/04/27 02:03:17, kochi wrote: > Did you rebase after the Blink big renaming? > ...
12 months ago (2017-04-27 03:44:31 UTC) #17
AKVT
PTAL! https://codereview.chromium.org/2839993002/diff/1/content/browser/android/ime_adapter_android.cc File content/browser/android/ime_adapter_android.cc (right): https://codereview.chromium.org/2839993002/diff/1/content/browser/android/ime_adapter_android.cc#newcode314 content/browser/android/ime_adapter_android.cc:314: rfh->Send(new FrameMsg_AdvanceFocusInForm(rfh->GetRoutingID(), forward)); On 2017/04/26 21:31:59, EhsanK wrote: ...
12 months ago (2017-04-27 15:07:02 UTC) #18
dcheng
Also, please address kochi's comment about naming in Blink. Blink mostly follows Chrome's naming conventions ...
12 months ago (2017-04-27 15:33:03 UTC) #19
Changwan Ryu
Please add some tests to ImeTest.java . https://codereview.chromium.org/2839993002/diff/60001/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java File content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java (right): https://codereview.chromium.org/2839993002/diff/60001/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java#newcode625 content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java:625: restartInput(); I ...
12 months ago (2017-04-27 16:34:54 UTC) #20
Changwan Ryu
also adding yosin@ for InputMethodController. yosin@ and kochi@, does it make sense for IMC to ...
12 months ago (2017-04-27 16:44:18 UTC) #22
yosin_UTC9
Could you provide design doc? I'm not sure what "Smart Go NEXT feature." Thanks in ...
12 months ago (2017-04-28 01:07:28 UTC) #23
kochi
On 2017/04/28 01:07:28, yosin_OOO_til_May_7 wrote: > Could you provide design doc? I'm not sure what ...
12 months ago (2017-04-28 01:14:33 UTC) #24
yosin_UTC9
On 2017/04/28 at 01:14:33, kochi wrote: > On 2017/04/28 01:07:28, yosin_OOO_til_May_7 wrote: > > Could ...
12 months ago (2017-04-28 03:30:16 UTC) #25
yosin_UTC9
https://codereview.chromium.org/2839993002/diff/60001/content/renderer/render_frame_impl.cc File content/renderer/render_frame_impl.cc (right): https://codereview.chromium.org/2839993002/diff/60001/content/renderer/render_frame_impl.cc#newcode2244 content/renderer/render_frame_impl.cc:2244: void RenderFrameImpl::OnAdvanceFocusInForm(bool forward) { Please avoid to use |bool| ...
12 months ago (2017-04-28 03:32:11 UTC) #26
AKVT
PTAL! I am in process of writing few tests in ImeTest.java and will upload in ...
11 months, 3 weeks ago (2017-05-03 14:35:12 UTC) #27
nasko
The C++ parts of content/ look mostly good. One recommendation on the IPC and I'll ...
11 months, 3 weeks ago (2017-05-03 23:53:25 UTC) #28
AKVT
PTAL! https://codereview.chromium.org/2839993002/diff/80001/content/common/frame_messages.h File content/common/frame_messages.h (right): https://codereview.chromium.org/2839993002/diff/80001/content/common/frame_messages.h#newcode967 content/common/frame_messages.h:967: // Tells the renderer to advance the focus ...
11 months, 3 weeks ago (2017-05-04 17:08:04 UTC) #29
nasko
Thanks! content/ (minus java parts) LGTM.
11 months, 3 weeks ago (2017-05-04 17:22:28 UTC) #30
Changwan Ryu
https://codereview.chromium.org/2839993002/diff/60001/content/public/android/java/src/org/chromium/content/browser/input/ImeUtils.java File content/public/android/java/src/org/chromium/content/browser/input/ImeUtils.java (right): https://codereview.chromium.org/2839993002/diff/60001/content/public/android/java/src/org/chromium/content/browser/input/ImeUtils.java#newcode125 content/public/android/java/src/org/chromium/content/browser/input/ImeUtils.java:125: boolean enterKeyAction = false; On 2017/05/03 14:35:12, AKVT wrote: ...
11 months, 3 weeks ago (2017-05-04 20:38:16 UTC) #31
yosin_UTC9
core/editing/InputMethodController.cpp lgtm
11 months, 2 weeks ago (2017-05-08 03:40:54 UTC) #32
kochi
Reviewed FocusController.*. Could you also add a link to your design doc in the CL ...
11 months, 2 weeks ago (2017-05-09 02:47:43 UTC) #33
kochi
On 2017/04/25 21:31:13, nasko wrote: > On 2017/04/25 14:38:50, AKVT wrote: > > PTAL! > ...
11 months, 2 weeks ago (2017-05-09 02:51:36 UTC) #34
AKVT
On 2017/05/09 02:51:36, kochi wrote: > On 2017/04/25 21:31:13, nasko wrote: > > On 2017/04/25 ...
11 months, 2 weeks ago (2017-05-09 08:14:22 UTC) #35
AKVT
PTAL! @changwan - May plan for testing in ImeTest is as below; 1) Modify the ...
11 months, 2 weeks ago (2017-05-09 11:05:53 UTC) #37
Changwan Ryu
Regarding testing approach, Please do NOT try to inject fake focus IDs. You can focus ...
11 months, 2 weeks ago (2017-05-09 14:43:58 UTC) #38
AKVT
On 2017/05/09 14:43:58, Changwan Ryu wrote: > Regarding testing approach, > > Please do NOT ...
11 months, 2 weeks ago (2017-05-09 14:52:49 UTC) #39
Changwan Ryu
On 2017/05/09 14:52:49, AKVT wrote: > On 2017/05/09 14:43:58, Changwan Ryu wrote: > > Regarding ...
11 months, 2 weeks ago (2017-05-09 15:02:33 UTC) #40
Changwan Ryu
BTW, could you describe your CL in more details in the commit subject / description? ...
11 months, 2 weeks ago (2017-05-09 17:07:46 UTC) #41
AKVT
PTAL! @changwan - Regarding test failure, could you give some initial inputs ? https://codereview.chromium.org/2839993002/diff/120001/content/public/android/java/src/org/chromium/content/browser/input/ImeUtils.java File ...
11 months, 2 weeks ago (2017-05-09 17:22:18 UTC) #44
Changwan Ryu
https://codereview.chromium.org/2839993002/diff/140001/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java File content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java (right): https://codereview.chromium.org/2839993002/diff/140001/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java#newcode43 content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java:43: EditorInfo.IME_FLAG_NO_FULLSCREEN | EditorInfo.IME_FLAG_NO_EXTRACT_UI; Instead of make another dependency on ...
11 months, 2 weeks ago (2017-05-09 18:12:16 UTC) #47
AKVT
PTAL! https://codereview.chromium.org/2839993002/diff/140001/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java File content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java (right): https://codereview.chromium.org/2839993002/diff/140001/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java#newcode43 content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java:43: EditorInfo.IME_FLAG_NO_FULLSCREEN | EditorInfo.IME_FLAG_NO_EXTRACT_UI; On 2017/05/09 18:12:16, Changwan Ryu ...
11 months, 2 weeks ago (2017-05-10 09:31:38 UTC) #48
Changwan Ryu
https://codereview.chromium.org/2839993002/diff/180001/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java File content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java (right): https://codereview.chromium.org/2839993002/diff/180001/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java#newcode423 content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java:423: // Form1 forward and backward focus Do we really ...
11 months, 2 weeks ago (2017-05-10 16:20:59 UTC) #49
kochi
FocusController.* lgtm
11 months, 2 weeks ago (2017-05-11 02:27:21 UTC) #50
AKVT
PTAL! https://codereview.chromium.org/2839993002/diff/180001/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java File content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java (right): https://codereview.chromium.org/2839993002/diff/180001/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java#newcode423 content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java:423: // Form1 forward and backward focus On 2017/05/10 ...
11 months, 2 weeks ago (2017-05-11 08:47:39 UTC) #51
Changwan Ryu
https://codereview.chromium.org/2839993002/diff/200001/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java File content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java (right): https://codereview.chromium.org/2839993002/diff/200001/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java#newcode437 content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java:437: Assert.assertTrue((EditorInfo.IME_ACTION_NEXT & getImeActions()) != 0); This is somewhat concerning. ...
11 months, 2 weeks ago (2017-05-11 09:10:52 UTC) #52
AKVT
I doubt we need to get the data from computeEditorInfo() and make available for querying. ...
11 months, 2 weeks ago (2017-05-11 09:48:49 UTC) #53
AKVT
PTAL! Added inputType check in tests.
11 months, 2 weeks ago (2017-05-11 10:09:35 UTC) #54
Changwan Ryu
https://codereview.chromium.org/2839993002/diff/200001/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java File content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java (right): https://codereview.chromium.org/2839993002/diff/200001/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java#newcode437 content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java:437: Assert.assertTrue((EditorInfo.IME_ACTION_NEXT & getImeActions()) != 0); On 2017/05/11 09:48:49, AKVT ...
11 months, 2 weeks ago (2017-05-11 12:04:38 UTC) #55
AKVT
@changwan - Please address my query and continue review on PS12 https://codereview.chromium.org/2839993002/diff/200001/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java File content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java (right): ...
11 months, 2 weeks ago (2017-05-11 12:16:44 UTC) #56
Changwan Ryu
On 2017/05/11 12:16:44, AKVT wrote: > @changwan - Please address my query and continue review ...
11 months, 2 weeks ago (2017-05-11 15:32:21 UTC) #57
Changwan Ryu
11 months, 2 weeks ago (2017-05-11 15:32:38 UTC) #58
AKVT
On 2017/05/11 15:32:21, Changwan Ryu wrote: > On 2017/05/11 12:16:44, AKVT wrote: > > @changwan ...
11 months, 2 weeks ago (2017-05-11 15:49:41 UTC) #59
Changwan Ryu
On 2017/05/11 15:49:41, AKVT wrote: > On 2017/05/11 15:32:21, Changwan Ryu wrote: > > On ...
11 months, 2 weeks ago (2017-05-11 15:55:26 UTC) #60
AKVT
On 2017/05/11 15:55:26, Changwan Ryu wrote: > On 2017/05/11 15:49:41, AKVT wrote: > > On ...
11 months, 2 weeks ago (2017-05-11 16:13:05 UTC) #61
Changwan Ryu
To answer the previous question, advancing the focus takes a round trip to the Blink ...
11 months, 2 weeks ago (2017-05-11 22:57:13 UTC) #62
AKVT
PTAL! https://codereview.chromium.org/2839993002/diff/220001/content/public/android/java/src/org/chromium/content/browser/input/ImeUtils.java File content/public/android/java/src/org/chromium/content/browser/input/ImeUtils.java (right): https://codereview.chromium.org/2839993002/diff/220001/content/public/android/java/src/org/chromium/content/browser/input/ImeUtils.java#newcode65 content/public/android/java/src/org/chromium/content/browser/input/ImeUtils.java:65: imeAction = EditorInfo.IME_ACTION_SEARCH; On 2017/05/11 22:57:12, Changwan Ryu ...
11 months, 2 weeks ago (2017-05-12 13:26:26 UTC) #63
Changwan Ryu
lgtm https://codereview.chromium.org/2839993002/diff/240001/content/public/android/javatests/src/org/chromium/content/browser/input/ImeActivityTestRule.java File content/public/android/javatests/src/org/chromium/content/browser/input/ImeActivityTestRule.java (right): https://codereview.chromium.org/2839993002/diff/240001/content/public/android/javatests/src/org/chromium/content/browser/input/ImeActivityTestRule.java#newcode573 content/public/android/javatests/src/org/chromium/content/browser/input/ImeActivityTestRule.java:573: return mFactory.initializeAndGet(view, imeAdapter, inputType, inputFlags, inputMode, On 2017/05/12 ...
11 months, 2 weeks ago (2017-05-12 22:22:02 UTC) #64
AKVT
@nasko and @dcheng - PTAL content/common/frame_messages.h changes as it needs SECURITY_OWNERS stamping. @tkent - PTAL ...
11 months, 2 weeks ago (2017-05-13 04:00:46 UTC) #65
dcheng
I think nasko already approved, but ipc LGTM as well. https://codereview.chromium.org/2839993002/diff/240001/content/public/android/javatests/src/org/chromium/content/browser/input/ImeActivityTestRule.java File content/public/android/javatests/src/org/chromium/content/browser/input/ImeActivityTestRule.java (right): https://codereview.chromium.org/2839993002/diff/240001/content/public/android/javatests/src/org/chromium/content/browser/input/ImeActivityTestRule.java#newcode573 ...
11 months, 2 weeks ago (2017-05-14 02:53:21 UTC) #66
tkent
The previous CL was reverted due to a power regression [1]. Does this CL address ...
11 months, 1 week ago (2017-05-14 23:24:59 UTC) #67
AKVT
PTAL! https://codereview.chromium.org/2839993002/diff/240001/third_party/WebKit/Source/core/page/FocusController.cpp File third_party/WebKit/Source/core/page/FocusController.cpp (right): https://codereview.chromium.org/2839993002/diff/240001/third_party/WebKit/Source/core/page/FocusController.cpp#newcode1102 third_party/WebKit/Source/core/page/FocusController.cpp:1102: // TODO(ajith.v) Extend it for Select element, Radio ...
11 months, 1 week ago (2017-05-15 06:40:42 UTC) #68
AKVT
On 2017/05/14 23:24:59, tkent wrote: > The previous CL was reverted due to a power ...
11 months, 1 week ago (2017-05-15 06:43:03 UTC) #69
AKVT
@tkent & @kochi - I initiated try jobs for measuring power.android_acceptance in android port. Please ...
11 months, 1 week ago (2017-05-15 13:06:14 UTC) #70
Changwan Ryu
On 2017/05/15 13:06:14, AKVT wrote: > @tkent & @kochi - I initiated try jobs for ...
11 months, 1 week ago (2017-05-16 04:56:06 UTC) #75
AKVT
On 2017/05/16 04:56:06, Changwan Ryu wrote: > On 2017/05/15 13:06:14, AKVT wrote: > > @tkent ...
11 months, 1 week ago (2017-05-16 06:52:38 UTC) #76
AKVT
@changwan, @kochi and @tkent - Suggested test run is completed by bot now. Most of ...
11 months, 1 week ago (2017-05-17 06:48:03 UTC) #77
Changwan Ryu
On 2017/05/17 06:48:03, AKVT wrote: > @changwan, @kochi and @tkent - Suggested test run is ...
11 months, 1 week ago (2017-05-17 14:29:50 UTC) #78
AKVT
On 2017/05/17 14:29:50, Changwan Ryu wrote: > On 2017/05/17 06:48:03, AKVT wrote: > > @changwan, ...
11 months, 1 week ago (2017-05-17 14:41:04 UTC) #79
Changwan Ryu
On 2017/05/17 14:41:04, AKVT wrote: > On 2017/05/17 14:29:50, Changwan Ryu wrote: > > On ...
11 months, 1 week ago (2017-05-18 01:11:05 UTC) #80
tkent
> tkent@, would it be ok to let this land and have it reverted in ...
11 months, 1 week ago (2017-05-18 23:23:32 UTC) #81
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2839993002/280001
11 months, 1 week ago (2017-05-19 03:47:04 UTC) #84
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_rel_ng/builds/430211) linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED, ...
11 months, 1 week ago (2017-05-19 04:28:04 UTC) #86
AKVT
On 2017/05/19 04:28:04, commit-bot: I haz the power wrote: > Try jobs failed on following ...
11 months, 1 week ago (2017-05-19 10:45:54 UTC) #87
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2839993002/300001
11 months, 1 week ago (2017-05-19 10:46:41 UTC) #90
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_rel_ng/builds/430421)
11 months, 1 week ago (2017-05-19 11:59:07 UTC) #92
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2839993002/300001
11 months, 1 week ago (2017-05-19 18:31:57 UTC) #94
commit-bot: I haz the power
Try jobs failed on following builders: android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_swarming_rel/builds/182517)
11 months, 1 week ago (2017-05-19 19:42:49 UTC) #96
AKVT
@changwan - After rebase I saw testShowAndHideSoftInput is failing, so I preserved resetUpdateSelectionList() method. PTAL!
10 months, 4 weeks ago (2017-05-30 14:54:46 UTC) #97
Changwan Ryu
On 2017/05/30 14:54:46, AKVT wrote: > @changwan - After rebase I saw testShowAndHideSoftInput is failing, ...
10 months, 4 weeks ago (2017-05-30 20:04:08 UTC) #98
AKVT
On 2017/05/30 20:04:08, Changwan Ryu wrote: > On 2017/05/30 14:54:46, AKVT wrote: > > @changwan ...
10 months, 3 weeks ago (2017-05-31 12:34:49 UTC) #99
AKVT
Unfortunately, none of the failures are not able to reproduce locally. ImeTest, AutoFillPopupTest etc. all ...
10 months, 3 weeks ago (2017-06-02 10:35:30 UTC) #100
AKVT
@kochi & @changwan - PTAL. I have removed a DCHECK from Element.cpp for solving launch ...
10 months, 3 weeks ago (2017-06-06 18:14:02 UTC) #101
kochi
@changwan - probably you are better at looking at this recent changes. could you take ...
10 months, 2 weeks ago (2017-06-09 03:53:25 UTC) #102
AKVT
PTAL - PS23 I have fixed the flaky failures in ImeTest.
10 months, 2 weeks ago (2017-06-09 05:11:21 UTC) #103
Changwan Ryu
I have hard time diff'ing patch 22 and patch 23 for the change you made. ...
10 months, 2 weeks ago (2017-06-09 15:40:04 UTC) #104
Changwan Ryu
https://codereview.chromium.org/2839993002/diff/440001/third_party/WebKit/Source/core/page/FocusController.cpp File third_party/WebKit/Source/core/page/FocusController.cpp (right): https://codereview.chromium.org/2839993002/diff/440001/third_party/WebKit/Source/core/page/FocusController.cpp#newcode1072 third_party/WebKit/Source/core/page/FocusController.cpp:1072: WebFocusType focus_type) { could you try adding element->GetDocument().UpdateStyleAndLayoutIgnorePendingStylesheets(); at ...
10 months, 2 weeks ago (2017-06-09 19:34:30 UTC) #105
AKVT
On 2017/06/09 15:40:04, Changwan Ryu wrote: > I have hard time diff'ing patch 22 and ...
10 months, 2 weeks ago (2017-06-10 03:34:29 UTC) #106
AKVT
On 2017/06/09 15:40:04, Changwan Ryu wrote: > I have hard time diff'ing patch 22 and ...
10 months, 2 weeks ago (2017-06-10 03:34:32 UTC) #107
AKVT
On 2017/06/09 15:40:04, Changwan Ryu wrote: > I have hard time diff'ing patch 22 and ...
10 months, 2 weeks ago (2017-06-10 03:34:34 UTC) #108
AKVT
On 2017/06/09 15:40:04, Changwan Ryu wrote: > I have hard time diff'ing patch 22 and ...
10 months, 2 weeks ago (2017-06-10 03:34:38 UTC) #109
AKVT
On 2017/06/09 15:40:04, Changwan Ryu wrote: > I have hard time diff'ing patch 22 and ...
10 months, 2 weeks ago (2017-06-10 03:34:39 UTC) #110
AKVT
@changwan - i will try to call UpdateStyleAndLayoutIgnorePendingStylesheets() and share result in next patch. Thank ...
10 months, 2 weeks ago (2017-06-10 03:37:56 UTC) #111
AKVT
PTAL! https://codereview.chromium.org/2839993002/diff/440001/third_party/WebKit/Source/core/dom/Element.cpp File third_party/WebKit/Source/core/dom/Element.cpp (left): https://codereview.chromium.org/2839993002/diff/440001/third_party/WebKit/Source/core/dom/Element.cpp#oldcode2897 third_party/WebKit/Source/core/dom/Element.cpp:2897: !GetDocument().NeedsLayoutTreeUpdateForNode(*this)); On 2017/06/09 15:40:04, Changwan Ryu wrote: > ...
10 months, 2 weeks ago (2017-06-12 12:21:24 UTC) #112
Changwan Ryu
https://codereview.chromium.org/2839993002/diff/460001/content/test/data/android/input/input_forms.html File content/test/data/android/input/input_forms.html (right): https://codereview.chromium.org/2839993002/diff/460001/content/test/data/android/input/input_forms.html#newcode25 content/test/data/android/input/input_forms.html:25: <input id="input_radio" type="radio" style="width:50px;height:50px" /><br/> could you explain why ...
10 months, 2 weeks ago (2017-06-12 20:47:24 UTC) #113
AKVT
@changwan - PTAL https://codereview.chromium.org/2839993002/diff/460001/content/test/data/android/input/input_forms.html File content/test/data/android/input/input_forms.html (right): https://codereview.chromium.org/2839993002/diff/460001/content/test/data/android/input/input_forms.html#newcode25 content/test/data/android/input/input_forms.html:25: <input id="input_radio" type="radio" style="width:50px;height:50px" /><br/> On ...
10 months, 2 weeks ago (2017-06-13 06:41:50 UTC) #114
Changwan Ryu
lgtm https://codereview.chromium.org/2839993002/diff/460001/content/test/data/android/input/input_forms.html File content/test/data/android/input/input_forms.html (right): https://codereview.chromium.org/2839993002/diff/460001/content/test/data/android/input/input_forms.html#newcode25 content/test/data/android/input/input_forms.html:25: <input id="input_radio" type="radio" style="width:50px;height:50px" /><br/> On 2017/06/13 06:41:49, ...
10 months, 2 weeks ago (2017-06-13 15:43:27 UTC) #115
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2839993002/460001
10 months, 2 weeks ago (2017-06-13 15:48:53 UTC) #118
commit-bot: I haz the power
Try jobs failed on following builders: ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/builds/236410)
10 months, 2 weeks ago (2017-06-13 15:52:28 UTC) #120
AKVT
On 2017/06/13 15:43:27, Changwan Ryu wrote: > lgtm > > https://codereview.chromium.org/2839993002/diff/460001/content/test/data/android/input/input_forms.html > File content/test/data/android/input/input_forms.html (right): ...
10 months, 2 weeks ago (2017-06-13 15:59:40 UTC) #121
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2839993002/480001
10 months, 2 weeks ago (2017-06-13 16:00:33 UTC) #124
commit-bot: I haz the power
Try jobs failed on following builders: mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_compile_dbg_ng/builds/440467) mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, ...
10 months, 2 weeks ago (2017-06-13 16:03:19 UTC) #126
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2839993002/500001
10 months, 2 weeks ago (2017-06-13 16:10:08 UTC) #129
commit-bot: I haz the power
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_arm64_dbg_recipe/builds/287907) android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, ...
10 months, 2 weeks ago (2017-06-13 17:00:10 UTC) #131
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2839993002/500001
10 months, 2 weeks ago (2017-06-13 19:36:01 UTC) #133
commit-bot: I haz the power
Committed patchset #26 (id:500001) as https://chromium.googlesource.com/chromium/src/+/8d80ad1410dcb23f55c323a97e57eed750a611dc
10 months, 2 weeks ago (2017-06-13 20:21:32 UTC) #136
kochi
10 months, 1 week ago (2017-06-15 01:47:17 UTC) #137
Message was sent while issue was closed.
A revert of this CL (patchset #26 id:500001) has been created in
https://codereview.chromium.org/2938123002/ by kochi@chromium.org.

The reason for reverting is: This caused several clusterfuzz bugs
(733197, 733218, 733282) - let me revert for
investigation on those..

Powered by Google App Engine
This is Rietveld 408576698