Issue metadata
Sign in to add a comment
|
Regression:Black patch is seen while opening chrome://history/ page after changing search engine to Bing.
Reported by
vku...@etouch.net,
Apr 9 2018
|
||||||||||||||||||||||
Issue descriptionChrome Version: 67.0.3390.0 (Official Build)Revision ae47769a6d274cb8eeca0f8e7b9198d0b043975f-refs/heads/master@{#548636} 32/64-bit OS: Windows(7,8,8.1,10) What steps will reproduce the problem? (1)Launch chrome and navigate to chrome://settings/ (2)Scroll down and change search engine used to 'Bing' (3)Press Ctrl+T to open NTP and close it, now hit Ctrl+H to open chrome://history and observe Actual: Black patch appear while opening chrome://history/ page after changing search engine to Bing. Expected: No such black patch should be seen while opening chrome://history/ page after changing search engine to Bing. This is a regression issue broken in 'M67' and below is the manual bisect info Good Build: 67.0.3376.1(Revision:544069) Bad Build: 67.0.3377.0(Revision:544610) Note:Issue not seen on Linux(14.04 LTS) & Mac(10.12.6,10.13.1,10.13.5) OS
,
Apr 9 2018
,
Apr 10 2018
,
Apr 10 2018
,
Apr 12 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/272cb4d86b5efa2e9c02a2eb44eb9405ff874daa commit 272cb4d86b5efa2e9c02a2eb44eb9405ff874daa Author: yiyix <yiyix@chromium.org> Date: Thu Apr 12 15:05:57 2018 Enable the initial RenderFrameMetadata to be sent to Browser In the current implementation, if the new RenderFrameMetadata is same as the initial value, then Browser will not receive the RenderFrameMetadata until it changes. After this patch, Browser will always receive the initial RenderFrameMetadata. Bug:830540 Change-Id: Ic0723d03aff1e4e7fa9eaabe80c27d5dd9bb31d0 Reviewed-on: https://chromium-review.googlesource.com/1006374 Reviewed-by: Fady Samuel <fsamuel@chromium.org> Reviewed-by: Jonathan Ross <jonross@chromium.org> Reviewed-by: Scott Violet <sky@chromium.org> Commit-Queue: Fady Samuel <fsamuel@chromium.org> Cr-Commit-Position: refs/heads/master@{#550200} [modify] https://crrev.com/272cb4d86b5efa2e9c02a2eb44eb9405ff874daa/content/renderer/render_frame_metadata_observer_impl.cc [modify] https://crrev.com/272cb4d86b5efa2e9c02a2eb44eb9405ff874daa/content/renderer/render_frame_metadata_observer_impl.h
,
Apr 17 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/272cb4d86b5efa2e9c02a2eb44eb9405ff874daa commit 272cb4d86b5efa2e9c02a2eb44eb9405ff874daa Author: yiyix <yiyix@chromium.org> Date: Thu Apr 12 15:05:57 2018 Enable the initial RenderFrameMetadata to be sent to Browser In the current implementation, if the new RenderFrameMetadata is same as the initial value, then Browser will not receive the RenderFrameMetadata until it changes. After this patch, Browser will always receive the initial RenderFrameMetadata. Bug:830540 Change-Id: Ic0723d03aff1e4e7fa9eaabe80c27d5dd9bb31d0 Reviewed-on: https://chromium-review.googlesource.com/1006374 Reviewed-by: Fady Samuel <fsamuel@chromium.org> Reviewed-by: Jonathan Ross <jonross@chromium.org> Reviewed-by: Scott Violet <sky@chromium.org> Commit-Queue: Fady Samuel <fsamuel@chromium.org> Cr-Commit-Position: refs/heads/master@{#550200} [modify] https://crrev.com/272cb4d86b5efa2e9c02a2eb44eb9405ff874daa/content/renderer/render_frame_metadata_observer_impl.cc [modify] https://crrev.com/272cb4d86b5efa2e9c02a2eb44eb9405ff874daa/content/renderer/render_frame_metadata_observer_impl.h
,
Apr 17 2018
I haven't finish the fix yet. 1-2 patches needed.
,
Apr 25 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/aed202f9cec76ccbfcea6caa7c3ac6ee49ee8c79 commit aed202f9cec76ccbfcea6caa7c3ac6ee49ee8c79 Author: yiyix <yiyix@chromium.org> Date: Wed Apr 25 17:11:28 2018 Quick Fix Regression: Black patch is seen while opening new page The background color of a new tab is set to the root background color of the old tab (https://codereview.chromium.org/2737553004/). Prior to my change, the root background color will be auto corrected to the correct color by receiving the next CompositorFrame (UpdateBackgroundColor is called in SubmitCompositorFrame). However, after my change, since no new render frame metadata is created by switching tabs, the new tab will be set with root background color of the old tab. When we open another new tab, the same color will carry to the new tab creation. I have worked on rewriting the background color setting logic, https://docs.google.com/document/d/1GLJzDVYuOsnTjysYAnCDWxUFofvxDBHtqTgYE60sEC8 I believe the complete fix requires more testing before merging to M67, so I reverted SetRootBackgroundColor related logics in this CL. Bug:830540 Change-Id: I6f9efaa580f4cac67b2aa3493cdcc093b92076f5 Reviewed-on: https://chromium-review.googlesource.com/1027208 Reviewed-by: Fady Samuel <fsamuel@chromium.org> Commit-Queue: Yi Xu <yiyix@chromium.org> Cr-Commit-Position: refs/heads/master@{#553609} [modify] https://crrev.com/aed202f9cec76ccbfcea6caa7c3ac6ee49ee8c79/content/browser/renderer_host/render_widget_host_view_aura.cc
,
Apr 26 2018
,
Apr 30 2018
,
Apr 30 2018
This bug requires manual review: M67 has already been promoted to the beta branch, so this requires manual review Please contact the milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Apr 30 2018
Could you pls clarify which CLs you're requesting merge for? Are they baked/verified in canary and safe to merge to M67?
,
May 1 2018
The cl that i am requesting merging is in comment 9, https://chromium- review.googlesource.com/1027208, posion: refs/heads/master@{#553609} They are verified and sage to merge.
,
May 1 2018
Approving merge for CL listed at #9 https://chromium- review.googlesource.com/1027208, Cr-Commit-Position: refs/heads/master@{#553609} to M67 branch 3396 based on comment #14. Pls merge ASAP so we can pick it up for this week beta release on Wednesday. Thank you.
,
May 1 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/99aab42086f419ee9d94005dbe1f33715f5eaa62 commit 99aab42086f419ee9d94005dbe1f33715f5eaa62 Author: yiyix <yiyix@chromium.org> Date: Tue May 01 20:02:02 2018 Quick Fix Regression: Black patch is seen while opening new page The background color of a new tab is set to the root background color of the old tab (https://codereview.chromium.org/2737553004/). Prior to my change, the root background color will be auto corrected to the correct color by receiving the next CompositorFrame (UpdateBackgroundColor is called in SubmitCompositorFrame). However, after my change, since no new render frame metadata is created by switching tabs, the new tab will be set with root background color of the old tab. When we open another new tab, the same color will carry to the new tab creation. I have worked on rewriting the background color setting logic, https://docs.google.com/document/d/1GLJzDVYuOsnTjysYAnCDWxUFofvxDBHtqTgYE60sEC8 I believe the complete fix requires more testing before merging to M67, so I reverted SetRootBackgroundColor related logics in this CL. Bug:830540 Change-Id: I6f9efaa580f4cac67b2aa3493cdcc093b92076f5 Reviewed-on: https://chromium-review.googlesource.com/1027208 Reviewed-by: Fady Samuel <fsamuel@chromium.org> Commit-Queue: Yi Xu <yiyix@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#553609}(cherry picked from commit aed202f9cec76ccbfcea6caa7c3ac6ee49ee8c79) Reviewed-on: https://chromium-review.googlesource.com/1037648 Reviewed-by: Yi Xu <yiyix@chromium.org> Cr-Commit-Position: refs/branch-heads/3396@{#423} Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428} [modify] https://crrev.com/99aab42086f419ee9d94005dbe1f33715f5eaa62/content/browser/renderer_host/render_widget_host_view_aura.cc
,
May 2 2018
Tested this issue on Windows 10 using chrome-67.0.3396.30 & seems issue working as intended as per C#0. Hence adding TE verified labels.Please find the attached screencast for reference. Thanks..!
,
May 2 2018
,
May 14 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6a4ede3e8b19101ba5b1735e0a2178b9541a21a3 commit 6a4ede3e8b19101ba5b1735e0a2178b9541a21a3 Author: yiyix <yiyix@chromium.org> Date: Mon May 14 10:38:30 2018 Fix Regression: Black patch is seen while opening new page In the current chrome, SetBackgroundColor is called to set |background_color_| of a RenderWidgetHost, and it called by over 14 callers that can change |background_color_| simultaneously. Any changes in the background color can easily cause races and bugs like: crbug.com/830540 and crbug.com/719230. This cl is aimed to create a more systematical way to update root background color, so that it's easier to update the root background color in the future. A design doc on all SetBackgroundColor analysis: https://docs.google.com/document/d/1GLJzDVYuOsnTjysYAnCDWxUFofvxDBHtqTgYE60sEC8 Bug:830540 Change-Id: I16472d11c53b707536c9aa65c6a660188d1f4d22 Reviewed-on: https://chromium-review.googlesource.com/1029733 Commit-Queue: Yi Xu <yiyix@chromium.org> Reviewed-by: Scott Violet <sky@chromium.org> Reviewed-by: Fady Samuel <fsamuel@chromium.org> Cr-Commit-Position: refs/heads/master@{#558253} [modify] https://crrev.com/6a4ede3e8b19101ba5b1735e0a2178b9541a21a3/chrome/browser/ui/browser.cc [modify] https://crrev.com/6a4ede3e8b19101ba5b1735e0a2178b9541a21a3/chrome/browser/ui/browser_unittest.cc [modify] https://crrev.com/6a4ede3e8b19101ba5b1735e0a2178b9541a21a3/chrome/browser/ui/views/frame/contents_web_view.cc [modify] https://crrev.com/6a4ede3e8b19101ba5b1735e0a2178b9541a21a3/content/browser/frame_host/render_frame_host_manager_unittest.cc [modify] https://crrev.com/6a4ede3e8b19101ba5b1735e0a2178b9541a21a3/content/browser/renderer_host/delegated_frame_host_client_aura.cc [modify] https://crrev.com/6a4ede3e8b19101ba5b1735e0a2178b9541a21a3/content/browser/renderer_host/render_widget_host_unittest.cc [modify] https://crrev.com/6a4ede3e8b19101ba5b1735e0a2178b9541a21a3/content/browser/renderer_host/render_widget_host_view_android.cc [modify] https://crrev.com/6a4ede3e8b19101ba5b1735e0a2178b9541a21a3/content/browser/renderer_host/render_widget_host_view_android.h [modify] https://crrev.com/6a4ede3e8b19101ba5b1735e0a2178b9541a21a3/content/browser/renderer_host/render_widget_host_view_aura.cc [modify] https://crrev.com/6a4ede3e8b19101ba5b1735e0a2178b9541a21a3/content/browser/renderer_host/render_widget_host_view_aura.h [modify] https://crrev.com/6a4ede3e8b19101ba5b1735e0a2178b9541a21a3/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc [modify] https://crrev.com/6a4ede3e8b19101ba5b1735e0a2178b9541a21a3/content/browser/renderer_host/render_widget_host_view_base.cc [modify] https://crrev.com/6a4ede3e8b19101ba5b1735e0a2178b9541a21a3/content/browser/renderer_host/render_widget_host_view_base.h [modify] https://crrev.com/6a4ede3e8b19101ba5b1735e0a2178b9541a21a3/content/browser/renderer_host/render_widget_host_view_child_frame.cc [modify] https://crrev.com/6a4ede3e8b19101ba5b1735e0a2178b9541a21a3/content/browser/renderer_host/render_widget_host_view_child_frame.h [modify] https://crrev.com/6a4ede3e8b19101ba5b1735e0a2178b9541a21a3/content/browser/renderer_host/render_widget_host_view_mac.h [modify] https://crrev.com/6a4ede3e8b19101ba5b1735e0a2178b9541a21a3/content/browser/renderer_host/render_widget_host_view_mac.mm [modify] https://crrev.com/6a4ede3e8b19101ba5b1735e0a2178b9541a21a3/content/browser/renderer_host/render_widget_host_view_mac_unittest.mm [modify] https://crrev.com/6a4ede3e8b19101ba5b1735e0a2178b9541a21a3/content/browser/web_contents/web_contents_android.cc [modify] https://crrev.com/6a4ede3e8b19101ba5b1735e0a2178b9541a21a3/content/public/browser/render_widget_host_view.h [modify] https://crrev.com/6a4ede3e8b19101ba5b1735e0a2178b9541a21a3/content/test/test_render_view_host.cc [modify] https://crrev.com/6a4ede3e8b19101ba5b1735e0a2178b9541a21a3/content/test/test_render_view_host.h [modify] https://crrev.com/6a4ede3e8b19101ba5b1735e0a2178b9541a21a3/extensions/browser/guest_view/web_view/web_view_guest.cc
,
May 14 2018
Is CL listed at #19 need a merge to M67?
,
May 15 2018
nope, the cause of the bug is reverted and merged in M67 in comment 16. The new cl is the real fix of the bug. |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by vku...@etouch.net
, Apr 9 2018Owner: yiyix@chromium.org
Status: Assigned (was: Unconfirmed)