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

Issue 774039 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Regression : Screenshot is not seen in 'Feedback' overlay when browser window is restored down.

Reported by avsha...@etouch.net, Oct 12 2017

Issue description

Chrome version : 63.0.3238.0 (Official Build) fc7c1e473dfde53eb32f2e6528d6b0f957f850f5-refs/heads/master@{#508208} 64 bit
OS : Mac Retina(10.12.6)

What steps will reproduce the problem?
1. Launch chrome and resize the browser window. (Kindly refer an attached screencast)
2. Open 'Wrench' menu, go to 'Help' and click on 'Report an issue..' option.
3. Observe the screenshot in 'Feedback' overlay.

Actual Result : Screenshot is not seen in 'Feedback' overlay when browser window is restored down.

Expected Result : Screenshot should be seen in 'Feedback' overlay even when browser window is resized.

This is a regression issue broken in ‘M-63’ and using the per-revision bisect providing the bisect results,
Good build : 63.0.3232.0 (Revision : 506257)
Bad build : 63.0.3233.0 (Revision : 506599)

You are probably looking for a change made after 506552 (known good), but no later than 506553 (first known bad).

CHANGELOG URL:
The script might not always return single CL as suspect as some perf builds might get missing due to failure.

https://chromium.googlesource.com/chromium/src/+log/5f7d27b42059440bf87b4f7931b0af28bd41719f..585cd042d41fd0ca97fc82db0e01d060bf6792d5

Suspect : https://chromium.googlesource.com/chromium/src/+/585cd042d41fd0ca97fc82db0e01d060bf6792d5

@dcheng : Could you please look into the issue, pardon me if it has nothing to do with your changes and if possible please assign it to concern owner.

Note : 
1. Issue is reproducible only when the browser window is resized.
2. This is Mac Retina 10.12.6 specific issue and the same is not reproducible on Mac mini machines.

Thank you.
 
Actual_Feedback_Overlay.mov
10.2 MB Download
Expected_Feedback_Overlay.mov
8.2 MB Download
Labels: ReleaseBlock-Stable
Tagging with blocker label, please undo if not the case.

Comment 2 by dcheng@chromium.org, Oct 16 2017

Cc: mkwst@chromium.org dcheng@chromium.org
 Issue 774763  has been merged into this issue.
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 18 2017

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

commit ef9f4b7027aa343123ec564c517894931eb3efb8
Author: Daniel Cheng <dcheng@chromium.org>
Date: Wed Oct 18 08:56:17 2017

Revert "Treat overly long URLs as invalid during canonicalization."

This reverts commit 585cd042d41fd0ca97fc82db0e01d060bf6792d5.

Reason for revert: it's relatively common to load > 2MB data URLs as
                   subresources =(

Bug:  774039 

Original change's description:
> Treat overly long URLs as invalid during canonicalization.
> 
> Bug: 571784
> Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
> Change-Id: Ied18e9d76e13028b7077a4ed3631f0a64ce8bdf6
> Reviewed-on: https://chromium-review.googlesource.com/678058
> Commit-Queue: Daniel Cheng <dcheng@chromium.org>
> Reviewed-by: Asanka Herath <asanka@chromium.org>
> Reviewed-by: Charlie Reis <creis@chromium.org>
> Reviewed-by: Mike West <mkwst@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#506553}

TBR=dcheng@chromium.org,creis@chromium.org,asanka@chromium.org,mkwst@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 571784
Change-Id: I011fd8a9bcf5534ec23bcee10cc1de0add21d150
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Reviewed-on: https://chromium-review.googlesource.com/725023
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509719}
[modify] https://crrev.com/ef9f4b7027aa343123ec564c517894931eb3efb8/content/browser/frame_host/navigation_controller_android.cc
[modify] https://crrev.com/ef9f4b7027aa343123ec564c517894931eb3efb8/content/browser/frame_host/navigator_impl.cc
[modify] https://crrev.com/ef9f4b7027aa343123ec564c517894931eb3efb8/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/ef9f4b7027aa343123ec564c517894931eb3efb8/net/base/data_url.cc
[modify] https://crrev.com/ef9f4b7027aa343123ec564c517894931eb3efb8/net/base/data_url.h
[modify] https://crrev.com/ef9f4b7027aa343123ec564c517894931eb3efb8/net/base/data_url_unittest.cc
[delete] https://crrev.com/bd46dbc8a70160b9d8cd416a10d3cb8d4bf0cf96/third_party/WebKit/LayoutTests/fast/dom/location-assign-long-hash.html
[modify] https://crrev.com/ef9f4b7027aa343123ec564c517894931eb3efb8/url/gurl_unittest.cc
[modify] https://crrev.com/ef9f4b7027aa343123ec564c517894931eb3efb8/url/url_constants.cc
[modify] https://crrev.com/ef9f4b7027aa343123ec564c517894931eb3efb8/url/url_constants.h
[modify] https://crrev.com/ef9f4b7027aa343123ec564c517894931eb3efb8/url/url_util.cc

Comment 4 by dcheng@chromium.org, Oct 19 2017

Labels: Merge-Request-63
Status: Fixed (was: Assigned)
Project Member

Comment 5 by sheriffbot@chromium.org, Oct 19 2017

Labels: -Merge-Request-63 Merge-Review-63 Hotlist-Merge-Review
This bug requires manual review: Reverts referenced in bugdroid comments after merge request.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 6 by gov...@chromium.org, Oct 19 2017

Before we approve merge for revert listed at #3, could you please confirm change is baked/verified in Canary and safe to merge to M63?
Project Member

Comment 7 by bugdroid1@chromium.org, Oct 20 2017

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

commit d9f27c4c5432140035553c9d06a7fd51ef9d76a6
Author: Francois Beaufort <beaufort.francois@gmail.com>
Date: Fri Oct 20 11:40:16 2017

feedback: Use toBlob instead of toDataURL when taking screenshot

Bug:  774039 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: If40e4c89791e492335f854d02c4a80808298026b
Reviewed-on: https://chromium-review.googlesource.com/725330
Commit-Queue: Mike West <mkwst@chromium.org>
Reviewed-by: Rahul Chaturvedi <rkc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510403}
[modify] https://crrev.com/d9f27c4c5432140035553c9d06a7fd51ef9d76a6/chrome/browser/resources/feedback/js/feedback.js

