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

Issue 777839 link

Starred by 7 users

Issue metadata

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



Sign in to add a comment

Regression: Unable to perform operations in 'Attach a photo' overlay of hangouts app

Project Member Reported by rkalavakuntla@chromium.org, Oct 24 2017

Issue description

Chrome Version:64.0.3246.0 dev
OS:windows,Ubuntu 14.04,Chrome OS

Test URL: https://chrome.google.com/webstore/search/hangouts?utm_source=chrome-ntp-icon&_category=apps

What steps will reproduce the problem?
(1)Sign into Chrome,add the Google Hangouts App by using above URL 
(2)Launch app ->select a user -> click on 'Attach a Photo' and try to upload photo by 'select a photo from your computer' and Observe(kindly refer video)

Actual: Unable to upload photo from computer
Expected: Should be able to upload photo

This is a Regression issue broken in M-63

Manual Bisect Info:
-------------------
Good Build:63.0.3223.0(503964)
Bad Build :63.0.3225.0(504540)



 
actual.webm
1.9 MB View Download
Expected.webm
1.4 MB View Download
Labels: -ReleaseBlock-Beta -M-64 hasbisect-per-revision ReleaseBlock-Stable M-63
Owner: lfg@chromium.org
Status: Assigned (was: Untriaged)
Using per-revision bisect providing the bisect results:

You are probably looking for a change made after 485575 (known good), but no later than 485576 (first known bad).
CHANGELOG URL:
https://chromium.googlesource.com/chromium/src/+log/1b62135c07abbd0fb531b03fddb3c2114cb06698..3131414035a033d1fb624913474dbdac04158198

Suspecting: https://chromium.googlesource.com/chromium/src/+/3131414035a033d1fb624913474dbdac04158198
Review-Url: https://chromium-review.googlesource.com/680158

Lucas Furukawa Gadani@:Could you please take a look into this issue and reassign if this issue is not related to your change.

Thanks..!!

Reprod on M63-ChromeOS:10032.14.0/63.0.3239.17 

Comment 3 by lfg@chromium.org, Oct 24 2017

Cc: lfg@chromium.org
Components: -Platform>Apps Platform>Apps>BrowserTag
Owner: wjmaclean@chromium.org
Assigning to wjmaclean@ who's currently looking at this.
There is a CL to fix this up for review at: https://chromium-review.googlesource.com/c/chromium/src/+/738278

We expect to get it landed within the next day or so.
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 28 2017

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

commit a21ff3d498fdbdc0114fad1cdd127f7675614eb5
Author: W. James MacLean <wjmaclean@chromium.org>
Date: Sat Oct 28 13:04:24 2017

GuestViewMessageFilter should update the embedder web contents before attach.

GuestViewMessageFilter::OnAttachToEmbedderFrame() up until now assumed
the guest was created with the WebContents* that will be the final owner.
This works for WebViews created in static HTML or from JS in the
embedder, but can fail if window.open is used, and the embedder
web contents may be created along with the new window.

This function should instead look up the embedder WebContents* from
the provided embedder local frame id, and use that to override in the
call to GuestViewBase::WillAttach().

Bug:  777839 
Change-Id: I4a33ecf3aaab6dbd645835191247670a3dccd1f3
Reviewed-on: https://chromium-review.googlesource.com/738278
Commit-Queue: James MacLean <wjmaclean@chromium.org>
Reviewed-by: Lucas Gadani <lfg@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512399}
[modify] https://crrev.com/a21ff3d498fdbdc0114fad1cdd127f7675614eb5/chrome/browser/apps/guest_view/web_view_browsertest.cc
[modify] https://crrev.com/a21ff3d498fdbdc0114fad1cdd127f7675614eb5/chrome/test/data/extensions/platform_apps/web_view/shim/main.js
[add] https://crrev.com/a21ff3d498fdbdc0114fad1cdd127f7675614eb5/chrome/test/data/extensions/platform_apps/web_view/shim/new_window_main.html
[modify] https://crrev.com/a21ff3d498fdbdc0114fad1cdd127f7675614eb5/components/guest_view/browser/guest_view_message_filter.cc
[modify] https://crrev.com/a21ff3d498fdbdc0114fad1cdd127f7675614eb5/content/public/test/browser_test_utils.cc
[modify] https://crrev.com/a21ff3d498fdbdc0114fad1cdd127f7675614eb5/content/public/test/browser_test_utils.h

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

