Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2808723002:
Remove unneeded calls to plugin.SetParentVisible. (Closed)
|
Created:
3 years, 8 months ago by joelhockey Modified:
3 years, 8 months ago CC:
blink-reviews, blink-reviews-frames_chromium.org, chromium-reviews Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionRemove unneeded calls to plugin.SetParentVisible.
Plugin (WebPluginContainerImpl) does not keep its own state about
visibility of parent, and once it is no longer inheriting from
FrameViewBase, it will not implement any ParentVisible methods.
If required, it could always call parent_.IsVisible.
BUG=637460
Review-Url: https://codereview.chromium.org/2808723002
Cr-Commit-Position: refs/heads/master@{#463409}
Committed: https://chromium.googlesource.com/chromium/src/+/e62eb16ec0d97fe0b59d39e3c5f9c0669e6d9cff
Patch Set 1 #Patch Set 2 : No FrameRectsChanged #Patch Set 3 : FrameRectsChanged is required #Messages
Total messages: 20 (14 generated)
The CQ bit was checked by joelhockey@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.
Description was changed from ========== [TEST] Do not set plugin parent visible. BUG= ========== to ========== Remove unneeded calls to plugin.SetParentVisible. BUG=637460 ==========
joelhockey@chromium.org changed reviewers: + dcheng@chromium.org, haraken@chromium.org
The CQ bit was checked by joelhockey@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...
Description was changed from ========== Remove unneeded calls to plugin.SetParentVisible. BUG=637460 ========== to ========== Remove unneeded calls to plugin.SetParentVisible. Plugin (WebPluginContainerImpl) does not keep its own state about visibility of parent, and once it is no longer inheriting from FrameViewBase, it will not implement any ParentVisible methods. If required, it could always call parent_.IsVisible. BUG=637460 ==========
LGTM
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 joelhockey@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": 40001, "attempt_start_ts": 1491860357918190,
"parent_rev": "75f0c42bc1fa4789792e165292bef3334b97151e", "commit_rev":
"e62eb16ec0d97fe0b59d39e3c5f9c0669e6d9cff"}
Message was sent while issue was closed.
Description was changed from ========== Remove unneeded calls to plugin.SetParentVisible. Plugin (WebPluginContainerImpl) does not keep its own state about visibility of parent, and once it is no longer inheriting from FrameViewBase, it will not implement any ParentVisible methods. If required, it could always call parent_.IsVisible. BUG=637460 ========== to ========== Remove unneeded calls to plugin.SetParentVisible. Plugin (WebPluginContainerImpl) does not keep its own state about visibility of parent, and once it is no longer inheriting from FrameViewBase, it will not implement any ParentVisible methods. If required, it could always call parent_.IsVisible. BUG=637460 Review-Url: https://codereview.chromium.org/2808723002 Cr-Commit-Position: refs/heads/master@{#463409} Committed: https://chromium.googlesource.com/chromium/src/+/e62eb16ec0d97fe0b59d39e3c5f9... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/e62eb16ec0d97fe0b59d39e3c5f9...
Message was sent while issue was closed.
LGTM
Message was sent while issue was closed.
A revert of this CL (patchset #3 id:40001) has been created in https://codereview.chromium.org/2809913004/ by joelhockey@chromium.org. The reason for reverting is: Although no tests failed from this change, and it seems that pepper plugins do nothing for visibility changes: https://cs.chromium.org/chromium/src/content/renderer/pepper/pepper_webplugin... I think that this is still important for browser_plugin: https://cs.chromium.org/chromium/src/content/renderer/browser_plugin/browser_.... |
