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

Issue 764997 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Feedback dialog doesn't show up in login screen when PlzNavigate is enabled

Project Member Reported by weidongg@chromium.org, Sep 13 2017

Issue description

What steps will reproduce the problem?
Check issue 601874 for steps to repro this bug. You would find feedback dialog does not shown.
Or run test LoginFeedbackTest.Basic with --enable-browser-side-navigation. You would see the test time out.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 14 2017

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

commit 03d21944e7e483392e2b8f91b365f2eb26bff7a4
Author: Weidong Guo <weidongg@chromium.org>
Date: Thu Sep 14 19:18:03 2017

cros: Fix LoginFeedbackTest.Basic time out

Change:
Make feedback private api use EventRouter::DispatchEventToExtension
when content::IsBrowserSideNavigationEnabled() is true.

BUG= 764997 
TEST=LoginFeedbackTest.Basic with flag --enable-browser-side-navigation

Change-Id: I3c8e9424a22d1cc1f8a95fe748549a4fec91eb1a
Reviewed-on: https://chromium-review.googlesource.com/667400
Commit-Queue: Weidong Guo <weidongg@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502011}
[modify] https://crrev.com/03d21944e7e483392e2b8f91b365f2eb26bff7a4/extensions/browser/api/feedback_private/feedback_private_api.cc

Labels: Merge-Request-62 Merge-Request-61
Labels: -Merge-Request-62 Merge-Approved-62
Merge approved for 62, please verify this CL makes it through the Chrome OS PFQ before merging, the branch has no PFQ protection.

Comment 4 by ketakid@google.com, Sep 18 2017

Labels: -Merge-Request-61 Merge-Approved-61
Approving merge to M61. Please merge and validate on M62 prior to merge on M61.

Comment 5 by nasko@chromium.org, Sep 21 2017

weidongg@, do you plan on merging this soon? The earlier we have it, the better.
Project Member

Comment 6 by sheriffbot@chromium.org, Sep 21 2017

Cc: bhthompson@google.com ketakid@google.com
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

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

Comment 7 by bugdroid1@chromium.org, Sep 21 2017

Labels: -merge-approved-61 merge-merged-3163
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/892de5b40d49e5003666687d214c3709688272af

commit 892de5b40d49e5003666687d214c3709688272af
Author: Ahmed Fakhry <afakhry@google.com>
Date: Thu Sep 21 17:18:42 2017

[Merge to M-61] cros: Fix LoginFeedbackTest.Basic time out

Change:
Make feedback private api use EventRouter::DispatchEventToExtension
when content::IsBrowserSideNavigationEnabled() is true.

TBR=weidongg@chromium.org, jam@chromium.org
BUG= 764997 
TEST=LoginFeedbackTest.Basic with flag --enable-browser-side-navigation

(cherry picked from commit 03d21944e7e483392e2b8f91b365f2eb26bff7a4)

Change-Id: I3c8e9424a22d1cc1f8a95fe748549a4fec91eb1a
Reviewed-on: https://chromium-review.googlesource.com/667400
Commit-Queue: Weidong Guo <weidongg@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#502011}
Reviewed-on: https://chromium-review.googlesource.com/677454
Cr-Commit-Position: refs/branch-heads/3163@{#1253}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/892de5b40d49e5003666687d214c3709688272af/chrome/browser/extensions/api/feedback_private/feedback_private_api.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Sep 21 2017

Labels: -merge-approved-62 merge-merged-3202
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/65cc905d9371d05fd82e69fda74b05b5a558b38d

commit 65cc905d9371d05fd82e69fda74b05b5a558b38d
Author: Ahmed Fakhry <afakhry@google.com>
Date: Thu Sep 21 17:34:23 2017

[Merge to M62] cros: Fix LoginFeedbackTest.Basic time out

Change:
Make feedback private api use EventRouter::DispatchEventToExtension
when content::IsBrowserSideNavigationEnabled() is true.

TBR=weidongg@chromium.org, jam@chromium.org
BUG= 764997 
TEST=LoginFeedbackTest.Basic with flag --enable-browser-side-navigation

(cherry picked from commit 03d21944e7e483392e2b8f91b365f2eb26bff7a4)

Change-Id: I3c8e9424a22d1cc1f8a95fe748549a4fec91eb1a
Reviewed-on: https://chromium-review.googlesource.com/667400
Commit-Queue: Weidong Guo <weidongg@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#502011}
Reviewed-on: https://chromium-review.googlesource.com/677483
Cr-Commit-Position: refs/branch-heads/3202@{#377}
Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098}
[modify] https://crrev.com/65cc905d9371d05fd82e69fda74b05b5a558b38d/extensions/browser/api/feedback_private/feedback_private_api.cc

Status: Fixed (was: Assigned)
Project Member

Comment 10 by bugdroid1@chromium.org, Oct 17 2017

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

commit b0df035268a08612322199d462960d726f5dfa11
Author: Weidong Guo <weidongg@chromium.org>
Date: Tue Oct 17 02:55:24 2017

cros: Fix LoginFeedbackTest.Basic time out

There's a race condition when event listener
feedbackPrivate.onFeedbackRequested is added before feedback dialog
extension is loaded completely. The CL prevents this by sending request
to the extension only after the extension is loaded.

BUG=754329, 764997 
TEST=LoginFeedbackTest.Basic with flag --enable-browser-side-navigation

Change-Id: I2988012d91dd3bd61e1a27ab2486a603e1e0fa64
Reviewed-on: https://chromium-review.googlesource.com/722027
Commit-Queue: Weidong Guo <weidongg@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509246}
[modify] https://crrev.com/b0df035268a08612322199d462960d726f5dfa11/chrome/browser/chromeos/login/ui/login_feedback.cc
[modify] https://crrev.com/b0df035268a08612322199d462960d726f5dfa11/extensions/browser/api/feedback_private/feedback_private_api.cc

