New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 613979 link

Starred by 4 users
Status: Fixed
Owner:
scottmg@chromium.org
Closed: Jun 2016
Cc:
wjmaclean@chromium.org
ajha@chromium.org
gov...@chromium.org
█ kavvaru@chromium.org
nyerramilli@chromium.org
█ cernekee@chromium.org
█ tinazh@chromium.org
█ ashej...@chromium.org
█ durga.behera@chromium.org
scottmg@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Regression: Page zoom doesn't restore but zoom bubble remains in guestmode

Project Member Reported by sc00335...@techmahindra.com, May 23 2016 
Version: 53.0.2746.0 dev 
OS: Ubuntu 14.04,windows

What steps will reproduce the problem?
(1) Browse as Guest >> Change zoom level[Say 200%] >> Close Guest window using "x" button
(2) Now again launch guest mode and observe for zoomed page content and zoom bubble
(3) Open new tab in guest mode and observe page content

Expected: Page zoom should remain even after closing and opening guest window.
Actual: Instead bubble remains and page content zoom restores to normal zoom level.

This is a regression issue broken in M52.

Manual Bisect:
Good Build: 52.0.2740.0 dev.
Bad Build: 52.0.2741.0 dev.

Tool Bisect:
CHANGELOG URL:
 https://chromium.googlesource.com/chromium/src/+log/d4f77baa51cf45c027f0f409b2e2a127ea867bcb..302068727a07d8c82d8cd81ee73c8a06e9a9a202

Suspecting  https://codereview.chromium.org/1964273002 from changelog

@scottmg: Please confirm the issue and help in re-assigning if it is not related to your change.
Actual_guestmodezoom.ogv
1.4 MB Download
Expected_guestmode zoom.ogv
1.3 MB Download

Comment 1 by ajha@chromium.org, May 23 2016

Labels: OS-Mac
Reproducible on the latest canary(53.0.2746.0) on Mac OS 10.11.5 as well.

Comment 2 Deleted

Project Member

Comment 3 by bugdroid1@chromium.org, May 25 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/34ce95dd9c2ca3b90fc87169f1e4ed266e7bb94b

commit 34ce95dd9c2ca3b90fc87169f1e4ed266e7bb94b
Author: scottmg <scottmg@chromium.org>
Date: Wed May 25 01:04:56 2016

