Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2938093002:
[Media Router] Remove an extra call to load extensionview (Closed)
|
Created:
3 years, 6 months ago by takumif Modified:
3 years, 6 months ago Reviewers:
imcheng CC:
chromium-reviews, chromium-apps-reviews_chromium.org, mfoltz+watch_chromium.org, arv+watch_chromium.org, imcheng+watch_chromium.org, extensions-reviews_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
Description[Media Router] Remove an extra call to load extensionview
Before the WebUI route controller patch, we were loading the extensionview only
when there were route changes. After the patch we are loading it on ready() as
well, which causes the extensionview to crash if casting to Hangouts.
BUG=733432
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
Review-Url: https://codereview.chromium.org/2938093002
Cr-Commit-Position: refs/heads/master@{#479819}
Committed: https://chromium.googlesource.com/chromium/src/+/a2c2c3e846345cc803c7cf14c210812eff757c22
Patch Set 1 #
Messages
Total messages: 19 (15 generated)
Description was changed from ========== [Media Router] Remove extra call to load extensionview BUG= ========== to ========== [Media Router] Remove extra call to load extensionview BUG= CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
Description was changed from ========== [Media Router] Remove extra call to load extensionview BUG= CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== [Media Router] Remove an extra call to load extensionview Before the WebUI route controller patch, we were loading the extensionview only when there were route changes. After the patch we are loading it on ready() as well, which causes the extensionview to crash if casting to Hangouts. BUG=733432 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
Description was changed from ========== [Media Router] Remove an extra call to load extensionview Before the WebUI route controller patch, we were loading the extensionview only when there were route changes. After the patch we are loading it on ready() as well, which causes the extensionview to crash if casting to Hangouts. BUG=733432 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== [Media Router] Remove an extra call to load extensionview Before the WebUI route controller patch, we were loading the extensionview only when there were route changes. After the patch we are loading it on ready() as well, which causes the extensionview to crash if casting to Hangouts. BUG=733432 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
takumif@chromium.org changed reviewers: + imcheng@chromium.org
Please take a look. Looking back at the WebUI controller patch, I cannot recall why I put the maybeLoadExtensionView_() call in the ready() function as well. I did some manual testing and didn't see any issues with removing the call. Also the call wasn't necessary before the WebUI controller patch to begin with. So it looks okay to me to remove.
lgtm
The CQ bit was checked by takumif@chromium.org 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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
The CQ bit was checked by takumif@chromium.org 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...
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 takumif@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 1, "attempt_start_ts": 1497559130056620, "parent_rev":
"455f6d8c01ce70db7f311a0308e943fc7b612358", "commit_rev":
"a2c2c3e846345cc803c7cf14c210812eff757c22"}
Message was sent while issue was closed.
Description was changed from ========== [Media Router] Remove an extra call to load extensionview Before the WebUI route controller patch, we were loading the extensionview only when there were route changes. After the patch we are loading it on ready() as well, which causes the extensionview to crash if casting to Hangouts. BUG=733432 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== [Media Router] Remove an extra call to load extensionview Before the WebUI route controller patch, we were loading the extensionview only when there were route changes. After the patch we are loading it on ready() as well, which causes the extensionview to crash if casting to Hangouts. BUG=733432 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2938093002 Cr-Commit-Position: refs/heads/master@{#479819} Committed: https://chromium.googlesource.com/chromium/src/+/a2c2c3e846345cc803c7cf14c210... ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1) as https://chromium.googlesource.com/chromium/src/+/a2c2c3e846345cc803c7cf14c210... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
