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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 6995099: Revert 88309 - Introduced additional spellcheck related histograms. (Closed)

Created:
9 years, 6 months ago by gmorrita
Modified:
9 years, 6 months ago
Reviewers:
Hajime Morrita
CC:
chromium-reviews, joi+watch-content_chromium.org, jam
Visibility:
Public.

Description

Revert 88309 - Introduced additional spellcheck related histograms. I revert this because of the lack of appropriate review. This change added: - "SpellCheck.SuggestionHitRatio" (percentage) - "SpellCheck.SuggestionHitRatio" (boolean) TEST=manual TBR=darin BUG=none Review URL: http://codereview.chromium.org/7121006 TBR=morrita@chromium.org Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=88456

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+7 lines, -45 lines) Patch
M chrome/browser/spellcheck_host.h View 1 chunk +0 lines, -4 lines 0 comments Download
M chrome/browser/spellcheck_host_impl.h View 3 chunks +0 lines, -10 lines 0 comments Download
M chrome/browser/spellcheck_host_impl.cc View 3 chunks +7 lines, -24 lines 0 comments Download
M content/browser/renderer_host/render_view_host.cc View 2 chunks +0 lines, -7 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
gmorrita
9 years, 6 months ago (2011-06-09 01:11:13 UTC) #1
jam
please also revert the change to the DEPS file On Wed, Jun 8, 2011 at ...
9 years, 6 months ago (2011-06-09 01:12:56 UTC) #2
gmorrita
9 years, 6 months ago (2011-06-09 01:32:33 UTC) #3 
On 2011/06/09 01:12:56, John Abd-El-Malek wrote:
> please also revert the change to the DEPS file
Sure. Done at http://src.chromium.org/viewvc/chrome?view=rev&revision=88461.
> 
> On Wed, Jun 8, 2011 at 6:11 PM, <mailto:morrita@google.com> wrote:
> 
> > Reviewers: morrita1,
> >
> > Description:
> > Revert 88309 - Introduced additional spellcheck related histograms.
> >
> > I revert this because of the lack of appropriate review.
> > This change added:
> > - "SpellCheck.**SuggestionHitRatio" (percentage)
> > - "SpellCheck.**SuggestionHitRatio" (boolean)
> >
> > TEST=manual
> > TBR=darin
> > BUG=none
> >
> > Review URL:
>
http://codereview.chromium.**org/7121006%3Chttp://codereview.chromium.org/712...>
> >
> > mailto:TBR=morrita@chromium.org
> >
> > Please review this at
>
http://codereview.chromium.**org/6995099/%3Chttp://codereview.chromium.org/69...>
> >
> > SVN Base:
>
svn://svn.chromium.org/chrome/**trunk/src/<http://svn.chromium.org/chrome/trunk/src/>
> >
> > Affected files:
> >  M     chrome/browser/spellcheck_**host.h
> >  M     chrome/browser/spellcheck_**host_impl.h
> >  M     chrome/browser/spellcheck_**host_impl.cc
> >  M     content/browser/renderer_host/**render_view_host.cc
> >
> >
> > Index: chrome/browser/spellcheck_**host.h
> > ==============================**==============================**=======
> > --- chrome/browser/spellcheck_**host.h    (revision 88455)
> > +++ chrome/browser/spellcheck_**host.h    (working copy)
> > @@ -88,10 +88,6 @@
> >   // to be uploaded via UMA
> >   virtual void RecordCheckedWordStats(bool misspell) = 0;
> >
> > -  // Collects a histogram for context menu showing as a spell correction
> > -  // attempt to be uploaded via UMA
> > -  virtual void RecordSuggestionStats(int delta) = 0;
> > -
> >   // Collects a histogram for misspelled word replacement
> >   // to be uploaded via UMA
> >   virtual void RecordReplacedWordStats(int delta) = 0;
> > Index: chrome/browser/spellcheck_**host_impl.cc
> > ==============================**==============================**=======
> > --- chrome/browser/spellcheck_**host_impl.cc      (revision 88455)
> > +++ chrome/browser/spellcheck_**host_impl.cc      (working copy)
> > @@ -91,7 +91,6 @@
> >       request_context_getter_(**request_context_getter),
> >       misspelled_word_count_(0),
> >       spellchecked_word_count_(0),
> > -      suggestion_count_(0),
> >       replaced_word_count_(0) {
> >   DCHECK(observer_);
> >   DCHECK(BrowserThread::**CurrentlyOn(BrowserThread::UI)**);
> > @@ -316,30 +315,16 @@
> >   UMA_HISTOGRAM_PERCENTAGE("**SpellCheck.MisspellRatio", percentage);
> >  }
> >
> > -void SpellCheckHostImpl::**RecordDictionaryCorruptionStat**s(bool
> > corrupted) {
> > -  UMA_HISTOGRAM_BOOLEAN("**SpellCheck.**DictionaryCorrupted", corrupted);
> > -}
> > -
> > -void SpellCheckHostImpl::**RecordSuggestionStats(int delta) {
> > -  suggestion_count_ += delta;
> > -  RecordReplacedWordStats(0);
> > -}
> > -
> >  void SpellCheckHostImpl::**RecordReplacedWordStats(int delta) {
> >   replaced_word_count_ += delta;
> > -
> > -  if (misspelled_word_count_) {
> > -    // zero |misspelled_word_count_| is possible when an extension
> > -    // gives the misspelling, which is not recorded as a part of this
> > -    // metrics.
> > -    int percentage = (100 * replaced_word_count_) /
> > misspelled_word_count_;
> > -    UMA_HISTOGRAM_PERCENTAGE("**SpellCheck.ReplaceRatio", percentage);
> > +  if (!misspelled_word_count_) {
> > +    // This is possible when an extension gives the misspelling,
> > +    // which is not recorded as a part of this metrics.
> > +    return;
> >   }
> >
> > -  if (suggestion_count_) {
> > -    int percentage = (100 * replaced_word_count_) / suggestion_count_;
> > -    UMA_HISTOGRAM_PERCENTAGE("**SpellCheck.SuggestionHitRatio"**,
> > percentage);
> > -  }
> > +  int percentage = (100 * replaced_word_count_) / misspelled_word_count_;
> > +  UMA_HISTOGRAM_PERCENTAGE("**SpellCheck.ReplaceRatio", percentage);
> >  }
> >
> >  void SpellCheckHostImpl::**OnURLFetchComplete(const URLFetcher* source,
> > @@ -387,9 +372,7 @@
> >
> >   // To prevent corrupted dictionary data from causing a renderer crash,
> > scan
> >   // the dictionary data and verify it is sane before save it to a file.
> > -  bool verified = hunspell::BDict::Verify(data_.**data(), data_.size());
> > -  RecordDictionaryCorruptionStat**s(!verified);
> > -  if (!verified) {
> > +  if (!hunspell::BDict::Verify(**data_.data(), data_.size())) {
> >     LOG(ERROR) << "Failure to verify the downloaded dictionary.";
> >     BrowserThread::PostTask(**BrowserThread::UI, FROM_HERE,
> >         NewRunnableMethod(this,
> > Index: chrome/browser/spellcheck_**host_impl.h
> > ==============================**==============================**=======
> > --- chrome/browser/spellcheck_**host_impl.h       (revision 88455)
> > +++ chrome/browser/spellcheck_**host_impl.h       (working copy)
> > @@ -87,10 +87,6 @@
> >   // Write a custom dictionary addition to disk.
> >   void WriteWordToCustomDictionary(**const std::string& word);
> >
> > -  // Collects a histogram for dictionary corruption rate
> > -  // to be uploaded via UMA
> > -  void RecordDictionaryCorruptionStat**s(bool corrupted);
> > -
> >   // Collects status of spellchecking enabling state, which is
> >   // to be uploaded via UMA
> >   virtual void RecordCheckedWordStats(bool misspell);
> > @@ -99,10 +95,6 @@
> >   // to be uploaded via UMA
> >   virtual void RecordReplacedWordStats(int delta);
> >
> > -  // Collects a histogram for context menu showing as a spell correction
> > -  // attempt to be uploaded via UMA
> > -  virtual void RecordSuggestionStats(int delta);
> > -
> >   // URLFetcher::Delegate implementation.  Called when we finish
> > downloading the
> >   // spellcheck dictionary; saves the dictionary to |data_|.
> >   virtual void OnURLFetchComplete(const URLFetcher* source,
> > @@ -161,8 +153,6 @@
> >   int misspelled_word_count_;
> >   // Number of checked words.
> >   int spellchecked_word_count_;
> > -  // Number of suggestion list showings.
> > -  int suggestion_count_;
> >   // Number of misspelled words replaced by a user.
> >   int replaced_word_count_;
> >  };
> > Index: content/browser/renderer_host/**render_view_host.cc
> > ==============================**==============================**=======
> > --- content/browser/renderer_host/**render_view_host.cc   (revision 88455)
> > +++ content/browser/renderer_host/**render_view_host.cc   (working copy)
> > @@ -16,7 +16,6 @@
> >  #include "base/utf_string_conversions.**h"
> >  #include "base/values.h"
> >  #include "chrome/browser/profiles/**profile.h"
> > -#include "chrome/browser/spellcheck_**host.h"
> >  #include "content/browser/browser_**message_filter.h"
> >  #include "content/browser/child_**process_security_policy.h"
> >  #include "content/browser/content_**browser_client.h"
> > @@ -961,12 +960,6 @@
> >   FilterURL(policy, renderer_id, &validated_params.frame_url);
> >
> >   view->ShowContextMenu(**validated_params);
> > -
> > -  Profile* profile = process()->profile();
> > -  DCHECK(profile);
> > -  if (!validated_params.dictionary_**suggestions.empty() &&
> > -      profile->GetSpellCheckHost())
> > -      profile->GetSpellCheckHost()->**RecordSuggestionStats(1);
> >  }
> >
> >  void RenderViewHost::OnMsgOpenURL(**const GURL& url,
> >
> >
> >

Powered by Google App Engine
This is Rietveld 408576698