Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2347273002:
Closing unused new tabs for ARC (Closed)
|
Created:
4 years, 3 months ago by djacobo_ Modified:
4 years, 3 months ago CC:
chromium-reviews, elijahtaylor+arcwatch_chromium.org, yusukes+watch_chromium.org, tfarina, hidehiko+watch_chromium.org, lhchavez+watch_chromium.org, oshima+watch_chromium.org, davemoore+watch_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionClosing unused new tabs for ARC
Some links open a new tab while navigating, given that we redirect some
of the traffic to ARC, it can be necessary to close any unused tab.
Since we are closing the contained WebContents on a tab, we need to make
sure the callback within the UI doesn't get called twice, since intent
picker has an observer for the WebContents.
BUG=647806
TEST=manual test
Committed: https://crrev.com/580779d7bc1eda07b42bd467d60ca6c18765c3db
Cr-Commit-Position: refs/heads/master@{#419335}
Patch Set 1 #
Total comments: 2
Messages
Total messages: 21 (14 generated)
The CQ bit was checked by djacobo@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Closing unused new tabs for ARC Some links open a new tab while navigating, given that we redirect some of the traffic to ARC, it can be necessary to close any unused tab. Since we are closing the contained WebContents on a tab, we need to make sure the callback within the UI doesn't get called twice, since intent picker has an observer for the WebContents. BUG=647806 TEST=manual test ========== to ========== Closing unused new tabs for ARC Some links open a new tab while navigating, given that we redirect some of the traffic to ARC, it can be necessary to close any unused tab. Since we are closing the contained WebContents on a tab, we need to make sure the callback within the UI doesn't get called twice, since intent picker has an observer for the WebContents. BUG=647806 TEST=manual test ==========
djacobo@google.com changed reviewers: + yusukes@chromium.org
djacobo@google.com changed reviewers: + sky@chromium.org
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by djacobo@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2347273002/diff/1/chrome/browser/chromeos/arc... File chrome/browser/chromeos/arc/arc_navigation_throttle.cc (right): https://codereview.chromium.org/2347273002/diff/1/chrome/browser/chromeos/arc... chrome/browser/chromeos/arc/arc_navigation_throttle.cc:252: if (handle->GetWebContents()->GetController().IsInitialNavigation()) Does handle->GetWebContents() always return a non-null object?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2347273002/diff/1/chrome/browser/chromeos/arc... File chrome/browser/chromeos/arc/arc_navigation_throttle.cc (right): https://codereview.chromium.org/2347273002/diff/1/chrome/browser/chromeos/arc... chrome/browser/chromeos/arc/arc_navigation_throttle.cc:252: if (handle->GetWebContents()->GetController().IsInitialNavigation()) On 2016/09/16 22:42:04, Yusuke Sato wrote: > Does handle->GetWebContents() always return a non-null object? the intent picker has an observer for the WebContents, so if there is no WebContents the picker shuts down returning CloseReason::ERROR, making it impossible to get in this part of the code.
LGTM
lgtm
The CQ bit was checked by djacobo@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Closing unused new tabs for ARC Some links open a new tab while navigating, given that we redirect some of the traffic to ARC, it can be necessary to close any unused tab. Since we are closing the contained WebContents on a tab, we need to make sure the callback within the UI doesn't get called twice, since intent picker has an observer for the WebContents. BUG=647806 TEST=manual test ========== to ========== Closing unused new tabs for ARC Some links open a new tab while navigating, given that we redirect some of the traffic to ARC, it can be necessary to close any unused tab. Since we are closing the contained WebContents on a tab, we need to make sure the callback within the UI doesn't get called twice, since intent picker has an observer for the WebContents. BUG=647806 TEST=manual test ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== Closing unused new tabs for ARC Some links open a new tab while navigating, given that we redirect some of the traffic to ARC, it can be necessary to close any unused tab. Since we are closing the contained WebContents on a tab, we need to make sure the callback within the UI doesn't get called twice, since intent picker has an observer for the WebContents. BUG=647806 TEST=manual test ========== to ========== Closing unused new tabs for ARC Some links open a new tab while navigating, given that we redirect some of the traffic to ARC, it can be necessary to close any unused tab. Since we are closing the contained WebContents on a tab, we need to make sure the callback within the UI doesn't get called twice, since intent picker has an observer for the WebContents. BUG=647806 TEST=manual test Committed: https://crrev.com/580779d7bc1eda07b42bd467d60ca6c18765c3db Cr-Commit-Position: refs/heads/master@{#419335} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/580779d7bc1eda07b42bd467d60ca6c18765c3db Cr-Commit-Position: refs/heads/master@{#419335} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
