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

Issue 747420 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug-Regression



Sign in to add a comment

100 kb regression in resource_sizes (MonochromePublic.apk) at 488465:488465

Project Member Reported by estevenson@chromium.org, Jul 21 2017

Issue description

Caused by "[RemotePlayback] Add media source info to the RemotePlayback availability url"

Commit: 09fa66e1baaa95d069bbd6620670311ae8aa4dc5

Link to size graph: https://chromeperf.appspot.com/report?sid=a097e74b1aa288511afb4cb616efe0f95ba4d347ad61d5e835072f23450938ba&num_points=10&rev=488465

Debugging size regressions is documented at: https://chromium.googlesource.com/chromium/src/+/master/docs/speed/apk_size_regressions.md#Debugging-Apk-Size-Increase

All native code growth (see attached diff for details).

It's not clear to me whether or not this increase was expected. From the CL description it seems that the size growth in this CL is from enabling the feature on Android but I wanted to double check since 100 kb is a large jump. Is there any way we can reduce the size here?

Please have a look and either:
  1. Close as “Won't Fix” with a short justification, or
  2. Land a revert / fix-up.

Thanks!
 
diff_results.txt
317 KB View Download
Cc: estevenson@chromium.org
Cc: m...@chromium.org mlamouri@chromium.org
Components: Internals>Cast>MediaFling Blink>Media>RemotePlayback
Looks like we could try to remove RPC Protobuf code and the demuxer from the Android build to make the regression smaller. I'll take a stab at it next week.
It's unclear to me from the CL description if this change is something needed for M61. If it was just a step along the way to a feature, and doesn't add functionality by itself, I'd like to revert it on the M61 branch (it slipped in right before branch cut).
I'm okay with reverting it on the branch, it's behind the flag anyway.
I have a long-term fix removing ~70Kb here: https://chromium-review.googlesource.com/c/584019
Cc: amineer@chromium.org
Labels: Merge-Request-61
+amineer - Okay if we revert this on M61 branch?
Note: not reverting on master, so there's no existing commit to merge. 
Labels: -Merge-Request-61 Merge-Approved-61
Labels: -Merge-Approved-61 Merge-Merged
Merged to branch as:
 https://chromium.googlesource.com/chromium/src/+/312663539c586249c35f9f4a9ee94162fb860a57
Project Member

Comment 9 by bugdroid1@chromium.org, Jul 28 2017

Labels: merge-merged-3163
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/312663539c586249c35f9f4a9ee94162fb860a57

commit 312663539c586249c35f9f4a9ee94162fb860a57
Author: Andrew Grieve <agrieve@chromium.org>
Date: Fri Jul 28 15:34:52 2017

Revert "[RemotePlayback] Add media source info to the RemotePlayback availability url"

This reverts commit 09fa66e1baaa95d069bbd6620670311ae8aa4dc5.

TBR=agrieve@chromium.org

(cherry picked from commit ec724bb577b376824afcfe1e71e4c06a97357d9f)