Comment 8 by dcheng@chromium.org, Oct 20 2017

OK, the revert has been in canary for 24 hours and nothing untoward (that I know of) has happened.

Comment 9 by gov...@chromium.org, Oct 20 2017

Labels: -Merge-Review-63 Merge-Approved-63
Approving merge for revert listed at #3 to M63 branch 3239 per comment #8.

Is CL listed at #7 need to be merged to M63 as well?


Nope, only the revert needs to be merged.
Ok, revert merge is approved. Please merge ASAP. Thank you.
Project Member

Comment 12 by bugdroid1@chromium.org, Oct 20 2017

Labels: -merge-approved-63 merge-merged-3239
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fc2f1955dfec5c6a6f42657284c562d79db09ac3

commit fc2f1955dfec5c6a6f42657284c562d79db09ac3
Author: Daniel Cheng <dcheng@chromium.org>
Date: Fri Oct 20 20:24:29 2017

Revert "Treat overly long URLs as invalid during canonicalization."

This reverts commit 585cd042d41fd0ca97fc82db0e01d060bf6792d5.

Reason for revert: it's relatively common to load > 2MB data URLs as
                   subresources =(

Bug:  774039 

Original change's description:
> Treat overly long URLs as invalid during canonicalization.
> 
> Bug: 571784
> Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
> Change-Id: Ied18e9d76e13028b7077a4ed3631f0a64ce8bdf6
> Reviewed-on: https://chromium-review.googlesource.com/678058
> Commit-Queue: Daniel Cheng <dcheng@chromium.org>
> Reviewed-by: Asanka Herath <asanka@chromium.org>
> Reviewed-by: Charlie Reis <creis@chromium.org>
> Reviewed-by: Mike West <mkwst@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#506553}

TBR=dcheng@chromium.org,creis@chromium.org,asanka@chromium.org,mkwst@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 571784
Change-Id: I011fd8a9bcf5534ec23bcee10cc1de0add21d150
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Reviewed-on: https://chromium-review.googlesource.com/725023
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#509719}(cherry picked from commit ef9f4b7027aa343123ec564c517894931eb3efb8)
Reviewed-on: https://chromium-review.googlesource.com/730875
Cr-Commit-Position: refs/branch-heads/3239@{#115}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
[modify] https://crrev.com/fc2f1955dfec5c6a6f42657284c562d79db09ac3/content/browser/frame_host/navigation_controller_android.cc
[modify] https://crrev.com/fc2f1955dfec5c6a6f42657284c562d79db09ac3/content/browser/frame_host/navigator_impl.cc
[modify] https://crrev.com/fc2f1955dfec5c6a6f42657284c562d79db09ac3/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/fc2f1955dfec5c6a6f42657284c562d79db09ac3/net/base/data_url.cc
[modify] https://crrev.com/fc2f1955dfec5c6a6f42657284c562d79db09ac3/net/base/data_url.h
[modify] https://crrev.com/fc2f1955dfec5c6a6f42657284c562d79db09ac3/net/base/data_url_unittest.cc
[delete] https://crrev.com/3d4aa1351d4f97127c04acff31438e3842c89aba/third_party/WebKit/LayoutTests/fast/dom/location-assign-long-hash.html
[modify] https://crrev.com/fc2f1955dfec5c6a6f42657284c562d79db09ac3/url/gurl_unittest.cc
[modify] https://crrev.com/fc2f1955dfec5c6a6f42657284c562d79db09ac3/url/url_constants.cc
[modify] https://crrev.com/fc2f1955dfec5c6a6f42657284c562d79db09ac3/url/url_constants.h
[modify] https://crrev.com/fc2f1955dfec5c6a6f42657284c562d79db09ac3/url/url_util.cc

Sign in to add a comment