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

Issue 518583003: Don't take a fake UGI every time we execute Javascript. (Closed)

Created:
6 years, 3 months ago by Zeeshan Qureshi
Modified:
6 years, 3 months ago
CC:
cbentzel+watch_chromium.org, chromium-apps-reviews_chromium.org, chromium-reviews, creis+watch_chromium.org, darin-cc_chromium.org, davidben+watch_chromium.org, dominich+watch_chromium.org, extensions-reviews_chromium.org, gavinp+prer_chromium.org, jam, nasko+codewatch_chromium.org, tburkard+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

Don't take a fake UGI every time we execute Javascript. This change will make it so that a fake UserGestureIndicator is not set for every javascript execution request triggered by the browser. This was interfering with how the virtual keyboard handles programmatic focus() calls on load. This is part 2 of 3, depends on blink change: https://codereview.chromium.org/516753002 BUG=408426, 406336 Committed: https://crrev.com/3454e9cfacdab576d72257e47b0d14d9a7b95453 Cr-Commit-Position: refs/heads/master@{#293339}

Patch Set 1 #

Total comments: 5

Patch Set 2 : Address comments #

Patch Set 3 : Disable test temporarily #

Patch Set 4 : Split IPC into separate ones for production/testing #

Patch Set 5 : Update test expectation #

Total comments: 4

Patch Set 6 : Fix styling #

Unified diffs Side-by-side diffs Delta from patch set Stats (+72 lines, -14 lines) Patch
M chrome/browser/extensions/extension_messages_apitest.cc View 1 2 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/extensions/webstore_installer_test.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/prerender/prerender_browsertest.cc View 1 2 3 4 5 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/ui/browser_browsertest.cc View 1 2 3 4 5 5 chunks +9 lines, -6 lines 0 comments Download
M chrome/test/base/tracing_browsertest.cc View 1 2 3 4 3 chunks +4 lines, -4 lines 0 comments Download
M content/browser/frame_host/render_frame_host_impl.h View 1 chunk +2 lines, -0 lines 0 comments Download
M content/browser/frame_host/render_frame_host_impl.cc View 1 2 3 4 5 1 chunk +7 lines, -0 lines 0 comments Download
M content/common/frame_messages.h View 1 2 3 1 chunk +7 lines, -0 lines 0 comments Download
M content/public/browser/render_frame_host.h View 1 1 chunk +4 lines, -0 lines 0 comments Download
M content/public/test/browser_test_utils.cc View 1 chunk +1 line, -1 line 0 comments Download
M content/renderer/render_frame_impl.h View 1 2 3 4 5 2 chunks +8 lines, -0 lines 0 comments Download
M content/renderer/render_frame_impl.cc View 1 2 3 4 5 2 chunks +25 lines, -0 lines 0 comments Download

Messages

Total messages: 26 (7 generated)
Zeeshan Qureshi
zeeshanq@chromium.org changed reviewers: + kenrb@chromium.org, rbyers@chromium.org
6 years, 3 months ago (2014-08-29 03:37:39 UTC) #1
Zeeshan Qureshi
Rick, can you do a first pass. Ken can you see if the IPC update ...
6 years, 3 months ago (2014-08-29 03:37:40 UTC) #2
Rick Byers
Nice, this seems straight forward and a definite step in the right direction. Eventually I ...
6 years, 3 months ago (2014-08-29 13:38:38 UTC) #3
Zeeshan Qureshi
https://codereview.chromium.org/518583003/diff/1/content/public/browser/render_frame_host.h File content/public/browser/render_frame_host.h (right): https://codereview.chromium.org/518583003/diff/1/content/public/browser/render_frame_host.h#newcode76 content/public/browser/render_frame_host.h:76: // ONLY FOR TESTS: Same as above but adds ...
6 years, 3 months ago (2014-08-29 16:16:37 UTC) #4
Rick Byers
lgtm https://codereview.chromium.org/518583003/diff/1/content/renderer/render_frame_impl.cc File content/renderer/render_frame_impl.cc (right): https://codereview.chromium.org/518583003/diff/1/content/renderer/render_frame_impl.cc#newcode1228 content/renderer/render_frame_impl.cc:1228: bool set_ugi_for_tests) { On 2014/08/29 16:16:37, Zeeshan Qureshi ...
6 years, 3 months ago (2014-08-29 16:21:44 UTC) #5
kenrb
ipc lgtm
6 years, 3 months ago (2014-08-29 16:31:19 UTC) #6
Zeeshan Qureshi
Disabling ExtensionApiTest.MessagingUserGesture temporarily as the blink side is setting 2 fake UGIs at the moment. ...
6 years, 3 months ago (2014-08-29 18:39:06 UTC) #7
Zeeshan Qureshi
+jamesr for content/renderer
6 years, 3 months ago (2014-08-29 19:09:27 UTC) #9
jamesr
lgtm assuming you delete these quickly. i'd recommend running 'git cl format' since the formatting ...
6 years, 3 months ago (2014-09-03 20:22:11 UTC) #10
Zeeshan Qureshi
Sorry, this was an issue with mixing Eclipse and Vim. https://codereview.chromium.org/518583003/diff/80001/content/renderer/render_frame_impl.h File content/renderer/render_frame_impl.h (right): https://codereview.chromium.org/518583003/diff/80001/content/renderer/render_frame_impl.h#newcode523 ...
6 years, 3 months ago (2014-09-03 21:18:10 UTC) #11
Zeeshan Qureshi
+jochen Can you please have a look at chrome/ and content/(browser|public)
6 years, 3 months ago (2014-09-03 21:19:31 UTC) #13
jochen (gone - plz use gerrit)
lgtm
6 years, 3 months ago (2014-09-04 09:45:49 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/zeeshanq@chromium.org/518583003/100001
6 years, 3 months ago (2014-09-04 15:06:26 UTC) #16
commit-bot: I haz the power
Try jobs failed on following builders: linux_gpu_triggered_tests on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu_triggered_tests/builds/53529) mac_gpu_retina_triggered_tests on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu_retina_triggered_tests/builds/43633) win_chromium_rel_swarming ...
6 years, 3 months ago (2014-09-04 16:08:59 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/zeeshanq@chromium.org/518583003/100001
6 years, 3 months ago (2014-09-04 16:43:26 UTC) #20
commit-bot: I haz the power
Try jobs failed on following builders: mac_gpu_retina_triggered_tests on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu_retina_triggered_tests/builds/43691) mac_gpu_triggered_tests on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu_triggered_tests/builds/43597)
6 years, 3 months ago (2014-09-04 18:33:49 UTC) #22
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/zeeshanq@chromium.org/518583003/100001
6 years, 3 months ago (2014-09-04 20:46:30 UTC) #24
commit-bot: I haz the power
Committed patchset #6 (id:100001) as 0fadc9445b4b25c2fa0f561c6bbc6524d2022ee8
6 years, 3 months ago (2014-09-04 21:31:42 UTC) #25
commit-bot: I haz the power
6 years, 3 months ago (2014-09-10 03:32:46 UTC) #26
Message was sent while issue was closed.
Patchset 6 (id:??) landed as
https://crrev.com/3454e9cfacdab576d72257e47b0d14d9a7b95453
Cr-Commit-Position: refs/heads/master@{#293339}

Powered by Google App Engine
This is Rietveld 408576698