Project Member

Comment 11 by bugdroid1@chromium.org, Oct 17 2017

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

commit 49bb976c0c7e3edc62080236b4f36f6895f2d8b6
Author: Weidong Guo <weidongg@chromium.org>
Date: Tue Oct 17 22:22:31 2017

cros: Fix LoginFeedbackTest.Basic time out

There's a race condition when event listener
feedbackPrivate.onFeedbackRequested is added before feedback dialog
extension is loaded completely. The CL prevents this by sending request
to the extension only after the extension is loaded.

BUG=754329, 764997 
TEST=LoginFeedbackTest.Basic with flag --enable-browser-side-navigation
TBR=afakhry@chromium.org

(cherry picked from commit b0df035268a08612322199d462960d726f5dfa11)

Change-Id: I2988012d91dd3bd61e1a27ab2486a603e1e0fa64
Reviewed-on: https://chromium-review.googlesource.com/722027
Commit-Queue: Weidong Guo <weidongg@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#509246}
Reviewed-on: https://chromium-review.googlesource.com/724206
Cr-Commit-Position: refs/branch-heads/3239@{#39}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
[modify] https://crrev.com/49bb976c0c7e3edc62080236b4f36f6895f2d8b6/chrome/browser/chromeos/login/ui/login_feedback.cc
[modify] https://crrev.com/49bb976c0c7e3edc62080236b4f36f6895f2d8b6/extensions/browser/api/feedback_private/feedback_private_api.cc

Project Member

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

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

commit 5d4554003d392c4bdf3ec9c192fa1c13473720cc
Author: Weidong Guo <weidongg@chromium.org>
Date: Tue Oct 17 22:23:12 2017

cros: Fix LoginFeedbackTest.Basic time out

There's a race condition when event listener
feedbackPrivate.onFeedbackRequested is added before feedback dialog
extension is loaded completely. The CL prevents this by sending request
to the extension only after the extension is loaded.

BUG=754329, 764997 
TEST=LoginFeedbackTest.Basic with flag --enable-browser-side-navigation
TBR=afakhry@chromium.org

(cherry picked from commit b0df035268a08612322199d462960d726f5dfa11)

Change-Id: I2988012d91dd3bd61e1a27ab2486a603e1e0fa64
Reviewed-on: https://chromium-review.googlesource.com/722027
Commit-Queue: Weidong Guo <weidongg@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#509246}
Reviewed-on: https://chromium-review.googlesource.com/724205
Cr-Commit-Position: refs/branch-heads/3202@{#707}
Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098}
[modify] https://crrev.com/5d4554003d392c4bdf3ec9c192fa1c13473720cc/chrome/browser/chromeos/login/ui/login_feedback.cc
[modify] https://crrev.com/5d4554003d392c4bdf3ec9c192fa1c13473720cc/extensions/browser/api/feedback_private/feedback_private_api.cc

 Issue 777529  has been merged into this issue.

Comment 14 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Comment 15 by dchan@chromium.org, Jan 23 2018

Status: Fixed (was: Archived)

Sign in to add a comment