Bug:  747420 
Change-Id: I193769115a2304ebf5c25c1c82fa57bcad96838b
Reviewed-on: https://chromium-review.googlesource.com/591928
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#98}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/312663539c586249c35f9f4a9ee94162fb860a57/content/renderer/media/webmediaplayer_ms_unittest.cc
[modify] https://crrev.com/312663539c586249c35f9f4a9ee94162fb860a57/media/base/media_observer.h
[modify] https://crrev.com/312663539c586249c35f9f4a9ee94162fb860a57/media/base/renderer_factory_selector.cc
[modify] https://crrev.com/312663539c586249c35f9f4a9ee94162fb860a57/media/blink/webmediaplayer_impl.cc
[modify] https://crrev.com/312663539c586249c35f9f4a9ee94162fb860a57/media/blink/webmediaplayer_impl.h
[modify] https://crrev.com/312663539c586249c35f9f4a9ee94162fb860a57/media/blink/webmediaplayer_impl_unittest.cc
[modify] https://crrev.com/312663539c586249c35f9f4a9ee94162fb860a57/media/media_options.gni
[modify] https://crrev.com/312663539c586249c35f9f4a9ee94162fb860a57/media/remoting/renderer_controller.cc
[modify] https://crrev.com/312663539c586249c35f9f4a9ee94162fb860a57/media/remoting/renderer_controller.h
[modify] https://crrev.com/312663539c586249c35f9f4a9ee94162fb860a57/media/remoting/renderer_controller_unittest.cc
[modify] https://crrev.com/312663539c586249c35f9f4a9ee94162fb860a57/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp
[modify] https://crrev.com/312663539c586249c35f9f4a9ee94162fb860a57/third_party/WebKit/Source/core/html/HTMLMediaElement.h
[modify] https://crrev.com/312663539c586249c35f9f4a9ee94162fb860a57/third_party/WebKit/Source/modules/remoteplayback/RemotePlayback.cpp
[modify] https://crrev.com/312663539c586249c35f9f4a9ee94162fb860a57/third_party/WebKit/Source/modules/remoteplayback/RemotePlayback.h
[modify] https://crrev.com/312663539c586249c35f9f4a9ee94162fb860a57/third_party/WebKit/Source/modules/remoteplayback/RemotePlaybackTest.cpp
[modify] https://crrev.com/312663539c586249c35f9f4a9ee94162fb860a57/third_party/WebKit/public/platform/WebMediaPlayerClient.h
[modify] https://crrev.com/312663539c586249c35f9f4a9ee94162fb860a57/third_party/WebKit/public/platform/modules/remoteplayback/WebRemotePlaybackClient.h

Project Member

Comment 10 by bugdroid1@chromium.org, Aug 8 2017

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

commit 45929e1d192f1a68acc2d88c4f13c45232c5043a
Author: Anton Vayvod <avayvod@google.com>
Date: Tue Aug 08 01:23:18 2017

Disable Media Remoting RPC on Android

Bug:  747420 
Test: Linux and Android builds succeed.
Change-Id: Iefdd3e65783a44410936053f66fff4e58cbdab99
Reviewed-on: https://chromium-review.googlesource.com/584019
Commit-Queue: Anton Vayvod <avayvod@chromium.org>
Reviewed-by: Thomas Guilbert <tguilbert@chromium.org>
Reviewed-by: Yuri Wiitala <miu@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492491}
[modify] https://crrev.com/45929e1d192f1a68acc2d88c4f13c45232c5043a/media/BUILD.gn
[modify] https://crrev.com/45929e1d192f1a68acc2d88c4f13c45232c5043a/media/media_options.gni
[modify] https://crrev.com/45929e1d192f1a68acc2d88c4f13c45232c5043a/media/remoting/BUILD.gn
[modify] https://crrev.com/45929e1d192f1a68acc2d88c4f13c45232c5043a/media/remoting/courier_renderer_factory.cc
[modify] https://crrev.com/45929e1d192f1a68acc2d88c4f13c45232c5043a/media/remoting/fake_remoter.cc
[modify] https://crrev.com/45929e1d192f1a68acc2d88c4f13c45232c5043a/media/remoting/renderer_controller.cc
[modify] https://crrev.com/45929e1d192f1a68acc2d88c4f13c45232c5043a/media/remoting/renderer_controller.h
[modify] https://crrev.com/45929e1d192f1a68acc2d88c4f13c45232c5043a/media/remoting/shared_session.cc
[modify] https://crrev.com/45929e1d192f1a68acc2d88c4f13c45232c5043a/media/remoting/shared_session.h

Status: Fixed (was: Assigned)
Graph shows a 60 kb reduction: https://chromeperf.appspot.com/group_report?keys=agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgmd6yqgoM.

Thanks for following up on this!

Sign in to add a comment