Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(347)

Issue 308793007: Fix Chromoting Android client crashed on launch (Closed)

Created:
6 years, 6 months ago by kelvinp
Modified:
6 years, 6 months ago
Reviewers:
Sergey Ulanov, Lambros
CC:
chromium-reviews, chromoting-reviews_chromium.org
Visibility:
Public.

Description

Fix Chromoting Android client crashed on launch We crash because mRefreshButton is null when updateUI() is called. mRefreshButton is initialized on onCreateOptionsMenu() According to http://stackoverflow.com/questions/13267030/oncreateoptionsmenu-is-never-called, seems like onCreateOptionMenu will never got called on some themes on phones with menu button. I can repro by overriding the theme to @android:style/Theme.Black.NoTitleBar The fix is adding a null check. BUG=364591 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=274354

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+3 lines, -3 lines) Patch
M remoting/android/java/src/org/chromium/chromoting/Chromoting.java View 2 chunks +3 lines, -3 lines 1 comment Download

Messages

Total messages: 6 (0 generated)
kelvinp
6 years, 6 months ago (2014-05-31 01:36:49 UTC) #1
Sergey Ulanov
Please update CL description to follow the format suggested here: http://dev.chromium.org/developers/contributing-code#TOC-Change-List-Description-Structured-Elements Particularly first line should ...
6 years, 6 months ago (2014-06-01 03:45:52 UTC) #2
Lambros
lgtm when description is updated. Please see comment as well. https://codereview.chromium.org/308793007/diff/1/remoting/android/java/src/org/chromium/chromoting/Chromoting.java File remoting/android/java/src/org/chromium/chromoting/Chromoting.java (right): https://codereview.chromium.org/308793007/diff/1/remoting/android/java/src/org/chromium/chromoting/Chromoting.java#newcode399 ...
6 years, 6 months ago (2014-06-02 17:07:22 UTC) #3
kelvinp
The CQ bit was checked by kelvinp@chromium.org
6 years, 6 months ago (2014-06-02 18:57:05 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/kelvinp@chromium.org/308793007/1
6 years, 6 months ago (2014-06-02 18:58:56 UTC) #5
commit-bot: I haz the power
6 years, 6 months ago (2014-06-02 22:26:24 UTC) #6
Message was sent while issue was closed.
Change committed as 274354

Powered by Google App Engine
This is Rietveld 408576698