Comment 7 by sheriffbot@chromium.org, Oct 29 2017

Labels: -Merge-Request-63 Merge-Review-63 Hotlist-Merge-Review
This bug requires manual review: M63 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), gkihumba@(ChromeOS), govind@(Desktop)

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

Comment 8 by gov...@chromium.org, Oct 29 2017

Before we approve merge to M63, could you pls confirm change listed at #5 is well baked/verified in Canary, having enough automation coverage and safe to merge to M63?
I think we can say 'yes' to the latter two conditions, so let's let it run until it's been in canary for a bit (and perhaps verified on canary as well).
Labels: TE-Verified-M64 TE-Verified-64.0.3254.0
Verified the issue on win-7,Ubuntu 14.04,Mac OS 10.12.6 using chrome version M-64 #64.0.3254.0 and issue is fixed.
Able to upload the photo.

Adding TE-Verified labels.

Thanks!
777839.webm
533 KB View Download
Cc: dominicc@chromium.org dglazkov@chromium.org abarth@chromium.org wjmaclean@chromium.org
 Issue 282477  has been merged into this issue.
Labels: -Merge-Review-63 Merge-Approved-63
Approving merge to M63 branch 3239 based on comment #9 & #10. Please merge ASAP so we can pick it up for tomorrow's Beta release. Thank you.
Project Member

Comment 13 by bugdroid1@chromium.org, Oct 31 2017

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

commit 0fe084ad1a028c060fcacbb6a027e6b703e8e8f9
Author: W. James MacLean <wjmaclean@chromium.org>
Date: Tue Oct 31 17:55:21 2017

GuestViewMessageFilter should update the embedder web contents before attach.

GuestViewMessageFilter::OnAttachToEmbedderFrame() up until now assumed
the guest was created with the WebContents* that will be the final owner.
This works for WebViews created in static HTML or from JS in the
embedder, but can fail if window.open is used, and the embedder
web contents may be created along with the new window.

This function should instead look up the embedder WebContents* from
the provided embedder local frame id, and use that to override in the
call to GuestViewBase::WillAttach().

TBR=wjmaclean@chromium.org

(cherry picked from commit a21ff3d498fdbdc0114fad1cdd127f7675614eb5)

Bug:  777839 
Change-Id: I4a33ecf3aaab6dbd645835191247670a3dccd1f3
Reviewed-on: https://chromium-review.googlesource.com/738278
Commit-Queue: James MacLean <wjmaclean@chromium.org>
Reviewed-by: Lucas Gadani <lfg@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#512399}
Reviewed-on: https://chromium-review.googlesource.com/747383
Reviewed-by: James MacLean <wjmaclean@chromium.org>
Cr-Commit-Position: refs/branch-heads/3239@{#316}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
[modify] https://crrev.com/0fe084ad1a028c060fcacbb6a027e6b703e8e8f9/chrome/browser/apps/guest_view/web_view_browsertest.cc
[modify] https://crrev.com/0fe084ad1a028c060fcacbb6a027e6b703e8e8f9/chrome/test/data/extensions/platform_apps/web_view/shim/main.js
[add] https://crrev.com/0fe084ad1a028c060fcacbb6a027e6b703e8e8f9/chrome/test/data/extensions/platform_apps/web_view/shim/new_window_main.html
[modify] https://crrev.com/0fe084ad1a028c060fcacbb6a027e6b703e8e8f9/components/guest_view/browser/guest_view_message_filter.cc
[modify] https://crrev.com/0fe084ad1a028c060fcacbb6a027e6b703e8e8f9/content/public/test/browser_test_utils.cc
[modify] https://crrev.com/0fe084ad1a028c060fcacbb6a027e6b703e8e8f9/content/public/test/browser_test_utils.h

Sign in to add a comment