Revert of Add FrameHost mojo service (patchset #23 id:490001 of https://codereview.chromium.org/1964273002/ )

Reason for revert:
Async request for zoom level doesn't work in all cases  https://crbug.com/614348   https://crbug.com/613979 .

I thought https://www.chromium.org/developers/design-documents/mojo/chrome-ipc-to-mojo-ipc-cheet-sheet#TOC-Threading-Model implied the ordering would be correct, but it seems that was too hopeful.

Original issue's description:
> Add FrameHost mojo service
>
> Adds new frame-level service with one initial method to handle host zoom
> level.
>
> This moves zoom level supply from async_resource_handler.cc to being a
> request made when render_frame_impl handles a willSendRequest.
>
> The goal of this change is to remove the dependency of
> content/browser/loader on the rest of content/browser in particular
> here, removing the use of c/b/host_zoom_map_impl.h in
> content/browser/loader/async_resource_handler.cc.
>
> BUG=598073, 609607 
> CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
>
> Committed: https://crrev.com/68c6f2ce16d9807b5cb82679099c82c40f39e911
> Cr-Commit-Position: refs/heads/master@{#394547}

TBR=dcheng@chromium.org,ben@chromium.org,jam@chromium.org,nasko@chromium.org,wjmaclean@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=598073, 609607 , 614348 , 613979 
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2007203002
Cr-Commit-Position: refs/heads/master@{#395761}

[modify] https://crrev.com/34ce95dd9c2ca3b90fc87169f1e4ed266e7bb94b/build/android/pylib/gtest/filter/content_browsertests_disabled
[modify] https://crrev.com/34ce95dd9c2ca3b90fc87169f1e4ed266e7bb94b/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/34ce95dd9c2ca3b90fc87169f1e4ed266e7bb94b/content/browser/frame_host/render_frame_host_impl.h
[modify] https://crrev.com/34ce95dd9c2ca3b90fc87169f1e4ed266e7bb94b/content/browser/host_zoom_map_impl.h
[modify] https://crrev.com/34ce95dd9c2ca3b90fc87169f1e4ed266e7bb94b/content/browser/loader/DEPS
[modify] https://crrev.com/34ce95dd9c2ca3b90fc87169f1e4ed266e7bb94b/content/browser/loader/async_resource_handler.cc
[modify] https://crrev.com/34ce95dd9c2ca3b90fc87169f1e4ed266e7bb94b/content/browser/loader/async_revalidation_manager_unittest.cc
[modify] https://crrev.com/34ce95dd9c2ca3b90fc87169f1e4ed266e7bb94b/content/browser/loader/resource_dispatcher_host_unittest.cc
[modify] https://crrev.com/34ce95dd9c2ca3b90fc87169f1e4ed266e7bb94b/content/browser/loader/resource_message_filter.cc
[modify] https://crrev.com/34ce95dd9c2ca3b90fc87169f1e4ed266e7bb94b/content/browser/loader/resource_message_filter.h
[modify] https://crrev.com/34ce95dd9c2ca3b90fc87169f1e4ed266e7bb94b/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/34ce95dd9c2ca3b90fc87169f1e4ed266e7bb94b/content/common/BUILD.gn
[delete] https://crrev.com/bcd06ba900e67c54ffffb3f421c6b0d1bcd8ef31/content/common/frame_host.mojom
[modify] https://crrev.com/34ce95dd9c2ca3b90fc87169f1e4ed266e7bb94b/content/common/view_messages.h
[modify] https://crrev.com/34ce95dd9c2ca3b90fc87169f1e4ed266e7bb94b/content/content_common_mojo_bindings.gyp
[modify] https://crrev.com/34ce95dd9c2ca3b90fc87169f1e4ed266e7bb94b/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/34ce95dd9c2ca3b90fc87169f1e4ed266e7bb94b/content/renderer/render_frame_impl.h
[modify] https://crrev.com/34ce95dd9c2ca3b90fc87169f1e4ed266e7bb94b/content/renderer/render_frame_impl_browsertest.cc
[modify] https://crrev.com/34ce95dd9c2ca3b90fc87169f1e4ed266e7bb94b/content/renderer/render_view_browsertest.cc
[modify] https://crrev.com/34ce95dd9c2ca3b90fc87169f1e4ed266e7bb94b/content/renderer/render_view_impl.cc
[modify] https://crrev.com/34ce95dd9c2ca3b90fc87169f1e4ed266e7bb94b/content/renderer/render_view_impl.h

Comment 4 by wjmaclean@chromium.org, May 25 2016

Cc: wjmaclean@chromium.org

Comment 5 by scottmg@chromium.org, May 25 2016

Labels: Merge-Request-52
I'll need to merge the revert to 52.

Comment 6 by tin...@google.com, May 25 2016

Labels: -Merge-Request-52 Merge-Review-52 Hotlist-Merge-Review
[Automated comment] DEPS changes referenced in bugdroid comments, needs manual review.

Comment 7 by scottmg@chromium.org, May 26 2016

Cc: cernekee@chromium.org nyerramilli@chromium.org ashej...@chromium.org scottmg@chromium.org
 Issue 614348  has been merged into this issue.

Comment 8 by scottmg@chromium.org, May 26 2016 

 Issue 614915  has been merged into this issue.

Comment 9 by scottmg@chromium.org, May 26 2016 

Issue 614518 has been merged into this issue.

Comment 10 by ligim...@chromium.org, May 26 2016 

Manually verified in https://bugs.chromium.org/p/chromium/issues/detail?id=614518#c11. So we can make a revert in M52.

Tina/ Krishna , can you tale appropriate action here.

Comment 11 by scottmg@chromium.org, May 27 2016 

tinazh: ping, ok to merge?

Comment 12 by scottmg@chromium.org, May 27 2016

Cc: tinazh@chromium.org

Comment 13 by ligim...@chromium.org, May 27 2016

Cc: gov...@chromium.org
Krishna, could u please approve the merge.

Comment 14 by gov...@chromium.org, May 27 2016

Labels: -Merge-Review-52 Merge-Approved-52
Approving merge of revert to M52 branch 2743 based on comment #10. Please merge asap. Thank you.
Project Member

Comment 15 by bugdroid1@chromium.org, May 27 2016

Labels: -merge-approved-52 merge-merged-2743
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b43ea3087da4e351f0bf94481c9810738c3ae938

commit b43ea3087da4e351f0bf94481c9810738c3ae938
Author: Scott Graham <scottmg@chromium.org>
Date: Fri May 27 22:40:35 2016

Revert of Add FrameHost mojo service (patchset #23 id:490001 of https://codereview.chromium.org/1964273002/ )

Reason for revert:
Async request for zoom level doesn't work in all cases  https://crbug.com/614348   https://crbug.com/613979 .

I thought https://www.chromium.org/developers/design-documents/mojo/chrome-ipc-to-mojo-ipc-cheet-sheet#TOC-Threading-Model implied the ordering would be correct, but it seems that was too hopeful.

Original issue's description:
> Add FrameHost mojo service
>
> Adds new frame-level service with one initial method to handle host zoom
> level.
>
> This moves zoom level supply from async_resource_handler.cc to being a
> request made when render_frame_impl handles a willSendRequest.
>
> The goal of this change is to remove the dependency of
> content/browser/loader on the rest of content/browser in particular
> here, removing the use of c/b/host_zoom_map_impl.h in
> content/browser/loader/async_resource_handler.cc.
>
> BUG=598073, 609607 
> CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
>
> Committed: https://crrev.com/68c6f2ce16d9807b5cb82679099c82c40f39e911
> Cr-Commit-Position: refs/heads/master@{#394547}

TBR=dcheng@chromium.org,ben@chromium.org,jam@chromium.org,nasko@chromium.org,wjmaclean@chromium.org
BUG=598073, 609607 , 614348 , 613979 
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2007203002
Cr-Commit-Position: refs/heads/master@{#395761}
(cherry picked from commit 34ce95dd9c2ca3b90fc87169f1e4ed266e7bb94b)

Review URL: https://codereview.chromium.org/2020633003 .

Cr-Commit-Position: refs/branch-heads/2743@{#111}
Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939}

[modify] https://crrev.com/b43ea3087da4e351f0bf94481c9810738c3ae938/build/android/pylib/gtest/filter/content_browsertests_disabled
[modify] https://crrev.com/b43ea3087da4e351f0bf94481c9810738c3ae938/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/b43ea3087da4e351f0bf94481c9810738c3ae938/content/browser/frame_host/render_frame_host_impl.h
[modify] https://crrev.com/b43ea3087da4e351f0bf94481c9810738c3ae938/content/browser/host_zoom_map_impl.h
[modify] https://crrev.com/b43ea3087da4e351f0bf94481c9810738c3ae938/content/browser/loader/DEPS
[modify] https://crrev.com/b43ea3087da4e351f0bf94481c9810738c3ae938/content/browser/loader/async_resource_handler.cc
[modify] https://crrev.com/b43ea3087da4e351f0bf94481c9810738c3ae938/content/browser/loader/async_revalidation_manager_unittest.cc
[modify] https://crrev.com/b43ea3087da4e351f0bf94481c9810738c3ae938/content/browser/loader/resource_dispatcher_host_unittest.cc
[modify] https://crrev.com/b43ea3087da4e351f0bf94481c9810738c3ae938/content/browser/loader/resource_message_filter.cc
[modify] https://crrev.com/b43ea3087da4e351f0bf94481c9810738c3ae938/content/browser/loader/resource_message_filter.h
[modify] https://crrev.com/b43ea3087da4e351f0bf94481c9810738c3ae938/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/b43ea3087da4e351f0bf94481c9810738c3ae938/content/common/BUILD.gn
[delete] https://crrev.com/d2634566be6ed8b068e1ff5414525627a8adb7fc/content/common/frame_host.mojom
[modify] https://crrev.com/b43ea3087da4e351f0bf94481c9810738c3ae938/content/common/view_messages.h
[modify] https://crrev.com/b43ea3087da4e351f0bf94481c9810738c3ae938/content/content_common_mojo_bindings.gyp
[modify] https://crrev.com/b43ea3087da4e351f0bf94481c9810738c3ae938/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/b43ea3087da4e351f0bf94481c9810738c3ae938/content/renderer/render_frame_impl.h
[modify] https://crrev.com/b43ea3087da4e351f0bf94481c9810738c3ae938/content/renderer/render_frame_impl_browsertest.cc
[modify] https://crrev.com/b43ea3087da4e351f0bf94481c9810738c3ae938/content/renderer/render_view_browsertest.cc
[modify] https://crrev.com/b43ea3087da4e351f0bf94481c9810738c3ae938/content/renderer/render_view_impl.cc
[modify] https://crrev.com/b43ea3087da4e351f0bf94481c9810738c3ae938/content/renderer/render_view_impl.h

Comment 16 by durga.behera@chromium.org, May 30 2016

Labels: TE-Verified-M52 TE-Verified-52.0.2743.17
verified the issue on Mac 10.11.5,Win 7 and Ubuntu 14.04 using 52.0.2743.17 and its working fine.
613979_May_30.mp4
887 KB Download

Comment 17 by ligim...@chromium.org, Jun 3 2016 

scottmg@ if there is no pending work, could you please mark as fixed.

Comment 18 by scottmg@chromium.org, Jun 6 2016

Status: Fixed (was: Assigned)
►

Sign in to add a comment